From: Matthew Wilcox <email@example.com> To: Qian Cai <firstname.lastname@example.org> Cc: Huang Ying <email@example.com>, firstname.lastname@example.org, "Kirill A. Shutemov" <email@example.com> Subject: Re: page cache: Store only head pages in i_pages Date: Sat, 30 Mar 2019 20:23:26 -0700 Message-ID: <20190331032326.GA10344@bombadil.infradead.org> (raw) In-Reply-To: <20190330141052.GZ10344@bombadil.infradead.org> On Sat, Mar 30, 2019 at 07:10:52AM -0700, Matthew Wilcox wrote: > On Fri, Mar 29, 2019 at 08:04:32PM -0700, Matthew Wilcox wrote: > > Excellent! I'm not comfortable with the rule that you have to be holding > > the i_pages lock in order to call find_get_page() on a swap address_space. > > How does this look to the various smart people who know far more about the > > MM than I do? > > > > The idea is to ensure that if this race does happen, the page will be > > handled the same way as a pagecache page. If __delete_from_swap_cache() > > can be called while the page is still part of a VMA, then this patch > > will break page_to_pgoff(). But I don't think that can happen ... ? > > Oh, blah, that can totally happen. reuse_swap_page() calls > delete_from_swap_cache(). Need a new plan. I don't see a good solution here that doesn't involve withdrawing this patch and starting over. Bad solutions: - Take the i_pages lock around each page lookup call in the swap code (not just the one you found; there are others like mc_handle_swap_pte() in memcontrol.c) - Call synchronize_rcu() in __delete_from_swap_cache() - Swap the roles of ->index and ->private for swap pages, and then don't clear ->index when deleting a page from the swap cache The first two would be slow and non-scalable. The third is still prone to a race where the page is looked up on one CPU, while another CPU removes it from one swap file then moves it to a different location, potentially in a different swap file. Hard to hit, but not a race we want to introduce. I believe that the swap code actually never wants to see subpages. So if we start again, introducing APIs (eg find_get_head()) which return the head page, then convert the swap code over to use those APIs, we don't need to solve the problem of finding the subpage of a swap page while not holding the page lock. I'm obviously reluctant to withdraw the patch, but I don't see a better option. Your testing has revealed a problem that needs a deeper solution than just adding a fix patch.
next prev parent reply index Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <firstname.lastname@example.org> 2019-03-23 3:38 ` 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 2019-03-30 3:04 ` Matthew Wilcox 2019-03-30 14:10 ` Matthew Wilcox 2019-03-31 3:23 ` Matthew Wilcox [this message] 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=20190331032326.GA10344@bombadil.infradead.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.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
Linux-mm Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \ firstname.lastname@example.org public-inbox-index linux-mm Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kvack.linux-mm AGPL code for this site: git clone https://public-inbox.org/public-inbox.git