All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Ben Myers <bpm@sgi.com>, Eric Sandeen <sandeen@redhat.com>,
	xfs-oss <xfs@oss.sgi.com>,
	linux-ext4@vger.kernel.org, Eric Whitney <enwlinux@gmail.com>
Subject: Re: possible dev branch regression - xfstest 285/1k
Date: Tue, 19 Mar 2013 13:07:09 +1100	[thread overview]
Message-ID: <20130319020709.GW6369@dastard> (raw)
In-Reply-To: <20130319014014.GA4660@thunk.org>

On Mon, Mar 18, 2013 at 09:40:14PM -0400, Theodore Ts'o wrote:
> On Tue, Mar 19, 2013 at 10:12:33AM +1100, Dave Chinner wrote:
> > I know that Ted has already asked "what is an extent", but that's
> > also missing the point. An extent is defined, just like for on-disk
> > extent records, as a region of a file that is both logically and
> > physically contiguous. From that, a fragmented file is a file that
> > is logically contiguous but physically disjointed, and a sparse file
> > is one that is logically disjointed. i.e. it is the relationship
> > between extents that defines "sparse" and "fragmented", not the
> > definition of an extent itself.
> 
> Dave --- I think we're talking about two different tests.  This
> particular test is xfstest #285.

Yeah, I just realised that as I was reading through my ext4 list
feed...

> The test in question is subtest #8, which preallocates a 4MB file, and
> then writes a block filled with 'a' which is sized to the file system
> block size, at offset 10*fs_block_size.  It then checks to make sure
> SEEK_HOLE and SEEK_DATA is what it expects.

Yup, and as I just said in reply to myself, this means the same
reasoning applies - we can simply change the file layout to make
holes large enough that zero-out isn't an issue.

> > Looking at the test itself, then.  The backwards synchronous write
> > trick that is used by 218?  That's an underhanded trick to make XFS
> > create a fragmented file. We are not testing that the defragmenter
> > knows that it's a backwards written file - we are testing that it
> > sees the file as logically contiguous and physically disjointed, and
> > then defragments it successfully.
> 
> What I was saying --- in the other mail thread --- is that it's open
> to question whether a file which is being written via a random-write
> pattern, resulting in a physically contiguous, but not contiguous from
> a logical block number point of view, is worth defragging or not.  It
> all depends on whether the file is likely to be read sequentially in
> the future, or whether it will continue to be accessed via a random
> access pattern.  In the latter case, it might not be worth defragging
> the file.

AFAICT, that's something the defragmenter has no information on.
For example, two files with identical fragmentation patterns may be
accessed differently - how does the defragmenter know about that and
hence treat each file differently?

> In fact, I tend to agree with the argument we might as well attempt to
> make the file logically contiguous so that it's efficient to read the
> file sequentially.  But the people at Fujitsu who wrote the algorithms
> in e2defrag had gone out of their way to detect this case and avoid
> defragging the file so long as the physical blocks in use were
> contiguous --- and I believe that's also a valid design decision.

Sure - I never said it wasn't a valid categorisation. What is now
obvious to everyone is that it's a different defintion of
fragmentation to what the test (and xfs_fsr) expects. ;)

> Depending on how we resolve this particular design question, we can
> then decide whether we need to make test #218 fs specific or not.
> There was no thought design choics made by ext4 should drive changes
> in how the defragger works in xfs or btrfs, or vice versa.

Exactly. :)

> So I was looking for discussion by the ext4 developers; I was not
> requesting any changes from the XFS developers with respect to test
> #218.  (Not yet; and perhaps not ever.)

I know - what i was trying to do was to make sure that everyone
understood the theory behind the test before the discussion went too
far off the beaten track...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Eric Whitney <enwlinux@gmail.com>,
	Eric Sandeen <sandeen@redhat.com>, Ben Myers <bpm@sgi.com>,
	linux-ext4@vger.kernel.org, xfs-oss <xfs@oss.sgi.com>
Subject: Re: possible dev branch regression - xfstest 285/1k
Date: Tue, 19 Mar 2013 13:07:09 +1100	[thread overview]
Message-ID: <20130319020709.GW6369@dastard> (raw)
In-Reply-To: <20130319014014.GA4660@thunk.org>

