Re: [RESEND PATCH 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
On Thu, Feb 21, 2019 at 08:48:41AM +0530, Souptick Joarder wrote: > Hi Ira, > > On Wed, Feb 20, 2019 at 11:01 AM wrote: > > > > From: Ira Weiny > > > > To facilitate additional options to get_user_pages_fast() change the > > singular write parameter to be gup_flags. > > > > This patch does not change any functionality. New functionality will > > follow in subsequent patches. > > > > Some of the get_user_pages_fast() call sites were unchanged because they > > already passed FOLL_WRITE or 0 for the write parameter. > > > > Signed-off-by: Ira Weiny > > --- [snip] > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c > > b/arch/powerpc/kvm/book3s_64_mmu_hv.c > > index bd2dcfbf00cd..8fcb0a921e46 100644 > > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > > @@ -582,7 +582,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, > > struct kvm_vcpu *vcpu, > > /* If writing != 0, then the HPTE must allow writing, if we get > > here */ > > write_ok = writing; > > hva = gfn_to_hva_memslot(memslot, gfn); > > - npages = get_user_pages_fast(hva, 1, writing, pages); > > + npages = get_user_pages_fast(hva, 1, writing ? FOLL_WRITE : 0, > > pages); > > Just requesting for opinion, > * writing ? FOLL_WRITE : 0 * is used in many places. How about placing it in a > macro/ inline ? I don't really think this would gain much. And I don't think it would be more clear. In fact I can't even think of a macro name which would make sense. I'm inclined to leave this as written. Ira > > > if (npages < 1) { > > /* Check if it's an I/O mapping */ > > down_read(¤t->mm->mmap_sem); > > @@ -1175,7 +1175,7 @@ void *kvmppc_pin_guest_page(struct kvm *kvm, unsigned > > long gpa, > > if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID)) > > goto err; > > hva = gfn_to_hva_memslot(memslot, gfn); > > - npages = get_user_pages_fast(hva, 1, 1, pages); > > + npages = get_user_pages_fast(hva, 1, FOLL_WRITE, pages); > > if (npages < 1) > > goto err; > > page = pages[0]; > > diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c > > index 24296f4cadc6..e0af53fd78c5 100644 > > --- a/arch/powerpc/kvm/e500_mmu.c > > +++ b/arch/powerpc/kvm/e500_mmu.c > > @@ -783,7 +783,7 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu, > > if (!pages) > > return -ENOMEM; > > > > - ret = get_user_pages_fast(cfg->array, num_pages, 1, pages); > > + ret = get_user_pages_fast(cfg->array, num_pages, FOLL_WRITE, pages); > > if (ret < 0) > > goto free_pages; > > > > diff --git a/arch/powerpc/mm/mmu_context_iommu.c > > b/arch/powerpc/mm/mmu_context_iommu.c > > index a712a650a8b6..acb0990c8364 100644 > > --- a/arch/powerpc/mm/mmu_context_iommu.c > > +++ b/arch/powerpc/mm/mmu_context_iommu.c > > @@ -190,7 +190,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, > > unsigned long ua, > > for (i = 0; i < entries; ++i) { > > cur_ua = ua + (i << PAGE_SHIFT); > > if (1 != get_user_pages_fast(cur_ua, > > - 1/* pages */, 1/* iswrite */, > > &page)) { > > + 1/* pages */, FOLL_WRITE, &page)) { > > ret = -EFAULT; > > for (j = 0; j < i; ++j) > > put_page(pfn_to_page(mem->hpas[j] >> > > @@ -209,7 +209,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, > > unsigned long ua, > > if (mm_iommu_move_page_from_cma(page)) > > goto populate; > > if (1 != get_user_pages_fast(cur_ua, > > - 1/* pages */, 1/* iswrite > > */, > > + 1/* pages */, FOLL_WRITE, > > &page)) { > > ret = -EFAULT; > > for (j = 0; j < i; ++j) > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > > index fcb55b02990e..69d9366b966c 100644 > > --- a/arch/s390/kvm/interrupt.c > > +++ b/arch/s390/kvm/interrupt.c > > @@ -2278,7 +2278,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, > > unsigned int id, __u64 addr) > > ret = -EFAULT; > > goto out; > > } > > - ret = get_user_pages_fast(map->addr, 1, 1, &map->page); > > + ret = get_user_pages_fast(map->addr, 1, FOLL_WRITE, &map->page); > > if (ret < 0) > > goto out; > > BUG_ON(ret != 1); > > diff --git a/arch/s390/mm/gup.c b/arch/s390/mm/gup.c > > index 2809d11c7a28..0a6faf3d9960 100644 > > --- a/arch/s390/mm/gup.c > > +++ b/arch/s390/mm/gup.c > > @@ -265,7 +265,7 @@ int __get_user_pages_fast(unsigned long star
Re: [RESEND PATCH 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
Hi Ira, On Wed, Feb 20, 2019 at 11:01 AM wrote: > > From: Ira Weiny > > To facilitate additional options to get_user_pages_fast() change the > singular write parameter to be gup_flags. > > This patch does not change any functionality. New functionality will > follow in subsequent patches. > > Some of the get_user_pages_fast() call sites were unchanged because they > already passed FOLL_WRITE or 0 for the write parameter. > > Signed-off-by: Ira Weiny > --- > arch/mips/mm/gup.c | 11 ++- > arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 ++-- > arch/powerpc/kvm/e500_mmu.c| 2 +- > arch/powerpc/mm/mmu_context_iommu.c| 4 ++-- > arch/s390/kvm/interrupt.c | 2 +- > arch/s390/mm/gup.c | 12 ++-- > arch/sh/mm/gup.c | 11 ++- > arch/sparc/mm/gup.c| 9 + > arch/x86/kvm/paging_tmpl.h | 2 +- > arch/x86/kvm/svm.c | 2 +- > drivers/fpga/dfl-afu-dma-region.c | 2 +- > drivers/gpu/drm/via/via_dmablit.c | 3 ++- > drivers/infiniband/hw/hfi1/user_pages.c| 3 ++- > drivers/misc/genwqe/card_utils.c | 2 +- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/misc/vmw_vmci/vmci_queue_pair.c| 6 -- > drivers/platform/goldfish/goldfish_pipe.c | 3 ++- > drivers/rapidio/devices/rio_mport_cdev.c | 4 +++- > drivers/sbus/char/oradax.c | 2 +- > drivers/scsi/st.c | 3 ++- > drivers/staging/gasket/gasket_page_table.c | 4 ++-- > drivers/tee/tee_shm.c | 2 +- > drivers/vfio/vfio_iommu_spapr_tce.c| 3 ++- > drivers/vhost/vhost.c | 2 +- > drivers/video/fbdev/pvr2fb.c | 2 +- > drivers/virt/fsl_hypervisor.c | 2 +- > drivers/xen/gntdev.c | 2 +- > fs/orangefs/orangefs-bufmap.c | 2 +- > include/linux/mm.h | 4 ++-- > kernel/futex.c | 2 +- > lib/iov_iter.c | 7 +-- > mm/gup.c | 10 +- > mm/util.c | 8 > net/ceph/pagevec.c | 2 +- > net/rds/info.c | 2 +- > net/rds/rdma.c | 3 ++- > 36 files changed, 81 insertions(+), 65 deletions(-) > > diff --git a/arch/mips/mm/gup.c b/arch/mips/mm/gup.c > index 0d14e0d8eacf..4c2b4483683c 100644 > --- a/arch/mips/mm/gup.c > +++ b/arch/mips/mm/gup.c > @@ -235,7 +235,7 @@ int __get_user_pages_fast(unsigned long start, int > nr_pages, int write, > * get_user_pages_fast() - pin user pages in memory > * @start: starting user address > * @nr_pages: number of pages from start to pin > - * @write: whether pages will be written to > + * @gup_flags: flags modifying pin behaviour > * @pages: array that receives pointers to the pages pinned. > * Should be at least nr_pages long. > * > @@ -247,8 +247,8 @@ int __get_user_pages_fast(unsigned long start, int > nr_pages, int write, > * requested. If nr_pages is 0 or negative, returns 0. If no pages > * were pinned, returns -errno. > */ > -int get_user_pages_fast(unsigned long start, int nr_pages, int write, > - struct page **pages) > +int get_user_pages_fast(unsigned long start, int nr_pages, > + unsigned int gup_flags, struct page **pages) > { > struct mm_struct *mm = current->mm; > unsigned long addr, len, end; > @@ -273,7 +273,8 @@ int get_user_pages_fast(unsigned long start, int > nr_pages, int write, > next = pgd_addr_end(addr, end); > if (pgd_none(pgd)) > goto slow; > - if (!gup_pud_range(pgd, addr, next, write, pages, &nr)) > + if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE, > + pages, &nr)) > goto slow; > } while (pgdp++, addr = next, addr != end); > local_irq_enable(); > @@ -289,7 +290,7 @@ int get_user_pages_fast(unsigned long start, int > nr_pages, int write, > pages += nr; > > ret = get_user_pages_unlocked(start, (end - start) >> PAGE_SHIFT, > - pages, write ? FOLL_WRITE : 0); > + pages, gup_flags); > > /* Have to be a bit careful with return values */ > if (nr > 0) { > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c > b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index bd2dcfbf00cd..8fcb0a921e46 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -582,7 +582,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, > struct kvm_vcpu *vcpu, >
Re: [RESEND PATCH 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
Hi Ira Martin and I looked at your patch and agree that it doesn't change functionality for Orangefs. Reviewed-by: Mike Marshall On Wed, Feb 20, 2019 at 12:32 AM wrote: > > From: Ira Weiny > > To facilitate additional options to get_user_pages_fast() change the > singular write parameter to be gup_flags. > > This patch does not change any functionality. New functionality will > follow in subsequent patches. > > Some of the get_user_pages_fast() call sites were unchanged because they > already passed FOLL_WRITE or 0 for the write parameter. > > Signed-off-by: Ira Weiny > --- > arch/mips/mm/gup.c | 11 ++- > arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 ++-- > arch/powerpc/kvm/e500_mmu.c| 2 +- > arch/powerpc/mm/mmu_context_iommu.c| 4 ++-- > arch/s390/kvm/interrupt.c | 2 +- > arch/s390/mm/gup.c | 12 ++-- > arch/sh/mm/gup.c | 11 ++- > arch/sparc/mm/gup.c| 9 + > arch/x86/kvm/paging_tmpl.h | 2 +- > arch/x86/kvm/svm.c | 2 +- > drivers/fpga/dfl-afu-dma-region.c | 2 +- > drivers/gpu/drm/via/via_dmablit.c | 3 ++- > drivers/infiniband/hw/hfi1/user_pages.c| 3 ++- > drivers/misc/genwqe/card_utils.c | 2 +- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/misc/vmw_vmci/vmci_queue_pair.c| 6 -- > drivers/platform/goldfish/goldfish_pipe.c | 3 ++- > drivers/rapidio/devices/rio_mport_cdev.c | 4 +++- > drivers/sbus/char/oradax.c | 2 +- > drivers/scsi/st.c | 3 ++- > drivers/staging/gasket/gasket_page_table.c | 4 ++-- > drivers/tee/tee_shm.c | 2 +- > drivers/vfio/vfio_iommu_spapr_tce.c| 3 ++- > drivers/vhost/vhost.c | 2 +- > drivers/video/fbdev/pvr2fb.c | 2 +- > drivers/virt/fsl_hypervisor.c | 2 +- > drivers/xen/gntdev.c | 2 +- > fs/orangefs/orangefs-bufmap.c | 2 +- > include/linux/mm.h | 4 ++-- > kernel/futex.c | 2 +- > lib/iov_iter.c | 7 +-- > mm/gup.c | 10 +- > mm/util.c | 8 > net/ceph/pagevec.c | 2 +- > net/rds/info.c | 2 +- > net/rds/rdma.c | 3 ++- > 36 files changed, 81 insertions(+), 65 deletions(-) > > diff --git a/arch/mips/mm/gup.c b/arch/mips/mm/gup.c > index 0d14e0d8eacf..4c2b4483683c 100644 > --- a/arch/mips/mm/gup.c > +++ b/arch/mips/mm/gup.c > @@ -235,7 +235,7 @@ int __get_user_pages_fast(unsigned long start, int > nr_pages, int write, > * get_user_pages_fast() - pin user pages in memory > * @start: starting user address > * @nr_pages: number of pages from start to pin > - * @write: whether pages will be written to > + * @gup_flags: flags modifying pin behaviour > * @pages: array that receives pointers to the pages pinned. > * Should be at least nr_pages long. > * > @@ -247,8 +247,8 @@ int __get_user_pages_fast(unsigned long start, int > nr_pages, int write, > * requested. If nr_pages is 0 or negative, returns 0. If no pages > * were pinned, returns -errno. > */ > -int get_user_pages_fast(unsigned long start, int nr_pages, int write, > - struct page **pages) > +int get_user_pages_fast(unsigned long start, int nr_pages, > + unsigned int gup_flags, struct page **pages) > { > struct mm_struct *mm = current->mm; > unsigned long addr, len, end; > @@ -273,7 +273,8 @@ int get_user_pages_fast(unsigned long start, int > nr_pages, int write, > next = pgd_addr_end(addr, end); > if (pgd_none(pgd)) > goto slow; > - if (!gup_pud_range(pgd, addr, next, write, pages, &nr)) > + if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE, > + pages, &nr)) > goto slow; > } while (pgdp++, addr = next, addr != end); > local_irq_enable(); > @@ -289,7 +290,7 @@ int get_user_pages_fast(unsigned long start, int > nr_pages, int write, > pages += nr; > > ret = get_user_pages_unlocked(start, (end - start) >> PAGE_SHIFT, > - pages, write ? FOLL_WRITE : 0); > + pages, gup_flags); > > /* Have to be a bit careful with return values */ > if (nr > 0) { > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c > b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index bd2dcfbf00cd..8fcb0a921e46 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/bo