From: Dan Carpenter <dan.carpenter@oracle.com>
To: chris@chris-wilson.co.uk
Cc: intel-gfx@lists.freedesktop.org
Subject: [bug report] drm/i915: Pass around sg_table to get_pages/put_pages backend
Date: Fri, 14 Dec 2018 18:09:50 +0300 [thread overview]
Message-ID: <20181214150950.GA11341@kadam> (raw)
[ This code is old, but it's pretty confusing so I decided to send the
email anyway. - dan ]
Hello Chris Wilson,
The patch 03ac84f1830e: "drm/i915: Pass around sg_table to
get_pages/put_pages backend" from Oct 28, 2016, leads to the
following static checker warning:
drivers/gpu/drm/i915/i915_gem_userptr.c:540 __i915_gem_userptr_get_pages_worker()
warn: passing zero to 'ERR_PTR'
drivers/gpu/drm/i915/i915_gem_userptr.c
507 ret = -ENOMEM;
^^^^^^^^^^^^^^
508 pinned = 0;
509
510 pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
511 if (pvec != NULL) {
512 struct mm_struct *mm = obj->userptr.mm->mm;
513 unsigned int flags = 0;
514
515 if (!i915_gem_object_is_readonly(obj))
516 flags |= FOLL_WRITE;
517
518 ret = -EFAULT;
^^^^^^^^^^^^^
519 if (mmget_not_zero(mm)) {
520 down_read(&mm->mmap_sem);
521 while (pinned < npages) {
522 ret = get_user_pages_remote
^^^^^^^^^^^^^^^^^^^^^^^^^^^
ret can be negative or zero or positive.
523 (work->task, mm,
524 obj->userptr.ptr + pinned * PAGE_SIZE,
525 npages - pinned,
526 flags,
527 pvec + pinned, NULL, NULL);
528 if (ret < 0)
529 break;
530
531 pinned += ret;
532 }
533 up_read(&mm->mmap_sem);
534 mmput(mm);
535 }
536 }
537
538 mutex_lock(&obj->mm.lock);
539 if (obj->userptr.work == &work->work) {
540 struct sg_table *pages = ERR_PTR(ret);
^^^^^^^^^^^^^^^^^^^^
It looks to me like the static checker could right that "ret" might not
be a negative error code. Is that intentional?
541
542 if (pinned == npages) {
543 pages = __i915_gem_userptr_alloc_pages(obj, pvec,
544 npages);
545 if (!IS_ERR(pages)) {
546 pinned = 0;
547 pages = NULL;
548 }
549 }
550
551 obj->userptr.work = ERR_CAST(pages);
552 if (IS_ERR(pages))
553 __i915_gem_userptr_set_active(obj, false);
554 }
555 mutex_unlock(&obj->mm.lock);
556
557 release_pages(pvec, pinned);
558 kvfree(pvec);
559
560 i915_gem_object_put(obj);
561 put_task_struct(work->task);
562 kfree(work);
563 }
regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
reply other threads:[~2018-12-14 21:25 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181214150950.GA11341@kadam \
--to=dan.carpenter@oracle.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.