public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Liu <jeff.liu@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Mark Tinguely <tinguely@sgi.com>,
	xfs@oss.sgi.com
Subject: Re: [PATCH v2 2/2] xfstests: introduce 280 for SEEK_DATA/SEEK_HOLE copy check
Date: Thu, 09 Feb 2012 17:50:37 +0800	[thread overview]
Message-ID: <4F3396ED.9070406@oracle.com> (raw)
In-Reply-To: <20120208224412.GA7479@dastard>

On 02/09/2012 06:44 AM, Dave Chinner wrote:

> On Wed, Feb 08, 2012 at 10:06:27PM +0800, Jeff Liu wrote:
>> On 02/08/2012 04:55 PM, Dave Chinner wrote:
>>
>>> On Mon, Feb 06, 2012 at 10:30:40PM +0800, Jeff Liu wrote:
>>>> Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.
>>>>
>>>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>>>
>>> This has the same problems with $seq.out as 279, so I won't repeat
>>> them here.
>>>
>>> .....
>>>> +_cleanup()
>>>> +{
>>>> +	rm -f $src $dest
>>>> +}
>>>> +
>>>> +# seek_copy_test_01()
>>>> +# create a 100Mytes file in preallocation mode.
>>>> +# fallocate offset start from 0.
>>>> +# the first data extent offset start from 80991, write 4Kbytes,
>>>> +# and then skip 195001 bytes for next write.
>>>
>>> Oh, man, you didn't write a program to do this, do you?
>>
>> Unfortunately, I have already included file creation at seek_copy_tester :(
>>
>>> This is what
>>> xfs_io is for - to create arbitary file configurations as quickly as
>>> you can type them.  Then all you need is a simple program that
>>> copies the extents, and the test can check everything else.
>>
>> Yes, xfs_io is pretty cool, and it really convenient for file creation for XFS.
> 
> xfs_io is filesystem agnostic. Currently it needs the "-F" flag to
> tell it to work on non-xfs filesystems, but Eric posted patches a
> couple of days ago to remove that (i.e to automatically detect XFS
> filesystems and enable all the xfs specific stuff).

Awesome! I just playing around it, so far so cool. :)

> 
>> I wrote it(create_data_and_holes()) in seek_copy_tester since I'd make it as a general SEEK_DATA/SEEK_HOLE tester
>> for other file systems without this utility too.
> 
> xfs_io is used all throughout xfstests in generic tests. Just look
> at common.punch::_test_generic_punch as an example. That function
> uses xfs_io to test the different methods of perallocation and hole
> punching supported by a bunch of different filesystems in 3
> different tests. IOWs, the generic tests use fallocate and the XFS
> specific tests use XFS ioctls, but all tests use xfs_io to run the
> commands....

Now I understand your opinions, those changes will be reflect in V3.

> 
>>>> +# seek_copy_test_02()
>>>> +# create a 100Mytes file in preallocation mode.
>>>> +# fallocate offset start from 0.
>>>> +# the first data extent offset start from 0, write 16Kbytes,
>>>> +# and then skip 8Mbytes for next write.
>>>> +# Try flushing DIRTY pages to WRITEBACK mode, this is intended to
>>>> +# test data buffer lookup in WRITEBACK pages.
>>>
>>> There's no guarantee that that the seeks will occur while the pages
>>> are in the writeback. It's entirely dependent on IO latency -
>>> writing 16k of data to a disk cache will take less time than it
>>> takes to go back up into userspace and start the sparse copy.
>>> Indeed, i suspect that the 16x16k IOs that this tes does will fit
>>> all into that category even on basic SATA configs....
>>>
>>> Also, you could the fadvise command in xfs_io to do this, as
>>> POSIX_FADV_DONTNEED will trigger async writeback -it will then skip
>>> invalidation of pages under writeback so they will remain in the
>>> cache. i.e. '-c "fadvise -d 0 100m"'
>>>
>>> Ideally, we should add all the different sync methods to an xfs_io
>>> command...
>>
>> Thanks again for the detained info.
>> It's definitely depending on the IO latency to test cover those page status conversion.
>> I have verified the old patch with page probe routine on my laptop SATA disk controller,
>> but not tried against other faster controllers.  If we agree to make it as a general tester, maybe I can
>> try to implement it by referring to xfs_io fadvise, I guess it use posix_fadvise(2), will check it later.
> 
> Yes, it uses posix_fadvise64().
> 
> As it is, I spent 15 minutes adding support for sync_file_range()
> to xfs_io. The patch is attached below.

I'll apply your patch to try it out.

Thanks,
-Jeff

> 
>>>> +# the first data extent offset start from 512, write 4Kbytes,
>>>> +# and then skip 1Mbytes for next write.
>>>> +# don't make holes at the end of file.
>>>
>>> I'm not sure what this means - you always write zeros at the end of
>>> file, and the only difference is that "make holes at EOF" does an
>>> ftruncate to the total size before writing zeros up to it. It
>>> appears to me like you end up with the same file size and shape
>>> either way....
>>
>> Oops! this is a code bug. I want to create a hole at EOF if possible when "-E(wrote_hole_at_eof)" option was specified.
>> It can be fixed as below FIXME:
> 
> Yes, that'd work ;)
> 
> Cheers,
> 
> Dave.


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2012-02-09  9:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-06 14:30 [PATCH v2 2/2] xfstests: introduce 280 for SEEK_DATA/SEEK_HOLE copy check Jeff Liu
2012-02-06 22:30 ` Mark Tinguely
2012-02-07  7:40   ` Jeff Liu
2012-02-07 14:17     ` Mark Tinguely
2012-02-08  8:55 ` Dave Chinner
2012-02-08 14:06   ` Jeff Liu
2012-02-08 22:44     ` Dave Chinner
2012-02-09  9:50       ` Jeff Liu [this message]
2012-05-11 15:15 ` Rich Johnston
2012-05-15  4:47   ` Jeff Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F3396ED.9070406@oracle.com \
    --to=jeff.liu@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=tinguely@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox