Message ID | 20221201135045.31663-1-wonder_rock@126.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [1/1] mm/page_alloc: update comments in __free_pages_ok() | expand |
On Thu, 1 Dec 2022 21:50:45 +0800 wonder_rock@126.com wrote: > Add a comment to explain why we call get_pfnblock_migratetype() twice > in __free_pages_ok(). > > ... > > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1702,6 +1702,11 @@ static void __free_pages_ok(struct page *page, unsigned int order, > if (!free_pages_prepare(page, order, true, fpi_flags)) > return; > > + /* > + * Calling get_pfnblock_migratetype() without spin_lock_irqsave() here > + * is used to avoid calling get_pfnblock_migratetype() under the lock. > + * This will reduce the lock holding time. > + */ > migratetype = get_pfnblock_migratetype(page, pfn); > > spin_lock_irqsave(&zone->lock, flags); I guess that if the comment helped one reader, it will help others. But this is a pretty common trick in MM and most readers will recognize it. That being said, get_pfnblock_migratetype() is pretty lightweight. Particularly when compared with __free_one_page(). I wonder if the optimization does much good. If the second call to get_pfnblock_migratetype() is almost never performed then we just added a bunch of testing and branching inside the lock which actually made things worse!
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2d4c81224508..52dd4fff280b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1702,6 +1702,11 @@ static void __free_pages_ok(struct page *page, unsigned int order, if (!free_pages_prepare(page, order, true, fpi_flags)) return; + /* + * Calling get_pfnblock_migratetype() without spin_lock_irqsave() here + * is used to avoid calling get_pfnblock_migratetype() under the lock. + * This will reduce the lock holding time. + */ migratetype = get_pfnblock_migratetype(page, pfn); spin_lock_irqsave(&zone->lock, flags);