On Thu, 27 Jun 2024 19:19:40 -0400 Peter Xu <peterx@xxxxxxxxxx> 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.
