On 21/05/2015 23:23, jglisse@xxxxxxxxxx wrote:
> +int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem)
> +{
> + struct mm_struct *mm = get_task_mm(current);
> + struct ib_device *ib_device = context->device;
> + struct ib_mirror *ib_mirror;
> + struct pid *our_pid;
> + int ret;
> +
> + if (!mm || !ib_device->hmm_ready)
> + return -EINVAL;
> +
> + /* FIXME can this really happen ? */
No, following Yann Droneaud's patch 8abaae62f3fd ("IB/core: disallow
registering 0-sized memory region") ib_umem_get() checks against zero
sized umems.
> + if (unlikely(ib_umem_start(umem) == ib_umem_end(umem)))
> + return -EINVAL;
> +
> + /* Prevent creating ODP MRs in child processes */
> + rcu_read_lock();
> + our_pid = get_task_pid(current->group_leader, PIDTYPE_PID);
> + rcu_read_unlock();
> + put_pid(our_pid);
> + if (context->tgid != our_pid) {
> + mmput(mm);
> + return -EINVAL;
> + }
> +
> + umem->hugetlb = 0;
> + umem->odp_data = kmalloc(sizeof(*umem->odp_data), GFP_KERNEL);
> + if (umem->odp_data == NULL) {
> + mmput(mm);
> + return -ENOMEM;
> + }
> + umem->odp_data->private = NULL;
> + umem->odp_data->umem = umem;
> +
> + mutex_lock(&ib_device->hmm_mutex);
> + /* Is there an existing mirror for this process mm ? */
> + ib_mirror = ib_mirror_ref(context->ib_mirror);
> + if (!ib_mirror)
> + list_for_each_entry(ib_mirror, &ib_device->ib_mirrors, list) {
> + if (ib_mirror->base.hmm->mm != mm)
> + continue;
> + ib_mirror = ib_mirror_ref(ib_mirror);
> + break;
> + }
> +
> + if (ib_mirror == NULL ||
> + ib_mirror == list_first_entry(&ib_device->ib_mirrors,
> + struct ib_mirror, list)) {
Is the second check an attempt to check if the list_for_each_entry above
passed through all the entries and didn't break? Maybe I missing
something, but I think that would cause the ib_mirror to hold a pointer
such that ib_mirror->list == ib_mirrors (point to the list head), and
not to the first entry.
In any case, I think it would be more clear if you add another ib_mirror
variable for iterating the ib_mirrors list.
> + /* We need to create a new mirror. */
> + ib_mirror = kmalloc(sizeof(*ib_mirror), GFP_KERNEL);
> + if (ib_mirror == NULL) {
> + mutex_unlock(&ib_device->hmm_mutex);
> + mmput(mm);
> + return -ENOMEM;
> + }
> + kref_init(&ib_mirror->kref);
> + init_rwsem(&ib_mirror->hmm_mr_rwsem);
> + ib_mirror->umem_tree = RB_ROOT;
> + ib_mirror->ib_device = ib_device;
> +
> + ib_mirror->base.device = &ib_device->hmm_dev;
> + ret = hmm_mirror_register(&ib_mirror->base);
> + if (ret) {
> + mutex_unlock(&ib_device->hmm_mutex);
> + kfree(ib_mirror);
> + mmput(mm);
> + return ret;
> + }
> +
> + list_add(&ib_mirror->list, &ib_device->ib_mirrors);
> + context->ib_mirror = ib_mirror_ref(ib_mirror);
> + }
> + mutex_unlock(&ib_device->hmm_mutex);
> + umem->odp_data.ib_mirror = ib_mirror;
> +
> + down_write(&ib_mirror->umem_rwsem);
> + rbt_ib_umem_insert(&umem->odp_data->interval_tree, &mirror->umem_tree);
> + up_write(&ib_mirror->umem_rwsem);
> +
> + mmput(mm);
> + return 0;
> +}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>