From: Gregory Price <gregory.price@memverge.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gregory Price <gourry.memverge@gmail.com>,
linux-mm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arch@vger.kernel.org, linux-api@vger.kernel.org,
linux-cxl@vger.kernel.org, luto@kernel.org, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, arnd@arndb.de, akpm@linux-foundation.org,
x86@kernel.org
Subject: Re: [RFC v2 4/5] mm/migrate: Create move_phys_pages syscall
Date: Tue, 3 Oct 2023 13:58:12 -0400 [thread overview]
Message-ID: <ZRxWNFBdCAe9KIwj@memverge.com> (raw)
In-Reply-To: <20231002150746.00007516@Huawei.com>
On Mon, Oct 02, 2023 at 03:07:46PM +0100, Jonathan Cameron wrote:
> On Tue, 19 Sep 2023 19:09:07 -0400
> Gregory Price <gourry.memverge@gmail.com> wrote:
>
> > @@ -2181,6 +2290,10 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
> > int start, i;
> > int err = 0, err1;
> >
> > + /* This should never occur in regular operation */
> > + if (!mm && nodes_weight(task_nodes) > 0)
>
> You document below that task_nodes isn't used if !mm but it is used
> to indicate this condition...
>
good point, and in fact the code below does ignore the contents of
nodes_weight if mm is null, so i can just drop this check.
> > +set_status:
> > + *status = err;
>
> Given you update it unconditionally this seems to wipe out earlier
> potential errors with the nid.
>
> > +
> > + pages++;
> > + status++;
> > + }
> > +}
>
status is a list, the status++ increments the list pointer.
> > + /*
> > + * When the mm argument to do_pages_move is null, the task_nodes
> > + * argument is ignored, so pass in an empty nodemask as a dummy.
>
> If ignored, why bother clearing it? Looks like you are using it as
> as signal rather than ignoring it. So not sure this comment is correct.
>
> > + */
> > + nodes_clear(dummy_nodes);
I'm a bit of a stickler around uninitialized memory, seemed better to
ensure the nodelist was empty to force errors associated with an empty
nodelist should future updates break do_pages_move to remove the
ignoring mechanism.
That said, I updated the function such that it does actually ignore it,
but I'll leave the here since there's no harm in clearing the nodemask
and leads to more predictable behavior.
Unless there is particularly strong feelings about this, then i'm fine
to remove it.
next prev parent reply other threads:[~2023-10-03 17:58 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-19 23:09 [RFC v2 0/5] move_phys_pages syscall Gregory Price
2023-09-19 23:09 ` [RFC v2 1/5] mm/migrate: fix do_pages_move for compat pointers Gregory Price
2023-09-20 9:36 ` Arnd Bergmann
2023-09-19 23:09 ` [RFC v2 2/5] mm/migrate: remove unused mm argument from do_move_pages_to_node Gregory Price
2023-10-02 13:44 ` Jonathan Cameron
2023-09-19 23:09 ` [RFC v2 3/5] mm/migrate: refactor add_page_for_migration for code re-use Gregory Price
2023-10-02 13:51 ` Jonathan Cameron
2023-09-19 23:09 ` [RFC v2 4/5] mm/migrate: Create move_phys_pages syscall Gregory Price
2023-10-02 14:07 ` Jonathan Cameron
2023-10-03 17:58 ` Gregory Price [this message]
2023-09-19 23:09 ` [RFC v2 5/5] ktest: sys_move_phys_pages ktest Gregory Price
2023-10-02 14:09 ` Jonathan Cameron
2023-09-19 23:09 ` [RFC] man/move_phys_pages: migrate pages based on physical address Gregory Price
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=ZRxWNFBdCAe9KIwj@memverge.com \
--to=gregory.price@memverge.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=gourry.memverge@gmail.com \
--cc=hpa@zytor.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.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 read-only IMAP folder(s) and NNTP newsgroup(s).