All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Anthony PERARD <anthony.perard@vates.tech>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 1/2] xen/mm: add a NUMA node parameter to scrub_free_pages()
Date: Fri, 9 Jan 2026 15:46:50 +0100	[thread overview]
Message-ID: <aWEU2jKO12e5TYtz@Mac.lan> (raw)
In-Reply-To: <a8d09b82-3013-4476-b358-08b5fdc14cf1@suse.com>

On Fri, Jan 09, 2026 at 11:22:39AM +0100, Jan Beulich wrote:
> On 08.01.2026 18:55, Roger Pau Monne wrote:
> > Such parameter allow requesting to scrub memory only from the specified
> > node.  If there's no memory to scrub from the requested node the function
> > returns false.  If the node is already being scrubbed from a different CPU
> > the function returns true so the caller can differentiate whether there's
> > still pending work to do.
> 
> I'm really trying to understand both patches together, and peeking ahead I
> don't understand the above, which looks to describe ...
> 
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1339,16 +1339,27 @@ static void cf_check scrub_continue(void *data)
> >      }
> >  }
> >  
> > -bool scrub_free_pages(void)
> > +bool scrub_free_pages(nodeid_t node)
> >  {
> >      struct page_info *pg;
> >      unsigned int zone;
> >      unsigned int cpu = smp_processor_id();
> >      bool preempt = false;
> > -    nodeid_t node;
> >      unsigned int cnt = 0;
> >  
> > -    node = node_to_scrub(true);
> > +    if ( node != NUMA_NO_NODE )
> > +    {
> > +        if ( !node_need_scrub[node] )
> > +            /* Nothing to scrub. */
> > +            return false;
> > +
> > +        if ( node_test_and_set(node, node_scrubbing) )
> > +            /* Another CPU is scrubbing it. */
> > +            return true;
> 
> ... these two return-s. My problem being that patch 2 doesn't use the
> return value (while existing callers don't take this path). Is this then
> "just in case" for now (and making the meaning of the return values
> somewhat inconsistent for the function as a whole)?

I've added those so that the function return values are consistent,
even if not consumed right now, it would make no sense for the return
values to have different meaning when the node parameter is !=
NUMA_NO_NODE.  Or at least that was my impression.

In fact an earlier version of patch 2 did consume those values.  I've
moved to a different approach, but I think it's good to keep the
return values consistent regardless of the input parameters.

Thanks, Roger.


  reply	other threads:[~2026-01-09 14:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-08 17:55 [PATCH 0/2] xen/mm: limit in-place scrubbing Roger Pau Monne
2026-01-08 17:55 ` [PATCH 1/2] xen/mm: add a NUMA node parameter to scrub_free_pages() Roger Pau Monne
2026-01-09 10:22   ` Jan Beulich
2026-01-09 14:46     ` Roger Pau Monné [this message]
2026-01-09 14:50       ` Jan Beulich
2026-01-08 17:55 ` [PATCH 2/2] xen/mm: limit non-scrubbed allocations to a specific order Roger Pau Monne
2026-01-09 11:19   ` Jan Beulich
2026-01-13 14:01     ` Roger Pau Monné
2026-01-14  8:48       ` Jan Beulich
2026-01-15 10:48         ` Roger Pau Monné
2026-01-15 10:56           ` Jan Beulich
2026-01-15 13:05             ` Roger Pau Monné
2026-01-09 10:15 ` [PATCH 0/2] xen/mm: limit in-place scrubbing Jan Beulich
2026-01-09 10:29   ` Andrew Cooper
2026-01-09 11:32     ` Jan Beulich
2026-01-09 11:34       ` Andrew Cooper
2026-01-09 12:31     ` Roger Pau Monné

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=aWEU2jKO12e5TYtz@Mac.lan \
    --to=roger.pau@citrix.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=bertrand.marquis@arm.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.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 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.