Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 13 Feb 2020 18:08:24 +0100
From:   Michal Hocko <mhocko@kernel.org>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Cong Wang <xiyou.wangcong@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-mm <linux-mm@kvack.org>, Mel Gorman <mgorman@suse.de>,
        Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH] mm: avoid blocking lock_page() in kcompactd
Message-ID: <20200213170824.GJ31689@dhcp22.suse.cz>
References: <20200126233935.GA11536@bombadil.infradead.org>
 <20200127150024.GN1183@dhcp22.suse.cz>
 <20200127190653.GA8708@bombadil.infradead.org>
 <20200128081712.GA18145@dhcp22.suse.cz>
 <20200128083044.GB6615@bombadil.infradead.org>
 <20200128091352.GC18145@dhcp22.suse.cz>
 <20200128104857.GC6615@bombadil.infradead.org>
 <20200128113953.GA24244@dhcp22.suse.cz>
 <20200213074847.GB31689@dhcp22.suse.cz>
 <20200213164607.GR7778@bombadil.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200213164607.GR7778@bombadil.infradead.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu 13-02-20 08:46:07, Matthew Wilcox wrote:
> On Thu, Feb 13, 2020 at 08:48:47AM +0100, Michal Hocko wrote:
> > Can we pursue on this please? An explicit NOFS scope annotation with a
> > reference to compaction potentially locking up on pages in the readahead
> > would be a great start.
> 
> How about this (on top of the current readahead series):
> 
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 29ca25c8f01e..32fd32b913da 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -160,6 +160,16 @@ unsigned long page_cache_readahead_limit(struct address_space *mapping,
>  		.nr_pages = 0,
>  	};
>  
> +	/*
> +	 * Partway through the readahead operation, we will have added
> +	 * locked pages to the page cache, but will not yet have submitted
> +	 * them for I/O.  Adding another page may need to allocate
> +	 * memory, which can trigger memory migration.	Telling the VM

I would go with s@migration@compaction@ because it would make it more
obvious that this is about high order allocations.

The patch looks ok other than that but FS people would be better than me
to give it a proper review.

It would be great if you could mention that this shouldn't be a real
problem for many(/most?) filesystems because they tend to have NOFS like
mask in the the mapping but we would love to remove that in future
hopefully so this change is really desiable.

Thanks!

> +	 * we're in the middle of a filesystem operation will cause it
> +	 * to not touch file-backed pages, preventing a deadlock.
> +	 */
> +	unsigned int nofs = memalloc_nofs_save();
> +
>  	/*
>  	 * Preallocate as many pages as we will need.
>  	 */
> @@ -217,6 +227,7 @@ unsigned long page_cache_readahead_limit(struct address_space *mapping,
>  	 */
>  	read_pages(&rac, &page_pool);
>  	BUG_ON(!list_empty(&page_pool));
> +	memalloc_nofs_restore(nofs);
>  	return rac.nr_pages;
>  }
>  EXPORT_SYMBOL_GPL(page_cache_readahead_limit);

-- 
Michal Hocko
SUSE Labs