On Tue, Jan 15, 2019 at 10:12:59AM -0800, Davidlohr Bueso wrote:
> usnic_uiom_get_pages() uses gup_longterm() so we cannot really
> get rid of mmap_sem altogether in the driver, but we can get
> rid of some complexity that mmap_sem brings with only pinned_vm.
> We can get rid of the wq altogether as we no longer need to
> defer work to unpin pages as the counter is now atomic.
>
> Cc: benve@xxxxxxxxx
> Cc: neescoba@xxxxxxxxx
> Cc: pkaustub@xxxxxxxxx
> Signed-off-by: Davidlohr Bueso <dbueso@xxxxxxx>
Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> ---
> drivers/infiniband/hw/usnic/usnic_ib_main.c | 2 --
> drivers/infiniband/hw/usnic/usnic_uiom.c | 54 +++--------------------------
> drivers/infiniband/hw/usnic/usnic_uiom.h | 1 -
> 3 files changed, 4 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
> index b2323a52a0dd..64bc4fda36bf 100644
> --- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
> +++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
> @@ -691,7 +691,6 @@ static int __init usnic_ib_init(void)
> out_pci_unreg:
> pci_unregister_driver(&usnic_ib_pci_driver);
> out_umem_fini:
> - usnic_uiom_fini();
>
> return err;
> }
> @@ -704,7 +703,6 @@ static void __exit usnic_ib_destroy(void)
> unregister_inetaddr_notifier(&usnic_ib_inetaddr_notifier);
> unregister_netdevice_notifier(&usnic_ib_netdevice_notifier);
> pci_unregister_driver(&usnic_ib_pci_driver);
> - usnic_uiom_fini();
> }
>
> MODULE_DESCRIPTION("Cisco VIC (usNIC) Verbs Driver");
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index 22c40c432b9e..555d7bc93e72 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -47,8 +47,6 @@
> #include "usnic_uiom.h"
> #include "usnic_uiom_interval_tree.h"
>
> -static struct workqueue_struct *usnic_uiom_wq;
> -
> #define USNIC_UIOM_PAGE_CHUNK \
> ((PAGE_SIZE - offsetof(struct usnic_uiom_chunk, page_list)) /\
> ((void *) &((struct usnic_uiom_chunk *) 0)->page_list[1] - \
> @@ -129,7 +127,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
> uiomr->owning_mm = mm = current->mm;
> down_write(&mm->mmap_sem);
>
> - locked = npages + atomic_long_read(¤t->mm->pinned_vm);
> + locked = atomic_long_add_return(npages, ¤t->mm->pinned_vm);
> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>
> if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
> @@ -185,12 +183,11 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
> }
>
> out:
> - if (ret < 0)
> + if (ret < 0) {
> usnic_uiom_put_pages(chunk_list, 0);
> - else {
> - atomic_long_set(&mm->pinned_vm, locked);
> + atomic_long_sub(npages, ¤t->mm->pinned_vm);
> + } else
> mmgrab(uiomr->owning_mm);
> - }
>
> up_write(&mm->mmap_sem);
> free_page((unsigned long) page_list);
> @@ -436,43 +433,12 @@ static inline size_t usnic_uiom_num_pages(struct usnic_uiom_reg *uiomr)
> return PAGE_ALIGN(uiomr->length + uiomr->offset) >> PAGE_SHIFT;
> }
>
> -static void usnic_uiom_release_defer(struct work_struct *work)
> -{
> - struct usnic_uiom_reg *uiomr =
> - container_of(work, struct usnic_uiom_reg, work);
> -
> - down_write(&uiomr->owning_mm->mmap_sem);
> - atomic_long_sub(usnic_uiom_num_pages(uiomr), &uiomr->owning_mm->pinned_vm);
> - up_write(&uiomr->owning_mm->mmap_sem);
> -
> - __usnic_uiom_release_tail(uiomr);
> -}
> -
> void usnic_uiom_reg_release(struct usnic_uiom_reg *uiomr,
> struct ib_ucontext *context)
> {
> __usnic_uiom_reg_release(uiomr->pd, uiomr, 1);
>
> - /*
> - * We may be called with the mm's mmap_sem already held. This
> - * can happen when a userspace munmap() is the call that drops
> - * the last reference to our file and calls our release
> - * method. If there are memory regions to destroy, we'll end
> - * up here and not be able to take the mmap_sem. In that case
> - * we defer the vm_locked accounting to a workqueue.
> - */
> - if (context->closing) {
> - if (!down_write_trylock(&uiomr->owning_mm->mmap_sem)) {
> - INIT_WORK(&uiomr->work, usnic_uiom_release_defer);
> - queue_work(usnic_uiom_wq, &uiomr->work);
> - return;
> - }
> - } else {
> - down_write(&uiomr->owning_mm->mmap_sem);
> - }
> atomic_long_sub(usnic_uiom_num_pages(uiomr), &uiomr->owning_mm->pinned_vm);
> - up_write(&uiomr->owning_mm->mmap_sem);
> -
> __usnic_uiom_release_tail(uiomr);
> }
>
> @@ -601,17 +567,5 @@ int usnic_uiom_init(char *drv_name)
> return -EPERM;
> }
>
> - usnic_uiom_wq = create_workqueue(drv_name);
> - if (!usnic_uiom_wq) {
> - usnic_err("Unable to alloc wq for drv %s\n", drv_name);
> - return -ENOMEM;
> - }
> -
> return 0;
> }
> -
> -void usnic_uiom_fini(void)
> -{
> - flush_workqueue(usnic_uiom_wq);
> - destroy_workqueue(usnic_uiom_wq);
> -}
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.h b/drivers/infiniband/hw/usnic/usnic_uiom.h
> index b86a9731071b..c88cfa087e3a 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.h
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.h
> @@ -93,5 +93,4 @@ struct usnic_uiom_reg *usnic_uiom_reg_get(struct usnic_uiom_pd *pd,
> void usnic_uiom_reg_release(struct usnic_uiom_reg *uiomr,
> struct ib_ucontext *ucontext);
> int usnic_uiom_init(char *drv_name);
> -void usnic_uiom_fini(void);
> #endif /* USNIC_UIOM_H_ */
> --
> 2.16.4
>
