From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id AB907C433EF for ; Tue, 1 Mar 2022 13:27:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0CE968D0002; Tue, 1 Mar 2022 08:27:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 07E628D0001; Tue, 1 Mar 2022 08:27:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EAF118D0002; Tue, 1 Mar 2022 08:27:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0177.hostedemail.com [216.40.44.177]) by kanga.kvack.org (Postfix) with ESMTP id DF2D28D0001 for ; Tue, 1 Mar 2022 08:27:45 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 9CF3E8249980 for ; Tue, 1 Mar 2022 13:27:45 +0000 (UTC) X-FDA: 79195894890.30.3CDE063 Received: from mail-il1-f181.google.com (mail-il1-f181.google.com [209.85.166.181]) by imf14.hostedemail.com (Postfix) with ESMTP id C273A100003 for ; Tue, 1 Mar 2022 13:27:44 +0000 (UTC) Received: by mail-il1-f181.google.com with SMTP id k7so5528458ilo.8 for ; Tue, 01 Mar 2022 05:27:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=szeredi.hu; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0rqEHtyUvKInY3eDADJFtpu+gdfZq4axYpoJEFqNG8M=; b=I2qodJt4VEaNZJyZteahkXLrk3gcBP6spDM9mba7O5eU1RB0noMF3SaBN+E8vfdw3Y GW1Qja0562jHy97CnNHFiAvEnhDIzFZuk2nVuHxePbR8HX9rviCDiIna9neO0UiGoLDn jsXu+RAKBdy7qxqlxo03hClTY9xe+rR+zNfDg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0rqEHtyUvKInY3eDADJFtpu+gdfZq4axYpoJEFqNG8M=; b=4WF6suuYpmPKqzVUrUTj1OB11CmasBroCjRvncSTOzbEW+MzciwOWIxLRcI7966Y7p IeH2CuzdsJTZAT39stYz2n140ibckMsVnvy/un+qdFcyuvHSqyGtcNyeWBSGYrtZmabN CC3fl9LY7IjeEAshRQIaFjFEReTXSXlcXQ/7Wrbt3Q29WeVNUj/0TQsCw93Y4fyLooSe C+3pwTnrKVm3APH7/s2A6qzx7Y8xzw4GqdLGpi/tpXMlkHSVfYb6LR/pMaCHa3xno3Yj dTVgcZEGBlsHn4RhgIJUXz8bmAzM8axR685SGtM7MVNqraXBkvUn958TGPLdqgcSrxWB zHNA== X-Gm-Message-State: AOAM532fqU4/Vivp5YkFoTl5MtmbMG1mrRPQLXhCAJtPtAd84M1pjKVo JCbovYldtPal4fJuij7Mpyo6PHyo01jDU+wlIypZnese6lA= X-Google-Smtp-Source: ABdhPJwFD3IYwN4qrYy++4yIV02k4VVunIMIlONHamgVoi7ny6ji/ttTCSgiD81N18MIcwz3ac7fQNZ2oqyazBRiwXg= X-Received: by 2002:a92:ca4a:0:b0:2ba:878e:fd12 with SMTP id q10-20020a92ca4a000000b002ba878efd12mr22464625ilo.139.1646141263901; Tue, 01 Mar 2022 05:27:43 -0800 (PST) MIME-Version: 1.0 References: <164549971112.9187.16871723439770288255.stgit@noble.brown> <164549983737.9187.2627117501000365074.stgit@noble.brown> In-Reply-To: <164549983737.9187.2627117501000365074.stgit@noble.brown> From: Miklos Szeredi Date: Tue, 1 Mar 2022 14:27:33 +0100 Message-ID: Subject: Re: [PATCH 04/11] fuse: remove reliance on bdi congestion To: NeilBrown Cc: Andrew Morton , Jan Kara , Wu Fengguang , Jaegeuk Kim , Chao Yu , Jeff Layton , Ilya Dryomov , Trond Myklebust , Anna Schumaker , Ryusuke Konishi , "Darrick J. Wong" , Philipp Reisner , Lars Ellenberg , Paolo Valente , Jens Axboe , linux-doc@vger.kernel.org, linux-mm , linux-nilfs@vger.kernel.org, Linux NFS list , linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Ext4 , ceph-devel@vger.kernel.org, drbd-dev@lists.linbit.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C273A100003 X-Stat-Signature: mxr8jct7mxwqdzzbwe8coeskhw1h5ryh Authentication-Results: imf14.hostedemail.com; dkim=none ("invalid DKIM record") header.d=szeredi.hu header.s=google header.b=I2qodJt4; spf=pass (imf14.hostedemail.com: domain of miklos@szeredi.hu designates 209.85.166.181 as permitted sender) smtp.mailfrom=miklos@szeredi.hu; dmarc=none X-HE-Tag: 1646141264-373306 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 22 Feb 2022 at 04:18, NeilBrown wrote: > > The bdi congestion tracking in not widely used and will be removed. > > Fuse is one of a small number of filesystems that uses it, setting both > the sync (read) and async (write) congestion flags at what it determines > are appropriate times. > > The only remaining effect of the sync flag is to cause read-ahead to be > skipped. > The only remaining effect of the async flag is to cause (some) > WB_SYNC_NONE writes to be skipped. > > So instead of setting the flags, change: > - .readahead to stop when it has submitted all non-async pages > for read. > - .writepages to do nothing if WB_SYNC_NONE and the flag would be set > - .writepage to return AOP_WRITEPAGE_ACTIVATE if WB_SYNC_NONE > and the flag would be set. > > The writepages change causes a behavioural change in that pageout() can > now return PAGE_ACTIVATE instead of PAGE_KEEP, so SetPageActive() will > be called on the page which (I think) will further delay the next attempt > at writeout. This might be a good thing. > > Signed-off-by: NeilBrown > --- > fs/fuse/control.c | 17 ----------------- > fs/fuse/dev.c | 8 -------- > fs/fuse/file.c | 17 +++++++++++++++++ > 3 files changed, 17 insertions(+), 25 deletions(-) > > diff --git a/fs/fuse/control.c b/fs/fuse/control.c > index 000d2e5627e9..7cede9a3bc96 100644 > --- a/fs/fuse/control.c > +++ b/fs/fuse/control.c > @@ -164,7 +164,6 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file, > { > unsigned val; > struct fuse_conn *fc; > - struct fuse_mount *fm; > ssize_t ret; > > ret = fuse_conn_limit_write(file, buf, count, ppos, &val, > @@ -178,22 +177,6 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file, > down_read(&fc->killsb); > spin_lock(&fc->bg_lock); > fc->congestion_threshold = val; > - > - /* > - * Get any fuse_mount belonging to this fuse_conn; s_bdi is > - * shared between all of them > - */ > - > - if (!list_empty(&fc->mounts)) { > - fm = list_first_entry(&fc->mounts, struct fuse_mount, fc_entry); > - if (fc->num_background < fc->congestion_threshold) { > - clear_bdi_congested(fm->sb->s_bdi, BLK_RW_SYNC); > - clear_bdi_congested(fm->sb->s_bdi, BLK_RW_ASYNC); > - } else { > - set_bdi_congested(fm->sb->s_bdi, BLK_RW_SYNC); > - set_bdi_congested(fm->sb->s_bdi, BLK_RW_ASYNC); > - } > - } > spin_unlock(&fc->bg_lock); > up_read(&fc->killsb); > fuse_conn_put(fc); > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index cd54a529460d..e1b4a846c90d 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -315,10 +315,6 @@ void fuse_request_end(struct fuse_req *req) > wake_up(&fc->blocked_waitq); > } > > - if (fc->num_background == fc->congestion_threshold && fm->sb) { > - clear_bdi_congested(fm->sb->s_bdi, BLK_RW_SYNC); > - clear_bdi_congested(fm->sb->s_bdi, BLK_RW_ASYNC); > - } > fc->num_background--; > fc->active_background--; > flush_bg_queue(fc); > @@ -540,10 +536,6 @@ static bool fuse_request_queue_background(struct fuse_req *req) > fc->num_background++; > if (fc->num_background == fc->max_background) > fc->blocked = 1; > - if (fc->num_background == fc->congestion_threshold && fm->sb) { > - set_bdi_congested(fm->sb->s_bdi, BLK_RW_SYNC); > - set_bdi_congested(fm->sb->s_bdi, BLK_RW_ASYNC); > - } > list_add_tail(&req->list, &fc->bg_queue); > flush_bg_queue(fc); > queued = true; > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 829094451774..94747bac3489 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -966,6 +966,14 @@ static void fuse_readahead(struct readahead_control *rac) > struct fuse_io_args *ia; > struct fuse_args_pages *ap; > > + if (fc->num_background >= fc->congestion_threshold && > + rac->ra->async_size >= readahead_count(rac)) > + /* > + * Congested and only async pages left, so skip the > + * rest. > + */ > + break; Ah, you are taking care of it here... Regarding the async part: a potential (corner?) case is if task A is reading region X and triggering readahead for region Y and at the same time task B is reading region Y. In the congestion case it can happen that non-uptodate pages in Y are truncated off the pagecache while B is waiting for them to become uptodate. This shouldn't be too hard to trigger, just need two sequential readers of the same file, where one is just ahead of the other. I'll try to do a test program for this case specifically. Thanks, Miklos