Re: [PATCH] Simple fix

[Huacai Chen <chenhuacai@kernel.org>]

Hi, Lorenzo,

On Tue, Jan 7, 2025 at 4:48 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Tue, Jan 07, 2025 at 09:18:48AM +0800, Huacai Chen wrote:
> > Hi, all, I like simple fixes, so is this acceptable (based on an early
> > version from Koichiro Den)?
>
> No not at all. This is bizarre - in the mail you are replying to Koichiro
> agrees with me that the approach of his code that you've sent here (I don't
> know why) is fundamentally broken and suggest another.
>
> I am at a loss as to why you've sent this? Perhaps a miscommunication
> somewhere? :)
>
> In any case, please don't send '[PATCH] xxx' mails that aren't intended to
> be patches, a better form of this would be to say 'oh can we just do...'
> then to put this code in the mail underneath, without any '[PATCH]' prefix.
I wasn't in the CC list, and I also found the bug yesterday, so I can
only reply to this email with "git send-email --in-reply-to". This is
the reason why my reply looks so stranne.

>
> But please do review the discussion - the below is insufficient as simple
> as it is (sadly) because the boot CPU's delayed work will never be
> executed.
Koichiro's simple fix causes the boot CPU's delayed work to never be
executed, this is obvious, and of course I have read the early
discussion. And so I improve it, with a "cpu_online()" checking, then
the boot CPU is unaffected.

Huacai

>
> I will take a look at Koichiro's new approach as soon as I am able.
>
> Cheers!
>
> > ---
> >  mm/vmstat.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index 0889b75cef14..1badc24a21ff 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -2122,10 +2122,15 @@ static void __init start_shepherd_timer(void)
> >  {
> >       int cpu;
> >
> > -     for_each_possible_cpu(cpu)
> > +     for_each_possible_cpu(cpu) {
> >               INIT_DEFERRABLE_WORK(per_cpu_ptr(&vmstat_work, cpu),
> >                       vmstat_update);
> >
> > +             /* Will be enabled on vmstat_cpu_online() */
> > +             if (!cpu_online(cpu))
> > +                     disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> > +     }
> > +
> >       schedule_delayed_work(&shepherd,
> >               round_jiffies_relative(sysctl_stat_interval));
> >  }
> > --
> > 2.43.5
> >