On 2022/6/14 21:13, Matthew Wilcox wrote: > On Wed, Jun 08, 2022 at 08:17:35PM +0800, Miaohe Lin wrote: >> +++ b/mm/page_alloc.c >> @@ -6771,13 +6771,18 @@ static void __ref memmap_init_compound(struct page *head, >> set_page_count(page, 0); >> >> /* >> - * The first tail page stores compound_mapcount_ptr() and >> - * compound_order() and the second tail page stores >> - * compound_pincount_ptr(). Call prep_compound_head() after >> - * the first and second tail pages have been initialized to >> - * not have the data overwritten. >> + * The first tail page stores compound_mapcount_ptr(), >> + * compound_order() and compound_pincount_ptr(). Call >> + * prep_compound_head() after the first tail page have >> + * been initialized to not have the data overwritten. >> + * >> + * Note the idea to make this right after we initialize >> + * the offending tail pages is trying to take advantage >> + * of the likelihood of those tail struct pages being >> + * cached given that we will read them right after in >> + * prep_compound_head(). > > It's not that we'll read them again, it's that the cacheline will still > be in cache, and therefore dirty. Thanks for pointing this out. > > Honestly, I don't think we need this extra explanation in a comment. > Just change the first paragraph to reflect reality and leave it at that. Will do it in next version if prep_compound_head is not moved outside loop. > >> */ >> - if (pfn == head_pfn + 2) >> + if (unlikely(pfn == head_pfn + 1)) > > We definitely don't need the unlikely here. Could you please give me a more detailed explanation? IIUC, the above if condition will only meet at a probability of 1/512. So unlikely tells the compiler to do some optimization around it. Or am I miss something? Thanks! > >> prep_compound_head(head, order); >> } >> } >> >> Or am I miss something? >> >> Thanks! >> >>> . >>> >> > > . >
