All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: David Hildenbrand <david@redhat.com>
Cc: Donet Tom <donettom@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oscar Salvador <osalvador@suse.de>,
	Ritesh Harjani <ritesh.list@gmail.com>,
	Aboorva Devarajan <aboorvad@linux.ibm.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Danilo Krummrich <dakr@kernel.org>,
	Dave Jiang <dave.jiang@intel.com>
Subject: Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.
Date: Thu, 25 Sep 2025 12:34:20 +0300	[thread overview]
Message-ID: <aNUMnK23qKTjgEdO@kernel.org> (raw)
In-Reply-To: <0de65980-4333-434a-ae7d-2b7be46c2cca@redhat.com>

On Thu, Sep 25, 2025 at 10:54:07AM +0200, David Hildenbrand wrote:
> On 24.09.25 20:40, Donet Tom wrote:
> > register_one_node() and register_node() are small functions.
> > This patch merges them into a single function named register_node()
> > to improve code readability.
> > 
> > No functional changes are introduced.
> > 
> > Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> > ---
> 
> [...]
> 
> >   /**
> >    * unregister_node - unregister a node device
> >    * @node: node going away
> > @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
> >   }
> >   #endif /* CONFIG_MEMORY_HOTPLUG */
> > -int register_one_node(int nid)
> > +/*
> 
> We can directly convert this to proper kernel doc by using /**
> 
> > + * register_node - Setup a sysfs device for a node.
> > + * @nid - Node number to use when creating the device.
> > + *
> > + * Initialize and register the node device.
> 
> and briefly describing what the return value means
> 
> "Returns 0 on success, ..."

For kernel-doc it should be

Return: 0 on success, ...

> 
> > + */
> > +int register_node(int nid)
> >   {
> >   	int error;
> >   	int cpu;
> > @@ -880,14 +859,23 @@ int register_one_node(int nid)
> >   		return -ENOMEM;
> >   	INIT_LIST_HEAD(&node->access_list);
> > -	node_devices[nid] = node;
> > -	error = register_node(node_devices[nid], nid);
> > +	node->dev.id = nid;
> > +	node->dev.bus = &node_subsys;
> > +	node->dev.release = node_device_release;
> > +	node->dev.groups = node_dev_groups;
> > +
> > +	error = device_register(&node->dev);
> >   	if (error) {
> > -		node_devices[nid] = NULL;
> 
> Wondering why we did have this temporary setting of the node_devices[] in
> there. But I cannot immediately spot why it was required.

register_cpu_under_node() references node_devices, so that assignment can
be moved just before the loop that adds CPUs to node.
 
> -- 
> Cheers
> 
> David / dhildenb
> 

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2025-09-25  9:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-24 18:40 [PATCH 0/2] drivers/base/node: fold node register and unregister functions Donet Tom
2025-09-24 18:40 ` [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function Donet Tom
2025-09-24 19:15   ` Christophe Leroy
2025-09-25  5:01     ` Donet Tom
2025-09-25  8:54   ` David Hildenbrand
2025-09-25  9:34     ` Mike Rapoport [this message]
2025-09-25  9:37       ` David Hildenbrand
2025-09-25 13:21       ` Donet Tom
2025-09-25 13:20     ` Donet Tom
2025-09-24 18:40 ` [PATCH 2/2] drivers/base/node: merge unregister_one_node() and unregister_node() " Donet Tom
2025-09-24 19:19   ` Christophe Leroy
2025-09-25  5:03     ` Donet Tom
2025-09-25  8:55   ` David Hildenbrand
2025-09-25  9:36 ` [PATCH 0/2] drivers/base/node: fold node register and unregister functions Mike Rapoport

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=aNUMnK23qKTjgEdO@kernel.org \
    --to=rppt@kernel.org \
    --cc=aboorvad@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dakr@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=david@redhat.com \
    --cc=donettom@linux.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mingo@redhat.com \
    --cc=osalvador@suse.de \
    --cc=peterz@infradead.org \
    --cc=ritesh.list@gmail.com \
    --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 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.