Re: [PATCH v4 4/6] io_uring: rsrc: avoid use of vmas parameter in pin_user_pages()

[Lorenzo Stoakes]

On Tue, Apr 18, 2023 at 05:55:48PM +0200, David Hildenbrand wrote:
> On 18.04.23 17:49, Lorenzo Stoakes wrote:
> > We are shortly to remove pin_user_pages(), and instead perform the required
> > VMA checks ourselves. In most cases there will be a single VMA so this
> > should caues no undue impact on an already slow path.
> > 
> > Doing this eliminates the one instance of vmas being used by
> > pin_user_pages().
> > 
> > Suggested-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Signed-off-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>
> > ---
> >   io_uring/rsrc.c | 55 ++++++++++++++++++++++++++++---------------------
> >   1 file changed, 31 insertions(+), 24 deletions(-)
> > 
> > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> > index 7a43aed8e395..3a927df9d913 100644
> > --- a/io_uring/rsrc.c
> > +++ b/io_uring/rsrc.c
> > @@ -1138,12 +1138,37 @@ static int io_buffer_account_pin(struct io_ring_ctx *ctx, struct page **pages,
> >   	return ret;
> >   }
> > +static int check_vmas_locked(unsigned long addr, unsigned long len)
> 
> TBH, the whole "_locked" suffix is a bit confusing.
> 
> I was wondering why you'd want to check whether the VMAs are locked ...
>

Yeah it's annoying partly because GUP itself is super inconsistent about
it. Idea is to try to indicate that you need to hold mmap_lock
obviously... let's drop _locked then since we're inconsistent with it anyway.

> > +{
> > +	struct file *file;
> > +	VMA_ITERATOR(vmi, current->mm, addr);
> > +	struct vm_area_struct *vma = vma_next(&vmi);
> > +	unsigned long end = addr + len;
> > +
> > +	if (WARN_ON_ONCE(!vma))
> > +		return -EINVAL;
> > +
> > +	file = vma->vm_file;
> > +	if (file && !is_file_hugepages(file))
> > +		return -EOPNOTSUPP;
> 
> You'd now be rejecting vma_is_shmem() here, no?
>

Good spot, I guess I was confused that io_uring would actually want to do
that... not sure who'd want to actually mapping some shmem for this
purpose!

I'll update to make it match the existing code.

> 
> -- 
> Thanks,
> 
> David / dhildenb
> 

To avoid spam, here's a -fix patch for both:-

----8<----