Received-SPF: pass (google.com: domain of linux-kernel+bounces-71109-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 19 Feb 2024 02:15:47 -0800 (PST)
From: Hugh Dickins <hughd@google.com>
To: Daniel Gomez <da.gomez@samsung.com>
cc: "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>, 
    "brauner@kernel.org" <brauner@kernel.org>, "jack@suse.cz" <jack@suse.cz>, 
    "hughd@google.com" <hughd@google.com>, 
    "akpm@linux-foundation.org" <akpm@linux-foundation.org>, 
    "dagmcr@gmail.com" <dagmcr@gmail.com>, 
    "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, 
    "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
    "linux-mm@kvack.org" <linux-mm@kvack.org>, 
    "willy@infradead.org" <willy@infradead.org>, 
    "hch@infradead.org" <hch@infradead.org>, 
    "mcgrof@kernel.org" <mcgrof@kernel.org>, 
    Pankaj Raghav <p.raghav@samsung.com>, 
    "gost.dev@samsung.com" <gost.dev@samsung.com>
Subject: Re: [RFC PATCH 0/9] shmem: fix llseek in hugepages
In-Reply-To: <25i3n46nanffixvzdby6jwxgboi64qnleixz33dposwuwmzj7p@6yvgyakozars>
Message-ID: <e3602f54-b333-7c8c-0031-6a14b32a3990@google.com>
References: <20240209142901.126894-1-da.gomez@samsung.com> <CGME20240214194911eucas1p187ae3bc5b2be4e0d2155f9ce792fdf8b@eucas1p1.samsung.com> <25i3n46nanffixvzdby6jwxgboi64qnleixz33dposwuwmzj7p@6yvgyakozars>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Wed, 14 Feb 2024, Daniel Gomez wrote:
> On Fri, Feb 09, 2024 at 02:29:01PM +0000, Daniel Gomez wrote:
> > Hi,
> > 
> > The following series fixes the generic/285 and generic/436 fstests for huge
> > pages (huge=always). These are tests for llseek (SEEK_HOLE and SEEK_DATA).
> > 
> > The implementation to fix above tests is based on iomap per-block tracking for
> > uptodate and dirty states but applied to shmem uptodate flag.
> 
> Hi Hugh, Andrew,
> 
> Could you kindly provide feedback on these patches/fixes? I'd appreciate your
> input on whether we're headed in the right direction, or maybe not.

I am sorry, Daniel, but I see this series as misdirected effort.

We do not want to add overhead to tmpfs and the kernel, just to pass two
tests which were (very reasonably) written for fixed block size, before
the huge page possibility ever came in.

If one opts for transparent huge pages in the filesystem, then of course
the dividing line between hole and data becomes more elastic than before.

It would be a serious bug if lseek ever reported an area of non-0 data as
in a hole; but I don't think that is what generic/285 or generic/436 find.

Beyond that, "man 2 lseek" is very forgiving of filesystem implementation.

I'll send you my stack of xfstests patches (which, as usual, I cannot
afford the time now to re-review and post): there are several tweaks to
seek_sanity_test in there for tmpfs huge pages, along with other fixes
for tmpfs (and some fixes to suit an old 32-bit build environment).

With those tweaks, generic/285 and generic/436 and others (but not all)
have been passing on huge tmpfs for several years.  If you see something
you'd like to add your name to in that stack, or can improve upon, please
go ahead and post to the fstests list (Cc me).

Thanks,
Hugh

> 
> Thanks,
> Daniel
> 
> > 
> > The motivation is to avoid any regressions in tmpfs once it gets support for
> > large folios.
> > 
> > Testing with kdevops
> > Testing has been performed using fstests with kdevops for the v6.8-rc2 tag.
> > There are currently different profiles supported [1] and for each of these,
> > a baseline of 20 loops has been performed with the following failures for
> > hugepages profiles: generic/080, generic/126, generic/193, generic/245,
> > generic/285, generic/436, generic/551, generic/619 and generic/732.
> > 
> > If anyone interested, please find all of the failures in the expunges directory:
> > https://github.com/linux-kdevops/kdevops/tree/master/workflows/fstests/expunges/6.8.0-rc2/tmpfs/unassigned
> > 
> > [1] tmpfs profiles supported in kdevops: default, tmpfs_noswap_huge_never,
> > tmpfs_noswap_huge_always, tmpfs_noswap_huge_within_size,
> > tmpfs_noswap_huge_advise, tmpfs_huge_always, tmpfs_huge_within_size and
> > tmpfs_huge_advise.
> > 
> > More information:
> > https://github.com/linux-kdevops/kdevops/tree/master/workflows/fstests/expunges/6.8.0-rc2/tmpfs/unassigned
> > 
> > All the patches has been tested on top of v6.8-rc2 and rebased onto latest next
> > tag available (next-20240209).
> > 
> > Daniel
> > 
> > Daniel Gomez (8):
> >   shmem: add per-block uptodate tracking for hugepages
> >   shmem: move folio zero operation to write_begin()
> >   shmem: exit shmem_get_folio_gfp() if block is uptodate
> >   shmem: clear_highpage() if block is not uptodate
> >   shmem: set folio uptodate when reclaim
> >   shmem: check if a block is uptodate before splice into pipe
> >   shmem: clear uptodate blocks after PUNCH_HOLE
> >   shmem: enable per-block uptodate
> > 
> > Pankaj Raghav (1):
> >   splice: don't check for uptodate if partially uptodate is impl
> > 
> >  fs/splice.c |  17 ++-
> >  mm/shmem.c  | 340 ++++++++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 332 insertions(+), 25 deletions(-)
> > 
> > -- 
> > 2.43.0