On Mon, Mar 18, 2013 at 09:40:14PM -0400, Theodore Ts'o wrote:
> On Tue, Mar 19, 2013 at 10:12:33AM +1100, Dave Chinner wrote:
> > I know that Ted has already asked "what is an extent", but that's
> > also missing the point. An extent is defined, just like for on-disk
> > extent records, as a region of a file that is both logically and
> > physically contiguous. From that, a fragmented file is a file that
> > is logically contiguous but physically disjointed, and a sparse file
> > is one that is logically disjointed. i.e. it is the relationship
> > between extents that defines "sparse" and "fragmented", not the
> > definition of an extent itself.
> 
> Dave --- I think we're talking about two different tests.  This
> particular test is xfstest #285.

Yeah, I just realised that as I was reading through my ext4 list
feed...

> The test in question is subtest #8, which preallocates a 4MB file, and
> then writes a block filled with 'a' which is sized to the file system
> block size, at offset 10*fs_block_size.  It then checks to make sure
> SEEK_HOLE and SEEK_DATA is what it expects.

Yup, and as I just said in reply to myself, this means the same
reasoning applies - we can simply change the file layout to make
holes large enough that zero-out isn't an issue.

> > Looking at the test itself, then.  The backwards synchronous write
> > trick that is used by 218?  That's an underhanded trick to make XFS
> > create a fragmented file. We are not testing that the defragmenter
> > knows that it's a backwards written file - we are testing that it
> > sees the file as logically contiguous and physically disjointed, and
> > then defragments it successfully.
> 
> What I was saying --- in the other mail thread --- is that it's open
> to question whether a file which is being written via a random-write
> pattern, resulting in a physically contiguous, but not contiguous from
> a logical block number point of view, is worth defragging or not.  It
> all depends on whether the file is likely to be read sequentially in
> the future, or whether it will continue to be accessed via a random
> access pattern.  In the latter case, it might not be worth defragging
> the file.

AFAICT, that's something the defragmenter has no information on.
For example, two files with identical fragmentation patterns may be
accessed differently - how does the defragmenter know about that and
hence treat each file differently?

> In fact, I tend to agree with the argument we might as well attempt to
> make the file logically contiguous so that it's efficient to read the
> file sequentially.  But the people at Fujitsu who wrote the algorithms
> in e2defrag had gone out of their way to detect this case and avoid
> defragging the file so long as the physical blocks in use were
> contiguous --- and I believe that's also a valid design decision.

Sure - I never said it wasn't a valid categorisation. What is now
obvious to everyone is that it's a different defintion of
fragmentation to what the test (and xfs_fsr) expects. ;)

> Depending on how we resolve this particular design question, we can
> then decide whether we need to make test #218 fs specific or not.
> There was no thought design choics made by ext4 should drive changes
> in how the defragger works in xfs or btrfs, or vice versa.

Exactly. :)

> So I was looking for discussion by the ext4 developers; I was not
> requesting any changes from the XFS developers with respect to test
> #218.  (Not yet; and perhaps not ever.)

I know - what i was trying to do was to make sure that everyone
understood the theory behind the test before the discussion went too
far off the beaten track...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2013-03-19  2:07 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-15 22:28 possible dev branch regression - xfstest 285/1k Eric Whitney
2013-03-16  2:32 ` Zheng Liu
2013-03-16 15:09 ` Zheng Liu
2013-03-17  3:06   ` Theodore Ts'o
2013-03-17  6:13     ` Zheng Liu
2013-03-18 16:10     ` Eric Sandeen
2013-03-18 16:54       ` gnehzuil.liu
2013-03-18 17:09       ` Theodore Ts'o
2013-03-18 17:34         ` Eric Sandeen
2013-03-18 17:34           ` Eric Sandeen
2013-03-18 20:41           ` Ben Myers
2013-03-18 23:12             ` Dave Chinner
2013-03-18 23:12               ` Dave Chinner
2013-03-19  1:40               ` Theodore Ts'o
2013-03-19  2:07                 ` Dave Chinner [this message]
2013-03-19  2:07                   ` Dave Chinner
2013-03-19  1:47               ` Dave Chinner
2013-03-19  1:47                 ` Dave Chinner
2013-03-19  2:00                 ` Theodore Ts'o
2013-03-19  2:22                   ` Dave Chinner
2013-03-19  2:22                     ` Dave Chinner
2013-03-19  2:28                   ` Eric Sandeen
2013-03-19  8:50                     ` Lukáš Czerner
2013-03-19  8:50                     ` Lukáš Czerner
2013-03-17  3:36   ` Eric Whitney

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=20130319020709.GW6369@dastard \
    --to=david@fromorbit.com \
    --cc=bpm@sgi.com \
    --cc=enwlinux@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=tytso@mit.edu \
    --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.