On Thu, May 03, 2018 at 10:04:34PM +0800, Lidong Chen wrote:
> The userspace may invoke ibv_reg_mr and ibv_dereg_mr by different threads.
> If when ibv_dereg_mr invoke and the thread which invoked ibv_reg_mr has
> exited, get_pid_task will return NULL, ib_umem_release does not decrease
> mm->pinned_vm. This patch fixes it by use tgid.
>
> Signed-off-by: Lidong Chen <lidongchen@xxxxxxxxxxx>
> ---
> drivers/infiniband/core/umem.c | 12 ++++++------
> include/rdma/ib_umem.h | 2 +-
> 2 files changed, 7 insertions(+), 7 deletions(-)
Why are we even using a struct pid for this? Does anyone know?
I'm surprised that struct task isn't held in the struct ib_umem..
Is group_leader really OK and always guarenteed to return the same
struct mm?? For some reason I have this recollection that the leader
can change under some situation..
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 9a4e899..8813ba5 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -119,7 +119,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> umem->length = size;
> umem->address = addr;
> umem->page_shift = PAGE_SHIFT;
> - umem->pid = get_task_pid(current, PIDTYPE_PID);
> + umem->tgid = get_task_pid(current->group_leader, PIDTYPE_PID);
> /*
> * We ask for writable memory if any of the following
> * access flags are set. "Local write" and "remote write"
> @@ -132,7 +132,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
>
> if (access & IB_ACCESS_ON_DEMAND) {
> - put_pid(umem->pid);
> + put_pid(umem->tgid);
> ret = ib_umem_odp_get(context, umem, access);
> if (ret) {
> kfree(umem);
> @@ -148,7 +148,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>
> page_list = (struct page **) __get_free_page(GFP_KERNEL);
> if (!page_list) {
> - put_pid(umem->pid);
> + put_pid(umem->tgid);
> kfree(umem);
> return ERR_PTR(-ENOMEM);
> }
> @@ -231,7 +231,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> if (ret < 0) {
> if (need_release)
> __ib_umem_release(context->device, umem, 0);
> - put_pid(umem->pid);
> + put_pid(umem->tgid);
> kfree(umem);
> } else
> current->mm->pinned_vm = locked;
> @@ -274,8 +274,8 @@ void ib_umem_release(struct ib_umem *umem)
>
> __ib_umem_release(umem->context->device, umem, 1);
>
> - task = get_pid_task(umem->pid, PIDTYPE_PID);
> - put_pid(umem->pid);
> + task = get_pid_task(umem->tgid, PIDTYPE_PID);
> + put_pid(umem->tgid);
> if (!task)
> goto out;
> mm = get_task_mm(task);
> diff --git a/include/rdma/ib_umem.h b/include/rdma/ib_umem.h
> index 23159dd..2398849 100644
> --- a/include/rdma/ib_umem.h
> +++ b/include/rdma/ib_umem.h
> @@ -48,7 +48,7 @@ struct ib_umem {
> int writable;
> int hugetlb;
> struct work_struct work;
> - struct pid *pid;
> + struct pid *tgid;
> struct mm_struct *mm;
> unsigned long diff;
> struct ib_umem_odp *odp_data;
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
