On Wed, Oct 07, 2020 at 07:12:24PM +0200, Daniel Vetter wrote: > On Wed, Oct 7, 2020 at 6:53 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > > > On Wed, Oct 07, 2020 at 06:44:18PM +0200, Daniel Vetter wrote: > > > > > > - /* > > > - * While get_vaddr_frames() could be used for transient (kernel > > > - * controlled lifetime) pinning of memory pages all current > > > - * users establish long term (userspace controlled lifetime) > > > - * page pinning. Treat get_vaddr_frames() like > > > - * get_user_pages_longterm() and disallow it for filesystem-dax > > > - * mappings. > > > - */ > > > - if (vma_is_fsdax(vma)) { > > > - ret = -EOPNOTSUPP; > > > - goto out; > > > - } > > > - > > > - if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > - vec->got_ref = true; > > > - vec->is_pfns = false; > > > - ret = pin_user_pages_locked(start, nr_frames, > > > - gup_flags, (struct page **)(vec->ptrs), &locked); > > > - goto out; > > > - } > > > > The vm_flags still need to be checked before going into the while > > loop. If the break is taken then nothing would check vm_flags > > Hm right that's a bin inconsistent. follow_pfn also checks for this, > so I think we can just ditch this entirely both here and in the do {} > while () check, simplifying the latter to just while (vma). Well, just > make it a real loop with less confusing control flow probably. It does read very poorly with the redundant check, espeically since I keep forgetting follow_pfn does it too :\ Jason _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel