Re: [PATCH 6/8] punch: skip fpunch tests when op length not congruent with file allocation unit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 13, 2022 at 10:33:05AM -0700, Darrick J. Wong wrote:
> On Thu, Jul 14, 2022 at 01:04:26AM +0800, Zorro Lang wrote:
> > On Tue, Jul 12, 2022 at 05:57:07PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > 
> > > Skip the generic fpunch tests on a file when the file's allocation unit
> > > size is not congruent with the proposed testing operations.
> > > 
> > > This can be the case when we're testing reflink and fallocate on the XFS
> > > realtime device.  For those configurations, the file allocation unit is
> > > a realtime extent, which can be any integer multiple of the block size.
> > > If the request length isn't an exact multiple of the allocation unit
> > > size, reflink and fallocate will fail due to alignment issues, so
> > > there's no point in running these tests.
> > > 
> > > Assuming this edgecase configuration of an edgecase feature is
> > > vanishingly rare, let's just _notrun the tests instead of rewriting a
> > > ton of tests to do their integrity checking by hand.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > ---
> > >  common/punch |    1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > 
> > > diff --git a/common/punch b/common/punch
> > > index 4d16b898..7560edf8 100644
> > > --- a/common/punch
> > > +++ b/common/punch
> > > @@ -250,6 +250,7 @@ _test_generic_punch()
> > >  	_8k="$((multiple * 8))k"
> > >  	_12k="$((multiple * 12))k"
> > >  	_20k="$((multiple * 20))k"
> > > +	_require_congruent_file_oplen $TEST_DIR $((multiple * 4096))
> > 
> > Should the $TEST_DIR be $testfile, or $(dirname $testfile) ?
> 
> Ah, right, that ought to be $(dirname $testfile), thanks for catching
> that.  I guess I didn't catch that because all the current callers pass
> in $TEST_DIR/<somefile>, which is functionally the same, but a landmine
> nonetheless.

Yeah, I checked all cases which call _test_generic_punch(), they all use
$TEST_DIR. In this patchset (e.g. patch 2/8) you sometimes use $testdir
for _require_congruent_file_oplen, sometimes use $TEST_DIR or $SCRATCH_MNT
directly (even there's $testdir too), although they're not wrong :)

This patchset really touch many cases, they looks good mostly, but to avoid
bringing in regressions, I have to give them more tests before merging. And
welcome more reviewing from others :)

Thanks,
Zorro

> 
> --D
> 
> > >  
> > >  	# initial test state must be defined, otherwise the first test can fail
> > >  	# due ot stale file state left from previous tests.
> > > 
> > 
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux