Re: [PATCH next 2/3] shmem: Fix data loss when folio truncated

From: Matthew Wilcox <willy@infradead.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz>,
	William Kucharski <william.kucharski@oracle.com>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH next 2/3] shmem: Fix data loss when folio truncated
Date: Sat, 8 Jan 2022 21:25:41 +0000	[thread overview]
Message-ID: <YdoBVXifWo8/Oxie@casper.infradead.org> (raw)
In-Reply-To: <54faec38-f3f9-a326-65e2-cba91a40ccca@google.com>

On Sat, Jan 08, 2022 at 09:12:08AM -0800, Hugh Dickins wrote:
> On Fri, 7 Jan 2022, Matthew Wilcox wrote:
> > On Sun, Jan 02, 2022 at 05:34:05PM -0800, Hugh Dickins wrote:
> > > xfstests generic 098 214 263 286 412 used to pass on huge tmpfs (well,
> > > three of those _require_odirect, enabled by a shmem_direct_IO() stub),
> > > but still fail even with the partial_end fix.
> > > 
> > > generic/098 output mismatch shows actual data loss:
> > >     --- tests/generic/098.out
> > >     +++ /home/hughd/xfstests/results//generic/098.out.bad
> > >     @@ -4,9 +4,7 @@
> > >      wrote 32768/32768 bytes at offset 262144
> > >      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > >      File content after remount:
> > >     -0000000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > >     -*
> > >     -0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >     +0000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >     ...
> > 
> > generic/098 is passing for me ;-(  I'm using 'always' for THPs.
> > I'll have to try harder.  Regardless, I think your fix is good ...
> 
> Worrying that the test behaves differently.  Your 'always':
> you have '-o huge=always' in the exported TMPFS_MOUNT_OPTIONS?
> That should be enough, but I admit to belt and braces by also
> echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled

Ah, I hadn't done TMPFS_MOUNT_OPTIONS, just the
    echo always >/sys/kernel/mm/transparent_hugepage/shmem_enabled

Adding TMPFS_MOUNT_OPTIONS and retrying with what I originally posted
reproduces the bug.  Retrying with the current for-next branch doesn't.
So now I can confirm that there was a bug and your patch fixed it.
And maybe I can avoid introducing more bugs of this nature in the future.