All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hongfu Li" <hongfu.li@linux.dev>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/userptr: Convert page APIs to folio equivalents in put_pages
Date: Thu, 23 Jul 2026 06:54:22 +0000	[thread overview]
Message-ID: <20260723065422.B010B1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260723064651.4100-1-hongfu.li@linux.dev>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional PAGE_SIZE stepping over a scatterlist performs non-idempotent folio operations multiple times for large folios.
--

--- Patch [1]: [PATCH] drm/i915/userptr: Convert page APIs to folio equivalents in put_pages ---
commit a2440a520ca40b971ab3143efb8ecac51e56d073
Author: Hongfu Li <lihongfu@kylinos.cn>

drm/i915/userptr: Convert page APIs to folio equivalents in put_pages

This commit replaces page-based APIs with folio equivalents in
i915_gem_userptr_put_pages(). It caches the folio at the start of the loop
to avoid repeated compound head lookups inside page API wrappers.

> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> index 043095f93ac60..bd91782f4bbc7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
[ ... ]
> @@ -177,7 +178,9 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
>  		obj->mm.dirty = false;
>  
>  	for_each_sgt_page(page, sgt_iter, pages) {
> -		if (obj->mm.dirty && trylock_page(page)) {
> +		folio = page_folio(page);
> +
> +		if (obj->mm.dirty && folio_trylock(folio)) {

[Severity: High]
This isn't a bug introduced by this patch, but does this loop process the same
folio multiple times for large folios?

In i915_gem_userptr_put_pages(), the for_each_sgt_page() macro unconditionally
steps by PAGE_SIZE. This causes page_folio() to repeatedly return the same folio
for the tail pages of a large folio.

For non-idempotent operations, this causes the folio to be processed redundantly.
If the scatterlist is backed by a 2MB THP, the same folio is processed 512
times. This means the code unnecessarily locks and unlocks the folio 512 times,
and folio_mark_accessed() causes the folio to repeatedly toggle between
referenced and active states.

Because large folios bypass per-CPU LRU batches, this hammers the global zone
LRU spinlock repeatedly per THP, causing severe system-wide lock contention.

Could we track the last processed folio to avoid duplicate processing of tail
pages in this loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723064651.4100-1-hongfu.li@linux.dev?part=1

  reply	other threads:[~2026-07-23  6:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23  6:46 [PATCH] drm/i915/userptr: Convert page APIs to folio equivalents in put_pages Hongfu Li
2026-07-23  6:54 ` sashiko-bot [this message]
2026-07-23 15:19 ` ✓ i915.CI.BAT: success for " Patchwork

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=20260723065422.B010B1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hongfu.li@linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.