linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "NeilBrown" <neilb@suse.de>
To: "Matthew Wilcox" <willy@infradead.org>
Cc: "Chuck Lever III" <chuck.lever@oracle.com>,
	"Bruce Fields" <bfields@fieldses.org>,
	"Linux NFS Mailing List" <linux-nfs@vger.kernel.org>,
	"Mel Gorman" <mgorman@suse.com>, "Linux-MM" <linux-mm@kvack.org>
Subject: Re: [PATCH] SUNRPC: use congestion_wait() in svc_alloc_args()
Date: Tue, 07 Sep 2021 10:41:33 +1000	[thread overview]
Message-ID: <163097529362.2518.16697605173806213577@noble.neil.brown.name> (raw)
In-Reply-To: <163096695999.2518.10383290668057550257@noble.neil.brown.name>

On Tue, 07 Sep 2021, NeilBrown wrote:
> Even if we just provided
> 
>  void reclaim_progress_wait(void)
>  {
>         schedule_timeout_uninterruptible(HZ/20);
>  }
> 
> that would be an improvement.

I contemplated providing a patch, but the more I thought about it, the
less sure I was.

When does a single-page GFP_KERNEL allocation fail?  Ever?

I know that if I add __GFP_NOFAIL then it won't fail and that is
preferred to looping.
I know that if I add __GFP_RETRY_MAILFAIL (or others) then it might
fail.
But that is the semantics for a plain GFP_KERNEL ??

I recall a suggestion one that it would only fail if the process was
being killed by the oom killer.  That seems reasonable and would suggest
that retrying is really bad.  Is that true?

For svc_alloc_args(), it might be better to fail and have the calling
server thread exit.  This would need to be combined with dynamic
thread-count management so that when a request arrived, a new thread
might be started.

So maybe we really don't want reclaim_progress_wait(), and all current
callers of congestion_wait() which are just waiting for allocation to
succeed should be either change to use __GFP_NOFAIL, or to handle
failure.
For the ext4_truncate case() that would be easier if there were a
PF_MEMALLOC_NOFAIL flag though maybe passing down __GFP_MNOFAIL isn't
too hard.

(this is why we all work-around problems in the platform rather than
 fixing them.  Fixing them is just too hard...)

NeilBrown

  reply	other threads:[~2021-09-07  0:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06  4:44 [PATCH] SUNRPC: use congestion_wait() in svc_alloc_args() NeilBrown
2021-09-06 15:46 ` Chuck Lever III
2021-09-06 20:20   ` Matthew Wilcox
2021-09-06 22:13     ` Bruce Fields
2021-09-06 22:22     ` NeilBrown
2021-09-07  0:41       ` NeilBrown [this message]
2021-09-07 14:53         ` Chuck Lever III
2021-09-07 15:39           ` Bruce Fields
2021-09-07 15:41           ` Mel Gorman
2021-09-07 16:21             ` Chuck Lever III
2021-09-07 21:47           ` NeilBrown
2021-09-07  8:17       ` Mel Gorman
2021-09-06 21:52   ` NeilBrown

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=163097529362.2518.16697605173806213577@noble.neil.brown.name \
    --to=neilb@suse.de \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mgorman@suse.com \
    --cc=willy@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).