On Mon 17-12-18 06:41:01, Matthew Wilcox wrote: > On Mon, Dec 17, 2018 at 03:10:44PM +0100, Michal Hocko wrote: > > On Mon 17-12-18 04:25:46, Matthew Wilcox wrote: > > > It's worth noticing that squashfs _is_ in fact holding a page locked in > > > squashfs_copy_cache() when it calls grab_cache_page_nowait(). I'm not > > > sure if this will lead to trouble or not because I'm insufficiently > > > familiar with the reclaim path. > > > > Hmm, this is more interesting then. If there is any memcg accounted > > allocation down that path _and_ the squashfs writeout can lock more > > pages and mark them writeback before they are really sent to the storage > > then we have a problem. See [1] > > > > [1] http://lkml.kernel.org/r/20181213092221.27270-1-mhocko@xxxxxxxxxx > > Squashfs is read only, so it'll never have dirty pages and never do > writeout. > > But ... maybe the GFP flags being used for grab_cache_page_nowait() are > wrong. It does, after all, say "nowait". Perhaps it shouldn't be trying > direct reclaim at all, but rather fail earlier. Like this: > > +++ b/mm/filemap.c > @@ -1550,6 +1550,8 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t offset, > gfp_mask |= __GFP_WRITE; > if (fgp_flags & FGP_NOFS) > gfp_mask &= ~__GFP_FS; > + if (fgp_flags & FGP_NOWAIT) > + gfp_mask &= ~__GFP_DIRECT_RECLAIM; > > page = __page_cache_alloc(gfp_mask); > if (!page) Isn't FGP_NOWAIT about page lock rather than the allocation context? -- Michal Hocko SUSE Labs
