From: Peter Xu <peterx@redhat.com>
To: John Hubbard <jhubbard@nvidia.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Matthew Wilcox <willy@infradead.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Hugh Dickins <hughd@google.com>, Jan Kara <jack@suse.cz>,
Kirill Shutemov <kirill@shutemov.name>,
Jason Gunthorpe <jgg@nvidia.com>,
Andrew Morton <akpm@linux-foundation.org>,
Kirill Tkhai <ktkhai@virtuozzo.com>,
Michal Hocko <mhocko@suse.com>, Oleg Nesterov <oleg@redhat.com>,
Jann Horn <jannh@google.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Matthew Wilcox <willy@infradead.org>,
Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: [PATCH 3/3] mm: gup: pack has_pinned in MMF_HAS_PINNED
Date: Fri, 7 May 2021 10:34:56 -0400 [thread overview]
Message-ID: <YJVQEGSLil7wYhEe@t490s> (raw)
In-Reply-To: <69055843-185d-20ea-213b-10494a2f7246@nvidia.com>
On Thu, May 06, 2021 at 11:42:59PM -0700, John Hubbard wrote:
> > +#define MMF_HAS_PINNED 28 /* FOLL_PIN has run, never cleared */
>
> How about this instead, so that we effectively retain the comment block
> that is otherwise being deleted from mm.h:
>
> /*
> * MMF_HAS_PINNED: Whether this mm has pinned any pages. This can be either
> * replaced in the future by mm.pinned_vm when it becomes stable, or grow into a
> * counter on its own. We're aggresive on this bit for now: even if the pinned
> * pages were unpinned later on, we'll still keep this bit set for the lifecycle
> * of this mm, just for simplicity.
> */
> #define MMF_HAS_PINNED 28 /* FOLL_PIN ran. Never cleared. */
Sure, good to know the comment is still valid!
> > @@ -1292,8 +1292,8 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm,
> > BUG_ON(*locked != 1);
> > }
> > - if (flags & FOLL_PIN && !atomic_read(&mm->has_pinned))
> > - atomic_set(&mm->has_pinned, 1);
> > + if (flags & FOLL_PIN && !test_bit(MMF_HAS_PINNED, &mm->flags))
> > + set_bit(MMF_HAS_PINNED, &mm->flags);
>
> I expect this suggestion to be controversial, but I'm going to float it
> anyway. The above is a little less clear than it used to be, *and* it is
> in two places so far, so how about factoring out a tiny subroutine, like this:
Definitely less "controversial" than expected, isn't it? ;)
Thanks for the suggestion, it looks much better indeed. Also I'll rename the
helper to mm_set_has_pinned_flag() as suggested by Matthew.
--
Peter Xu
prev parent reply other threads:[~2021-05-07 14:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-06 23:25 [PATCH 0/3] mm/gup: Fix pin page write cache bouncing on has_pinned Peter Xu
2021-05-06 23:25 ` [PATCH 1/3] mm/gup_benchmark: Support threading Peter Xu
2021-05-07 4:37 ` John Hubbard
2021-05-07 14:04 ` Peter Xu
2021-05-06 23:25 ` [PATCH 2/3] mm: gup: allow FOLL_PIN to scale in SMP Peter Xu
2021-05-07 2:34 ` Linus Torvalds
2021-05-07 6:07 ` John Hubbard
2021-05-07 14:13 ` Peter Xu
2021-05-06 23:25 ` [PATCH 3/3] mm: gup: pack has_pinned in MMF_HAS_PINNED Peter Xu
2021-05-07 6:42 ` John Hubbard
2021-05-07 7:39 ` Linus Torvalds
2021-05-07 11:14 ` Matthew Wilcox
2021-05-07 14:34 ` Peter Xu [this message]
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=YJVQEGSLil7wYhEe@t490s \
--to=peterx@redhat.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=jack@suse.cz \
--cc=jannh@google.com \
--cc=jgg@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=kirill@shutemov.name \
--cc=ktkhai@virtuozzo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=oleg@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).