Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 13 Jan 2021 12:55:32 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH 0/1] mm: restore full accuracy in COW page reuse |
| |
On Wed, Jan 13, 2021 at 03:32:32PM +0300, Kirill A. Shutemov wrote: > On Tue, Jan 12, 2021 at 07:31:07PM -0800, Linus Torvalds wrote: > > On Tue, Jan 12, 2021 at 6:16 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > The thing about the speculative page cache references is that they can > > > temporarily bump a refcount on a page which _used_ to be in the page > > > cache and has now been reallocated as some other kind of page. > > > > Oh, and thinking about this made me think we might actually have a > > serious bug here, and it has nothing what-so-ever to do with COW, GUP, > > or even the page count itself. > > > > It's unlikely enough that I think it's mostly theoretical, but tell me > > I'm wrong. > > > > PLEASE tell me I'm wrong: > > > > CPU1 does page_cache_get_speculative under RCU lock > > > > CPU2 frees and re-uses the page > > > > CPU1 CPU2 > > ---- ---- > > > > page = xas_load(&xas); > > if (!page_cache_get_speculative(page)) > > goto repeat; > > .. succeeds .. > > > > remove page from XA > > release page > > reuse for something else > > How can it be reused if CPU1 hold reference to it?
Yes, Linus mis-stated it:
page = xas_load(&xas); remove page from XA release page reuse for something else if (!page_cache_get_speculative(page)) goto repeat; if (unlikely(page != xas_reload(&xas))) { put_page(page);
... but as David pointed out, I fixed this in e320d3012d25
| ![\](/images/icornerr.gif) |