Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 31 Jan 2022 04:28:31 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     NeilBrown <neilb@suse.de>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Jeff Layton <jlayton@kernel.org>,
        Ilya Dryomov <idryomov@gmail.com>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>, linux-mm@kvack.org,
        linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] fuse: remove reliance on bdi congestion
Message-ID: <YfdlbxezYSOSYmJf@casper.infradead.org>
References: <164360127045.4233.2606812444285122570.stgit@noble.brown>
 <164360183348.4233.761031466326833349.stgit@noble.brown>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <164360183348.4233.761031466326833349.stgit@noble.brown>
Precedence: bulk

On Mon, Jan 31, 2022 at 03:03:53PM +1100, NeilBrown wrote:
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 182b24a14804..5f74e2585f50 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -781,6 +781,9 @@ static int fuse_dax_writepages(struct address_space *mapping,
>  	struct inode *inode = mapping->host;
>  	struct fuse_conn *fc = get_fuse_conn(inode);
>  
> +	if (wbc->sync_mode == WB_SYNC_NONE &&
> +	    fc->num_background >= fc->congestion_threshold)
> +		return 0;
>  	return dax_writeback_mapping_range(mapping, fc->dax->dev, wbc);

This makes no sense.  Doing writeback for DAX means flushing the
CPU cache (in a terribly inefficient way), but it's not going to
be doing anything in the background; it's a sync operation.

> +++ b/fs/fuse/file.c
> @@ -958,6 +958,8 @@ static void fuse_readahead(struct readahead_control *rac)
>  
>  	if (fuse_is_bad(inode))
>  		return;
> +	if (fc->num_background >= fc->congestion_threshold)
> +		return;

This seems like a bad idea to me.  If we don't even start reads on
readahead pages, they'll get ->readpage called on them one at a time
and the reading thread will block.  It's going to lead to some nasty
performance problems, exactly when you don't want them.  Better to
queue the reads internally and wait for congestion to ease before
submitting the read.