From: Qian Cai <cai@lca.pw>
To: Matthew Wilcox <willy@infradead.org>
Cc: Huang Ying <ying.huang@intel.com>,
linux-mm@kvack.org, "Kirill A. Shutemov" <kirill@shutemov.name>
Subject: Re: page cache: Store only head pages in i_pages
Date: Fri, 29 Mar 2019 17:25:34 -0400 [thread overview]
Message-ID: <1553894734.26196.30.camel@lca.pw> (raw)
In-Reply-To: <20190329195941.GW10344@bombadil.infradead.org>
On Fri, 2019-03-29 at 12:59 -0700, Matthew Wilcox wrote:
> I don't understand how we get to this situation. We SetPageSwapCache()
> in add_to_swap_cache() right before we store the page in i_pages.
> We ClearPageSwapCache() in __delete_from_swap_cache() right after
> removing the page from the array. So how do we find a page in a swap
> address space that has PageSwapCache cleared?
>
> Indeed, we have a check which should trigger ...
>
> VM_BUG_ON_PAGE(!PageSwapCache(page), page);
>
> in __delete_from_swap_cache().
>
> Oh ... is it a race?
>
> * Its ok to check for PageSwapCache without the page lock
> * here because we are going to recheck again inside
> * try_to_free_swap() _with_ the lock.
>
> so CPU A does:
>
> page = find_get_page(swap_address_space(entry), offset)
> page = find_subpage(page, offset);
> trylock_page(page);
>
> while CPU B does:
>
> xa_lock_irq(&address_space->i_pages);
> __delete_from_swap_cache(page, entry);
> xas_store(&xas, NULL);
> ClearPageSwapCache(page);
> xa_unlock_irq(&address_space->i_pages);
>
> and if the ClearPageSwapCache happens between the xas_load() and the
> find_subpage(), we're stuffed. CPU A has a reference to the page, but
> not a lock, and find_get_page is running under RCU.
>
> I suppose we could fix this by taking the i_pages xa_lock around the
> call to find_get_pages(). If indeed, that's what this problem is.
> Want to try this patch?
Confirmed. Well spotted!
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 2b8d9c3fbb47..ed8e42be88b5 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -127,10 +127,14 @@ static int __try_to_reclaim_swap(struct swap_info_struct
> *si,
> unsigned long offset, unsigned long flags)
> {
> swp_entry_t entry = swp_entry(si->type, offset);
> + struct address_space *mapping = swap_address_space(entry);
> + unsigned long irq_flags;
> struct page *page;
> int ret = 0;
>
> - page = find_get_page(swap_address_space(entry), offset);
> + xa_lock_irqsave(&mapping->i_pages, irq_flags);
> + page = find_get_page(mapping, offset);
> + xa_unlock_irqrestore(&mapping->i_pages, irq_flags);
> if (!page)
> return 0;
> /*
next prev parent reply other threads:[~2019-03-29 21:25 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1553285568.26196.24.camel@lca.pw>
2019-03-23 3:38 ` page cache: Store only head pages in i_pages Matthew Wilcox
2019-03-23 23:50 ` Qian Cai
2019-03-24 2:06 ` Matthew Wilcox
2019-03-24 2:52 ` Qian Cai
2019-03-24 3:04 ` Matthew Wilcox
2019-03-24 15:42 ` Qian Cai
2019-03-27 10:48 ` William Kucharski
2019-03-27 11:50 ` Matthew Wilcox
2019-03-29 1:43 ` Qian Cai
2019-03-29 19:59 ` Matthew Wilcox
2019-03-29 21:25 ` Qian Cai [this message]
2019-03-30 3:04 ` Matthew Wilcox
2019-03-30 14:10 ` Matthew Wilcox
2019-03-31 3:23 ` Matthew Wilcox
2019-04-01 9:18 ` Kirill A. Shutemov
2019-04-01 9:27 ` Kirill A. Shutemov
2019-04-04 13:10 ` Qian Cai
2019-04-04 13:45 ` Kirill A. Shutemov
2019-04-04 21:28 ` Qian Cai
2019-04-05 13:37 ` Kirill A. Shutemov
2019-04-05 13:51 ` Matthew Wilcox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1553894734.26196.30.camel@lca.pw \
--to=cai@lca.pw \
--cc=kirill@shutemov.name \
--cc=linux-mm@kvack.org \
--cc=willy@infradead.org \
--cc=ying.huang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).