All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.