On Tue, Nov 03 2020 at 09:48, Linus Torvalds wrote:
> I have no complaints about the patch, but it strikes me that if people
> want to actually have much better debug coverage, this is where it
> should be (I like the "every other address" thing too, don't get me
> wrong).
>
> In particular, instead of these PageHighMem(page) tests, I think
> something like this would be better:
>
> #ifdef CONFIG_DEBUG_HIGHMEM
> #define page_use_kmap(page) ((page),1)
> #else
> #define page_use_kmap(page) PageHighMem(page)
> #endif
>
> adn then replace those "if (!PageHighMem(page))" tests with "if
> (!page_use_kmap())" instead.
>
> IOW, in debug mode, it would _always_ remap the page, whether it's
> highmem or not. That would really stress the highmem code and find any
> fragilities.
Yes, that makes a lot of sense. We just have to avoid that for the
architectures with aliasing issues.
> Anyway, this is all sepatrate from the series, which still looks fine
> to me. Just a reaction to seeing the patch, and Thomas' earlier
> mention that the highmem debugging doesn't actually do much.
Right, forcing it for both kmap and kmap_local is straight forward. I'll
cook a patch on top for that.
Thanks,
tglx
