On Mon, Apr 19, 2021 at 02:56:17PM +0300, Mike Rapoport wrote:
On Mon, Apr 19, 2021 at 12:23:02PM +0100, Matthew Wilcox wrote:
So you're calling page_is_secretmem() on a struct page without having a refcount on it. That is definitely not allowed. secretmem seems to be full of these kinds of races; I know this isn't the first one I've seen in it. I don't think this patchset is ready for this merge window.
There were races in the older version that did caching of large pages and those were fixed then, but this is anyway irrelevant because all that code was dropped in the latest respins.
I don't think that the fix of the race in gup_pte_range is that significant to wait 3 more months because of it.
I have no particular interest in secretmem, but it seems that every time I come across it while looking at something else, I see these kinds of major mistakes in it. That says to me it's not ready and hasn't seen enough review.
With that fixed, you'll have a head page that you can use for testing, which means you don't need to test PageCompound() (because you know the page isn't PageTail), you can just test PageHead().
I can't say I follow you here. page_is_secretmem() is intended to be a generic test, so I don't see how it will get PageHead() if it is called from other places.
static inline bool head_is_secretmem(struct page *head) { if (PageHead(head)) return false; ... }
static inline bool page_is_secretmem(struct page *page) { if (PageTail(page)) return false; return head_is_secretmem(page); }
(yes, calling it head is a misnomer, because it's not necessarily a head, it might be a base page, but until we have a name for pages which might be a head page or a base page, it'll have to do ...)