On Wed, May 1, 2019 at 4:24 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > Hi Pavel, > > This strikes me as wrong: > > #define PF_HEAD(page, enforce) PF_POISONED_CHECK(compound_head(page)) > > If we hit a page which is poisoned, PAGE_POISON_PATTERN is ~0, so PageTail > is set, and compound_head will return() 0xfff..ffe. PagePoisoned() > will then try to derefence that pointer and we'll get an oops that isn't > obviously PagePoisoned. > > I think this should have been: > > #define PF_HEAD(page, enforce) compound_head(PF_POISONED_CHECK(page)) Yes, I agree, this makes sense. > > One could make the argument for double-checking: > > #define PF_HEAD(page, enforce) PF_POISONED_CHECK(compound_head(PF_POISONED_CHECK(page))) > > but I think this is overkill; if a tail page is initialised, then there's > no way that its head page should have been uninitialised. Also agree, no need to check head if subpage is initialized. > > Would a patch something along these lines make sense? Compile-tested only. Yes, I like the re-ordering PF_POISONED_CHECK()s to be before the other accesses to PPs. Thank you, Pasha
