Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <20211206172340.fded3873aed7e853b54ab276@linux-foundation.org>
References: <20211206172340.fded3873aed7e853b54ab276@linux-foundation.org> <1638760762-27239-1-git-send-email-huangzhaoyang@gmail.com>
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     dhowells@redhat.com, Huangzhaoyang <huangzhaoyang@gmail.com>,
        Zhaoyang Huang <zhaoyang.huang@unisoc.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-cachefs@redhat.com
Subject: Re: [PATCH] mm: mask DIRECT_RECLAIM in kswapd
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2502102.1638956412.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Wed, 08 Dec 2021 09:40:12 +0000
Message-ID: <2502103.1638956412@warthog.procyon.org.uk>
Precedence: bulk

Andrew Morton <akpm@linux-foundation.org> wrote:

> >       __fscache_maybe_release_page
> > 	...
> >          if (!(gfp & __GFP_DIRECT_RECLAIM) || !(gfp & __GFP_FS)) {
> >                  fscache_stat(&fscache_n_store_vmscan_busy);
> >                  return false;
> >          }
> =

> Well, we have thus far been permitting kswapd's memory allocations to
> enter direct reclaim.  Forbidding that kernel-wide might be the right
> thing to do, or might not be.  But disabling it kernel-wide because of
> a peculiar hack in fscache is not good justification.

It's avoiding sleeping in ->releasepage() if fscache is doing something wi=
th
the page.  With the old I/O still used by nfs and cifs, PG_fscache means t=
hat
the page is known to fscache and it might be doing something with it in th=
e
background.  You have to ask fscache to release the page, which may requir=
e
I/O to take place, to get rid of the mark.

With the new I/O, as used by 9p, afs and ceph, where we're doing async DIO
between the page and the cache, PG_fscache just means that there's a DIO w=
rite
in progress from the page.  It will be cleared when the DIO completes.

I'm fine with changing the condition in the if-statement.  Note that in my
fscache-rewrite branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log=
/?h=3Dfscache-rewrite

I've been changing this to:

	if (!gfpflags_allow_blocking(gfp) || !(gfp & __GFP_FS))

and the old I/O is gone.  This is aimed at the next merge window.  If you =
want
me to change it there, let me know.

David