On 26/01/2017 08:12, Peter Xu wrote:
>
> /*
> * Here, we need to have the lock not only for vfio_get_vaddr(),
> * but also needs to make sure that the vaddr will be valid for
> * further operations.
> *
> * When we map new pages, we need the lock to make sure that vaddr
> * is valid along the way we build up the IO page table (via
> * vfio_dma_map()). Then, as long as the mapping is set up, we can
> * unlock since those pages will be pinned in kernel (which makes
> * sure that the RAM backend of vaddr will always be there, even
> * if the memory object is destroyed and RAM released).
> *
> * For unmapping case, we don't really need the protection since
> * the pages are in all cases locked in kernel, so we'll probably
> * be safe even without the lock. However, it won't hurt we have
> * the lock as well here.
> */
Even simpler, just before the definition of vfio_get_vaddr:
/* Called with rcu_read_lock held. */
and just before the vfio_dma_map call:
/* vaddr is only valid until rcu_read_unlock(). But after
* vfio_dma_map has set up the mapping the pages will be pinned
* by the kernel. This makes sure that the RAM backend of vaddr
* will always be there, even if the memory object is destroyed
* and its backing memory munmap-ed.
*/
I'm not sure that you can get rid of the lock for the unmapping case.
Better remove that part of the comment.
Thanks,
Paolo
