Re: [PATCH] generic/322: remove bad xfs_io sync_range

From: Eryu Guan <guaneryu@gmail.com>
To: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Josef Bacik <josef@toxicpanda.com>, fstests <fstests@vger.kernel.org>
Subject: Re: [PATCH] generic/322: remove bad xfs_io sync_range
Date: Sun, 18 Aug 2019 23:22:36 +0800	[thread overview]
Message-ID: <20190818152236.GA2845@desktop> (raw)
In-Reply-To: <20190815144957.tyfecxfgdhanaex7@MacBook-Pro-91.local>

On Thu, Aug 15, 2019 at 10:49:58AM -0400, Josef Bacik wrote:
> On Tue, Aug 06, 2019 at 03:48:32PM +0200, Andreas Gruenbacher wrote:
> > On Tue, 6 Aug 2019 at 00:27, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> > >
> > > The xfs_io sync_range command requires offset and length arguments.  Those are
> > > missing here, so the command fails with:
> > >
> > >   bad argument count 1 to sync_range, expected at least 2 arguments
> > >
> > > This went unnoticed because xfs_io still exits with status 0 in such cases,
> > > which looks like a separate bug.
> > >
> > > I'm assuming that the test did catch regressions as is and that the sync_range
> > > command isn't needed.  If this isn't the case, please fix the test.
> > 
> > Copying Josef who seems to be the author of this test case.
> 
> Looking back at it I think I added the sync_range because it was a little racey
> wether it would trip the problem or not.  The rename in btrfs would do the dirty
> writeout IIRC so that's probably why the problem still reproduced even though
> the sync file range was wrong.
> 
> That being said the sync_range needs to be there, so instead of deleting it we
> should have
> 
> sync_range -b 2M 1M

Hi Andreas, would you like to update the patch with the sync_range
fixed?

Thanks,
Eryu