Message-ID: <alpine.LFD.2.00.1402281324580.2231@localhost.localdomain>
Date:	Fri, 28 Feb 2014 13:38:47 +0100 (CET)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	Dave Chinner <david@...morbit.com>
cc:	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	xfs@....sgi.com
Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
 range

On Fri, 28 Feb 2014, Dave Chinner wrote:

> Date: Fri, 28 Feb 2014 06:35:33 +1100
> From: Dave Chinner <david@...morbit.com>
> To: Lukáš Czerner <lczerner@...hat.com>
> Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, xfs@....sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Thu, Feb 27, 2014 at 01:03:09PM +0100, Lukáš Czerner wrote:
> > On Thu, 27 Feb 2014, Dave Chinner wrote:
> > > > xfs/242 fails on ppc64 with latest linus tree
> > > 
> > > OK, that's a different kettle of fish. It's using 64k pages, right?
> > 
> > 64k pages, yes.
> ....
> > > Yeah, ok, the data in all the files is correct - the md5sums all
> > > match. What's different? Just about all unwritten extents are now
> > > written (i.e. data) or contain some portion of written extents.
> > > 
> > > So, ZERO_RANGE has the following size threshold for converting
> > > blocks to unwritten extents vs just zeroing them:
> > > 
> > > 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> > > 
> > > So if this is a 64k page size machine, it's going to have different
> > > extent layout compared to a 4k page size machine. The file data will
> > > still be the same, the difference will be zeroed blocks instead of
> > > unwritten blocks, and that's exactly what we see.
> > > 
> > > IOWs, the result in terms of data the application sees is correct,
> > > just the extent layout representing that zeroed data is different.
> > 
> > Ok, so that's yet another difference between xfs and ext4 code which
> > makes having generic test even more complicated.
> 
> It actually makes the need for a generic test more important. A
> generic test should handle the differences between block/page size
> behaviours - the issue is that xfs/242 was written and tested on 4k
> page machines, not 64k page machines.
> 
> We've already got the "multiple" code in _generic_test_punch to
> increase the size of the regions being worked on, so the simple fix
> here is to increase the sizes so that 64k page and 4k page behaviour
> is the same and results in the same extent layout. This is the
> normal way tests evolve as people run them on different hardware and
> configurations....
> 
> > So as I said before
> > I'll make the generic test (using _filter_hole_fiemap) and then ext4
> > specific test as well to really make sure that the extent
> > manipulation is right.
> 
> Which ignores the fact that if you turn off zeroout on ext4 then a
> generic test can also determine that the extent manipulation is
> correct. I really don't see the need for an ext4 specific test
> here...

I disagree, you assume that there is not a problem with the zeroout
code. And if there is, then the test would not be able to catch
that. And yes, bug in the zeroout code might affect zero range as
well even though we do not normally do zeroout on zero range except
block unaligned edges of the range.

Also ext4 does not need different testing for different page sizes.
That said I usually do ppc64 testing to test the operations with
granularity smaller than page size (sub page size zero out etc..).
With your proposal this goes away because we would actually test the
operation on larger extents - that's not helpful at all.

-Lukas

> 
> Cheers,
> 
> Dave.
> 

Powered by blists - more mailing lists