Re: [PATCH RESEND v2 3/5] mm_zone: add function to check if managed dma zone exists

Baoquan He
2021-12-09T13:23:39Z

On 12/09/21 at 02:10pm, David Hildenbrand wrote:
......
> >>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >>> index c5952749ad40..ac0ea42a4e5f 100644
> >>> --- a/mm/page_alloc.c
> >>> +++ b/mm/page_alloc.c
> >>> @@ -9459,4 +9459,15 @@ bool take_page_off_buddy(struct page *page)
> >>> spin_unlock_irqrestore(&zone->lock, flags);
> >>> return ret;
> >>> }
> >>> +
> >>> +bool has_managed_dma(void)
> >>> +{
> >>> + struct zone *zone;
> >>> +
> >>> + for_each_managed_zone(zone) {
> >>> + if (zone_is_dma(zone))
> >>> + return true;
> >>> + }
> >>> + return false;
> >>> +}
> >>
> >> Wouldn't it be "easier/faster" to just iterate online nodes and directly
> >> obtain the ZONE_DMA, checking if there are managed pages?
> >
> > Thanks, Dave.
> >
> > Please check for_each_managed_zone(), it is iterating online nodes and
> > it's each managed zone.
> >
> > Is below what you are suggesting? The only difference is I introduced
> > for_each_managed_zone() which can be reused later if needed. Not sure if
> > I got your suggestion correctly.
> >
> > bool has_managed_dma(void)
> > {
> > struct pglist_data *pgdat;
> > struct zone *zone;
> > enum zone_type i, j;
> >
> > for_each_online_pgdat(pgdat) {
> > for (i = 0; i < MAX_NR_ZONES - 1; i++) {
> > struct zone *zone = &pgdat->node_zones[i];
> > if (zone_is_dma(zone))
> > return true;
> > }
> > }
> > return false;
> >
> > }
>
>
> Even simpler, no need to iterate over zones at all, only over nodes:
>
> #ifdef CONFIG_ZONE_DMA
> bool has_managed_dma(void)
> {
> struct pglist_data *pgdat;
>
> for_each_online_pgdat(pgdat) {
> struct zone *zone = &pgdat->node_zones[ZONE_DMA];
>
> if (managed_zone(zone)
> return true;
> }
> return false;
> }
> #endif /* CONFIG_ZONE_DMA */
>
> Without CONFIG_ZONE_DMA, simply provide a dummy in the header that
> returns false.

Yeah, it only iterates the number of nodes times. I will take this in
v3. Thanks, David.