[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240627163242.39b0a716bd950a895c032136@linux-foundation.org>
Date: Thu, 27 Jun 2024 16:32:42 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Peter Xu <peterx@...hat.com>
Cc: Yang Shi <yang@...amperecomputing.com>, yangge1116@....com,
david@...hat.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [v2 PATCH] mm: gup: do not call try_grab_folio() in slow path
On Thu, 27 Jun 2024 19:19:40 -0400 Peter Xu <peterx@...hat.com> wrote:
> Yang,
>
> On Thu, Jun 27, 2024 at 03:14:13PM -0700, Yang Shi wrote:
> > The try_grab_folio() is supposed to be used in fast path and it elevates
> > folio refcount by using add ref unless zero. We are guaranteed to have
> > at least one stable reference in slow path, so the simple atomic add
> > could be used. The performance difference should be trivial, but the
> > misuse may be confusing and misleading.
>
> This first paragraph is IMHO misleading itself..
>
> I think we should mention upfront the important bit, on the user impact.
>
> Here IMO the user impact should be: Linux may fail longterm pin in some
> releavnt paths when applied over CMA reserved blocks. And if to extend a
> bit, that include not only slow-gup but also the new memfd pinning, because
> both of them used try_grab_folio() which used to be only for fast-gup.
It's still unclear how users will be affected. What do the *users*
see? If it's a slight slowdown, do we need to backport this at all?
>
> The patch itself looks mostly ok to me.
>
> There's still some "cleanup" part mangled together, e.g., the real meat
> should be avoiding the folio_is_longterm_pinnable() check in relevant
> paths. The rest (e.g. switch slow-gup / memfd pin to use folio_ref_add()
> not try_get_folio(), and renames) could be good cleanups.
>
> So a smaller fix might be doable, but again I don't have a strong opinion
> here.
The smaller the better for backporting, of course.
Powered by blists - more mailing lists