Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
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()
In-reply-to: <163096695999.2518.10383290668057550257@noble.neil.brown.name>
References: <163090344807.19339.10071205771966144716@noble.neil.brown.name>,
 <848A6498-CFF3-4C66-AE83-959F8221E930@oracle.com>,
 <YTZ4E0Zh6F/WSpy0@casper.infradead.org>,
 <163096695999.2518.10383290668057550257@noble.neil.brown.name>
Date:   Tue, 07 Sep 2021 10:41:33 +1000
Message-id: <163097529362.2518.16697605173806213577@noble.neil.brown.name>
Precedence: bulk

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