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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox