All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: NeilBrown <neilb@suse.de>
Cc: Dave Chinner <david@fromorbit.com>,
	Al Viro <viro@zeniv.linux.org.uk>, Jens Axboe <axboe@kernel.dk>,
	Oleg Nesterov <oleg@redhat.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/2] Allow a kthread to declare that it calls task_work_run()
Date: Tue, 5 Dec 2023 12:29:43 +0100	[thread overview]
Message-ID: <20231205-liedtexte-quantenphysik-804eab7f97d8@brauner> (raw)
In-Reply-To: <170176610023.7109.11175368186869568821@noble.neil.brown.name>

On Tue, Dec 05, 2023 at 07:48:20PM +1100, NeilBrown wrote:
> On Tue, 05 Dec 2023, Dave Chinner wrote:
> > On Mon, Dec 04, 2023 at 12:36:41PM +1100, NeilBrown wrote:
> > > User-space processes always call task_work_run() as needed when
> > > returning from a system call.  Kernel-threads generally do not.
> > > Because of this some work that is best run in the task_works context
> > > (guaranteed that no locks are held) cannot be queued to task_works from
> > > kernel threads and so are queued to a (single) work_time to be managed
> > > on a work queue.
> > > 
> > > This means that any cost for doing the work is not imposed on the kernel
> > > thread, and importantly excessive amounts of work cannot apply
> > > back-pressure to reduce the amount of new work queued.
> > > 
> > > I have evidence from a customer site when nfsd (which runs as kernel
> > > threads) is being asked to modify many millions of files which causes
> > > sufficient memory pressure that some cache (in XFS I think) gets cleaned
> > > earlier than would be ideal.  When __dput (from the workqueue) calls
> > > __dentry_kill, xfs_fs_destroy_inode() needs to synchronously read back
> > > previously cached info from storage.
> > 
> > We fixed that specific XFS problem in 5.9.
> > 
> > https://lore.kernel.org/linux-xfs/20200622081605.1818434-1-david@fromorbit.com/
> 
> Good to know - thanks.
> 
> > 
> > Can you reproduce these issues on a current TOT kernel?
> 
> I haven't tried.  I don't know if I know enough details of the work load
> to attempt it.
> 
> > 
> > If not, there's no bugs to fix in the upstream kernel. If you can,
> > then we've got more XFS issues to work through and fix. 
> > 
> > Fundamentally, though, we should not be papering over an XFS issue
> > by changing how core task_work infrastructure is used. So let's deal
> > with the XFS issue first....
> 
> I disagree.  This customer experience has demonstrated both a bug in XFS
> and bug in the interaction between fput, task_work, and nfsd.
> 
> If a bug in a filesystem that only causes a modest performance impact
> when used through the syscall API can bring the system to its knees
> through memory exhaustion when used by nfsd, then that is a robustness
> issue for nfsd.
> 
> I want to fix that robustness issue so that unusual behaviour in
> filesystems does not cause out-of-proportion bad behaviour in nfsd.
> 
> I highlighted this in the cover letter to the first version of my patch:
> 
> https://lore.kernel.org/all/170112272125.7109.6245462722883333440@noble.neil.brown.name/
> 
>   While this might point to a problem with the filesystem not handling the
>   final close efficiently, such problems should only hurt throughput, not
>   lead to memory exhaustion.

I'm still confused about this memory exhaustion claim?
If this is a filesystem problem it's pretty annoying that we have to
work around it by exposing task work to random modules.

  reply	other threads:[~2023-12-05 11:29 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-04  1:36 [PATCH 0/2 v2] Move all file-close work for nfsd into nfsd threads NeilBrown
2023-12-04  1:36 ` [PATCH 1/2] Allow a kthread to declare that it calls task_work_run() NeilBrown
2023-12-04  2:13   ` Jens Axboe
2023-12-04 21:02     ` NeilBrown
2023-12-04 22:09       ` Jens Axboe
2023-12-04 22:27         ` NeilBrown
2023-12-05 11:14         ` Christian Brauner
2023-12-05 14:06           ` Jeff Layton
2023-12-05 21:28           ` NeilBrown
2023-12-05 21:58             ` Jens Axboe
2023-12-05 22:03               ` Jens Axboe
2023-12-05 22:16                 ` NeilBrown
2023-12-05 23:23                   ` NeilBrown
2023-12-05 23:31                     ` Jens Axboe
2023-12-06 14:24                       ` Christian Brauner
2023-12-08  1:40                         ` NeilBrown
2023-12-06 14:29             ` Christian Brauner
2023-12-06  5:44           ` Christoph Hellwig
2023-12-04  2:25   ` Matthew Wilcox
2023-12-04 21:04     ` NeilBrown
2023-12-04  2:40   ` Al Viro
2023-12-04 16:12     ` Oleg Nesterov
2023-12-04 21:20     ` NeilBrown
2023-12-05  6:27       ` Dave Chinner
2023-12-05  6:41   ` Dave Chinner
2023-12-05  8:48     ` NeilBrown
2023-12-05 11:29       ` Christian Brauner [this message]
2023-12-05 11:25   ` Christian Brauner
2023-12-05 14:23     ` Chuck Lever
2023-12-04  1:36 ` [PATCH 2/2] nfsd: Don't leave work of closing files to a work queue NeilBrown
2023-12-04 16:58   ` Chuck Lever
2023-12-04 22:21     ` NeilBrown
2023-12-04 23:48       ` Chuck Lever
2023-12-05  6:36   ` kernel test robot

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=20231205-liedtexte-quantenphysik-804eab7f97d8@brauner \
    --to=brauner@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=chuck.lever@oracle.com \
    --cc=david@fromorbit.com \
    --cc=jlayton@kernel.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=neilb@suse.de \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.