On 2/27/24 17:38, Suren Baghdasaryan wrote: > On Tue, Feb 27, 2024 at 2:10 AM Vlastimil Babka <vbabka@xxxxxxx> wrote: >> >> On 2/21/24 20:40, Suren Baghdasaryan wrote: >> > When a high-order page is split into smaller ones, each newly split >> > page should get its codetag. The original codetag is reused for these >> > pages but it's recorded as 0-byte allocation because original codetag >> > already accounts for the original high-order allocated page. >> >> This was v3 but then you refactored (for the better) so the commit log >> could reflect it? > > Yes, technically mechnism didn't change but I should word it better. > Smth like this: > > When a high-order page is split into smaller ones, each newly split > page should get its codetag. After the split each split page will be > referencing the original codetag. The codetag's "bytes" counter > remains the same because the amount of allocated memory has not > changed, however the "calls" counter gets increased to keep the > counter correct when these individual pages get freed. Great, thanks. The concern with __free_pages() is not really related to splitting, so for this patch: Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> > >> >> > Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx> >> >> I was going to R-b, but now I recalled the trickiness of >> __free_pages() for non-compound pages if it loses the race to a >> speculative reference. Will the codetag handling work fine there? > > I think so. Each non-compoud page has its individual reference to its > codetag and will decrement it whenever the page is freed. IIUC the > logic in __free_pages(), when it loses race to a speculative > reference it will free all pages except for the first one and the The "tail" pages of this non-compound high-order page will AFAICS not have code tags assigned, so alloc_tag_sub() will be a no-op (or a warning with _DEBUG). > first one will be freed when the last put_page() happens. If prior to > this all these pages were split from one page then all of them will > have their own reference which points to the same codetag. Yeah I'm assuming there's no split before the freeing. This patch about splitting just reminded me of that tricky freeing scenario. So IIUC the "else if (!head)" path of __free_pages() will do nothing about the "tail" pages wrt code tags as there are no code tags. Then whoever took the speculative "head" page reference will put_page() and free it, which will end up in alloc_tag_sub(). This will decrement calls properly, but bytes will become imbalanced, because that put_page() will pass order-0 worth of bytes - the original order is lost. Now this might be rare enough that it's not worth fixing if that would be too complicated, just FYI. > Every time > one of these pages are freed that codetag's "bytes" and "calls" > counters will be decremented. I think accounting will work correctly > irrespective of where these pages are freed, in __free_pages() or by > put_page(). >
