On Tue, May 13, 2025 at 8:52 AM Andrew Morton <[email protected]> wrote:
>
> On Mon, 12 May 2025 10:49:35 +0800 Jason Xing <[email protected]>
> wrote:
>
> > From: Jason Xing <[email protected]>
> >
> > Use for_each_possible_cpu to create per-cpu relayfs file to avoid later
> > hotplug cpu which doesn't have its own file.
>
> I don't understand this. Exactly what problem are we trying to solve?
>
> > Reviewed-by: Yushan Zhou <[email protected]>
> > Signed-off-by: Jason Xing <[email protected]>
> > ---
> > kernel/relay.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/relay.c b/kernel/relay.c
> > index 27f7e701724f..dcb099859e83 100644
> > --- a/kernel/relay.c
> > +++ b/kernel/relay.c
> > @@ -519,7 +519,7 @@ struct rchan *relay_open(const char *base_filename,
> > kref_init(&chan->kref);
> >
> > mutex_lock(&relay_channels_mutex);
> > - for_each_online_cpu(i) {
> > + for_each_possible_cpu(i) {
>
> num_possible_cpus() can sometimes greatly exceed num_online_cpus(), so
> this is an unfortunate change. It would be better to implement the
> hotplug notifier?
For the record, the hotplug notifier has been retired by the commit
530e9b76ae8f("cpu/hotplug: Remove obsolete cpu hotplug
register/unregister functions"). And now in relay, a similar feature
called hotplug state machine has already been implemented by the
commit e6d4989a9ad1 ("relayfs: Convert to hotplug state machine"). So
the relay has hotplug support.
Sorry for missing this point. I would drop this patch.
Thanks,
Jason
>
> > buf = relay_open_buf(chan, i);
> > if (!buf)
> > goto free_bufs;
> > @@ -615,7 +615,7 @@ int relay_late_setup_files(struct rchan *chan,
> > * no files associated. So it's safe to call relay_setup_buf_file()
> > * on all currently online CPUs.
> > */
> > - for_each_online_cpu(i) {
> > + for_each_possible_cpu(i) {
> > buf = *per_cpu_ptr(chan->buf, i);
> > if (unlikely(!buf)) {
> > WARN_ONCE(1, KERN_ERR "CPU has no buffer!\n");
> > --
> > 2.43.5
