| Message ID | 20200408150148.25290-1-willy@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Improve page poisoning implementation | expand |
On Wed, Apr 8, 2020 at 11:01 AM Matthew Wilcox <willy@infradead.org> wrote: > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > I really don't like that this feature is called page poisoning. > We already had something called page poisoning and it's when you detect > a memory error in a page. This is just uninitialised pages. I don't Hi Matthew, Thank you for working on this. Uninitialized struct pages are often zeroed by firmware, and there were a number of implicit assumptions about that memory when I worked on deferred page initializations, this is why it is important to also test when struct pages are specifically set to a pattern that is not all zeroes, something that can happen during kexec, or when memory allocated and freed by kernel during boot. We have caught a good number of bugs using this mechanism. So, this is poisoning, but I agree "page poisoning" name is misleading, as we have this term used in another place. So, lets agree on a better term: how about memmap poisoning (s/page_poisoning/memmap_poisoning/)? Pasha
On 4/8/20 8:11 AM, Pavel Tatashin wrote: > On Wed, Apr 8, 2020 at 11:01 AM Matthew Wilcox <willy@infradead.org> wrote: >> >> From: "Matthew Wilcox (Oracle)" <willy@infradead.org> >> >> I really don't like that this feature is called page poisoning. >> We already had something called page poisoning and it's when you detect >> a memory error in a page. This is just uninitialised pages. I don't > > Hi Matthew, > > Thank you for working on this. Uninitialized struct pages are often > zeroed by firmware, and there were a number of implicit assumptions > about that memory when I worked on deferred page initializations, this > is why it is important to also test when struct pages are specifically > set to a pattern that is not all zeroes, something that can happen > during kexec, or when memory allocated and freed by kernel during > boot. We have caught a good number of bugs using this mechanism. So, > this is poisoning, but I agree "page poisoning" name is misleading, as > we have this term used in another place. So, lets agree on a better > term: how about memmap poisoning (s/page_poisoning/memmap_poisoning/)? > Better to avoid the "poison" word entirely. It's just too well-established as a hardware memory error case. Early ideas for other names, just to get started: use "uninitialize" or "deinitialize" instead of "poison". So approximately: page_deinit page_deinitialize page_uninit page_uninitialize mem_deinit ...other variations... thanks,
From: "Matthew Wilcox (Oracle)" <willy@infradead.org> I really don't like that this feature is called page poisoning. We already had something called page poisoning and it's when you detect a memory error in a page. This is just uninitialised pages. I don't have the time right now to do that renaming, so just fix some of the other problems I found with the implementation. This hasn't been checked out by the build-bots yet, so there may be some missing conversions. Matthew Wilcox (Oracle) (5): mm: Constify a lot of struct page arguments mm: Rename PF_POISONED_PAGE to page_poison_check mm: Remove casting away of constness mm: Check for page poison in both page_to_nid implementations mm: Check page poison before finding a head page include/linux/mm.h | 32 +++++++------- include/linux/mm_types.h | 11 +---- include/linux/mmdebug.h | 4 +- include/linux/page-flags.h | 74 ++++++++++++++++----------------- include/linux/page_owner.h | 6 +-- include/linux/page_ref.h | 4 +- include/linux/pageblock-flags.h | 2 +- include/linux/pagemap.h | 4 +- include/linux/swap.h | 2 +- mm/debug.c | 6 +-- mm/hugetlb.c | 6 +-- mm/page_alloc.c | 12 +++--- mm/page_owner.c | 2 +- mm/sparse.c | 1 + mm/swapfile.c | 6 +-- mm/util.c | 6 +-- 16 files changed, 85 insertions(+), 93 deletions(-)