Re: [PATCH 2/3] mm/vmstat: get fragmentation statistics from per-migragetype count

[Barry Song]

> >       if (order >= pageblock_order && !is_migrate_isolate(migratetype))
> >               __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, -nr_pages);
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index bb09c032eecf..9334bbbe1e16 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -1590,32 +1590,16 @@ static void pagetypeinfo_showfree_print(struct seq_file *m,
> >                                       zone->name,
> >                                       migratetype_names[mtype]);
> >               for (order = 0; order < NR_PAGE_ORDERS; ++order) {
> > -                     unsigned long freecount = 0;
> > -                     struct free_area *area;
> > -                     struct list_head *curr;
> > +                     unsigned long freecount;
> >                       bool overflow = false;
> >
> > -                     area = &(zone->free_area[order]);
> > -
> > -                     list_for_each(curr, &area->free_list[mtype]) {
> > -                             /*
> > -                              * Cap the free_list iteration because it might
> > -                              * be really large and we are under a spinlock
> > -                              * so a long time spent here could trigger a
> > -                              * hard lockup detector. Anyway this is a
> > -                              * debugging tool so knowing there is a handful
> > -                              * of pages of this order should be more than
> > -                              * sufficient.
> > -                              */
> > -                             if (++freecount >= 100000) {
> > -                                     overflow = true;
> > -                                     break;
> > -                             }
> > +                     /* Keep the same output format for user-space tools compatibility */
> > +                     freecount = READ_ONCE(zone->free_area[order].mt_nr_free[mtype]);
>
> I think it might be better for using an array of size NR_PAGE_ORDERS to store
> the free count for each order. Like the code below.

Right. If we want the freecount to accurately reflect the current system
state, we still need to take the zone lock.

Multiple independent WRITE_ONCE and READ_ONCE operations do not guarantee
correctness. They may ensure single-copy atomicity per access, but not for the
overall result.

Thanks
Barry