All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger@sun.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: Josef Bacik <jbacik@redhat.com>, linux-ext4@vger.kernel.org
Subject: Re: [RFC][PATCH] fiemap support for ext3
Date: Mon, 21 Apr 2008 20:33:15 -0600	[thread overview]
Message-ID: <20080422023315.GR2775@webber.adilger.int> (raw)
In-Reply-To: <480D1238.8080000@redhat.com>

On Apr 21, 2008  17:16 -0500, Eric Sandeen wrote:
> Andreas Dilger wrote:
> > Josef, thanks for doing this work.  Having more than a single filesystem
> > implement FIEMAP (especially a block-mapped one) is very useful.  
> 
> I have an xfs patch too if anyone wants it ;)

Please, do send it on to Dave Chinner.  He was one of the main contributors
to the FIEMAP specification.  He could also give another opinion on whether
the NUM_EXTENTS should return an "extent" for a hole or not.

> So hopefully we can roll out at least 3 fs's when it goes upstream.
> 
> > Did you
> > look at all at making a "generic_fiemap()" function?  It seems very little
> > of ext3_fiemap() is ext3 specific, only the call to ext3_force_commit()
> > (which could just be a sync on the inode), ext3_block_map() (generic for
> > all block-based filesystems), and truncate_mutex (would i_sem be enough?).
> 
> Yep, I agree, it'd be good if ! ->fiemap then go the generic route.
> 
> Although my only question/worry is do all filesystems behave sanely in
> the face of large b_size for getblocks?  All that can handle direct IO
> do anyway.
> 
> >> +int ext3_fiemap(struct inode *inode, unsigned long arg)
> >> +{
> >> +	/*
> >> +	 * if fm_start is in the middle of the current block, get the next
> >> +	 * block so we don't end up returning a start thats before the given
> >> +	 * fm_start
> >> +	 */
> >> +	start_blk = (fiemap_s->fm_start + (1 << inode->i_blkbits) - 1) >>
> >> +		inode->i_blkbits;
> > 
> > Hmm, I'd think that if someone is requesting the mapping for bytes [50-5000]
> > they wouldn't be very happy with the mapping returned being [4096-8191],
> > because it is missing part of the requested range.  Instead, the fm_start
> > should be rounded down to the start of the first block and up to the end
> > of the last block to return [0-8191] (fm_start = 0, fm_length = 8192).
> 
> In fact that should be part of the interface definition, right.  Should
> the returned mapping start at the beginning of the block that contains
> the requsted offset, or at the requested offset itself?  I'd vote for
> the former.
> 
> At some point I should probably write some QA for this thing to test
> various file layouts and make sure we get the "right" answers on all
> filesystems...
> 
> -Eric

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.


  reply	other threads:[~2008-04-22  2:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-18 21:09 [RFC][PATCH] fiemap support for ext3 Josef Bacik
2008-04-21 22:08 ` Andreas Dilger
2008-04-21 22:16   ` Eric Sandeen
2008-04-22  2:33     ` Andreas Dilger [this message]
2008-04-22  0:16   ` Josef Bacik

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=20080422023315.GR2775@webber.adilger.int \
    --to=adilger@sun.com \
    --cc=jbacik@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sandeen@redhat.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.