Hi, Lorenzo,
On Tue, Jan 7, 2025 at 4:48 PM Lorenzo Stoakes
<lorenzo.stoakes@xxxxxxxxxx> 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
> >
