linux-ext4.vger.kernel.org archive mirror
 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

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

Thread overview: 19+ 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 20:41           ` Ben Myers
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  1:47               ` Dave Chinner
2013-03-19  2:00                 ` Theodore Ts'o
2013-03-19  2:22                   ` Dave Chinner
2013-03-19  2:28                   ` Eric Sandeen
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).