From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: David Sterba <dsterba@suse.cz>,
linux-fsdevel@vger.kernel.org, adilger@dilger.ca,
linux-nilfs@vger.kernel.org, mfasheh@suse.com, xfs@oss.sgi.com,
hch@infradead.org, linux-btrfs@vger.kernel.org,
viro@zeniv.linux.org.uk, linux-ext4@vger.kernel.org,
ocfs2-devel@oss.oracle.com
Subject: Re: [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag
Date: Fri, 13 Dec 2013 03:06:08 -0800 [thread overview]
Message-ID: <20131213110608.GC14884@infradead.org> (raw)
In-Reply-To: <20131212232443.GL31386@dastard>
On Fri, Dec 13, 2013 at 10:24:43AM +1100, Dave Chinner wrote:
> I'd prefer to just see the new physical length field always filled
> out, regardless of whether it is a compressed extent or not. In
> terms of backwards compatibility to userspace, it makes no
> difference because the value of reserved/unused fields is undefined
> by the API. Yes, the implementation zeros them, but there's nothing
> in the documentation that says "reserved fields must be zero".
> Hence I think we should just set it for every extent.
>
> >From the point of view of the kernel API (fiemap_fill_next_extent),
> passing the physical extent size in the "len" parameter for normal
> extents, then passing 0 for the "physical length" makes absolutely
> no sense.
I tend to agree, but the additional complication here is that this is
a change to an existing API. We'd need another HAVE_PHYS_LEN flag for
non-compressed extents so that userspace can rely on it. Given that
it's only useful for that case I think the userspace API introduced
is the best we can get.
I think however that we should always pass the phys_len argument in the
kernel API just to make it less confusing.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: adilger@dilger.ca, linux-nilfs@vger.kernel.org, mfasheh@suse.com,
David Sterba <dsterba@suse.cz>,
xfs@oss.sgi.com, hch@infradead.org, ocfs2-devel@oss.oracle.com,
viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag
Date: Fri, 13 Dec 2013 03:06:08 -0800 [thread overview]
Message-ID: <20131213110608.GC14884@infradead.org> (raw)
In-Reply-To: <20131212232443.GL31386@dastard>
On Fri, Dec 13, 2013 at 10:24:43AM +1100, Dave Chinner wrote:
> I'd prefer to just see the new physical length field always filled
> out, regardless of whether it is a compressed extent or not. In
> terms of backwards compatibility to userspace, it makes no
> difference because the value of reserved/unused fields is undefined
> by the API. Yes, the implementation zeros them, but there's nothing
> in the documentation that says "reserved fields must be zero".
> Hence I think we should just set it for every extent.
>
> >From the point of view of the kernel API (fiemap_fill_next_extent),
> passing the physical extent size in the "len" parameter for normal
> extents, then passing 0 for the "physical length" makes absolutely
> no sense.
I tend to agree, but the additional complication here is that this is
a change to an existing API. We'd need another HAVE_PHYS_LEN flag for
non-compressed extents so that userspace can rely on it. Given that
it's only useful for that case I think the userspace API introduced
is the best we can get.
I think however that we should always pass the phys_len argument in the
kernel API just to make it less confusing.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: David Sterba <dsterba@suse.cz>,
linux-fsdevel@vger.kernel.org, adilger@dilger.ca,
linux-nilfs@vger.kernel.org, mfasheh@suse.com, xfs@oss.sgi.com,
hch@infradead.org, linux-btrfs@vger.kernel.org,
viro@zeniv.linux.org.uk, linux-ext4@vger.kernel.org,
ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag
Date: Fri, 13 Dec 2013 03:06:08 -0800 [thread overview]
Message-ID: <20131213110608.GC14884@infradead.org> (raw)
In-Reply-To: <20131212232443.GL31386@dastard>
On Fri, Dec 13, 2013 at 10:24:43AM +1100, Dave Chinner wrote:
> I'd prefer to just see the new physical length field always filled
> out, regardless of whether it is a compressed extent or not. In
> terms of backwards compatibility to userspace, it makes no
> difference because the value of reserved/unused fields is undefined
> by the API. Yes, the implementation zeros them, but there's nothing
> in the documentation that says "reserved fields must be zero".
> Hence I think we should just set it for every extent.
>
> >From the point of view of the kernel API (fiemap_fill_next_extent),
> passing the physical extent size in the "len" parameter for normal
> extents, then passing 0 for the "physical length" makes absolutely
> no sense.
I tend to agree, but the additional complication here is that this is
a change to an existing API. We'd need another HAVE_PHYS_LEN flag for
non-compressed extents so that userspace can rely on it. Given that
it's only useful for that case I think the userspace API introduced
is the best we can get.
I think however that we should always pass the phys_len argument in the
kernel API just to make it less confusing.
next prev parent reply other threads:[~2013-12-13 11:06 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-12 15:25 [PATCH 0/4 v3] fiemap: introduce EXTENT_DATA_COMPRESSED flag David Sterba
2013-12-12 15:26 ` [Ocfs2-devel] " David Sterba
2013-12-12 15:25 ` David Sterba
2013-12-12 15:25 ` [PATCH 1/4 v3] fiemap: fix comment at EXTENT_DATA_ENCRYPTED David Sterba
2013-12-12 15:25 ` [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag David Sterba
2013-12-12 15:26 ` [Ocfs2-devel] " David Sterba
2013-12-12 15:25 ` David Sterba
2013-12-12 23:24 ` Dave Chinner
2013-12-12 23:25 ` [Ocfs2-devel] " Dave Chinner
2013-12-12 23:24 ` Dave Chinner
2013-12-12 23:24 ` Dave Chinner
2013-12-13 0:02 ` Andreas Dilger
2013-12-13 0:02 ` [Ocfs2-devel] " Andreas Dilger
2013-12-13 0:02 ` Andreas Dilger
2013-12-13 0:02 ` Andreas Dilger
2013-12-13 1:22 ` Dave Chinner
2013-12-13 1:23 ` [Ocfs2-devel] " Dave Chinner
2013-12-13 1:22 ` Dave Chinner
2013-12-16 16:49 ` David Sterba
2013-12-16 16:49 ` [Ocfs2-devel] " David Sterba
2013-12-16 16:49 ` David Sterba
2014-07-17 6:07 ` Andreas Dilger
2014-07-17 6:07 ` [Ocfs2-devel] " Andreas Dilger
2014-07-17 6:07 ` Andreas Dilger
2014-07-17 6:07 ` Andreas Dilger
2014-07-24 19:22 ` David Sterba
2014-07-24 19:22 ` [Ocfs2-devel] " David Sterba
2014-07-24 19:22 ` David Sterba
2014-07-24 19:22 ` David Sterba
2014-07-24 22:34 ` Andreas Dilger
2014-07-24 22:34 ` [Ocfs2-devel] " Andreas Dilger
2014-07-24 22:34 ` Andreas Dilger
2014-07-25 6:20 ` Rohan Puri
2014-07-25 6:20 ` [Ocfs2-devel] " Rohan Puri
2014-07-25 6:20 ` Rohan Puri
2014-07-25 6:20 ` Rohan Puri
2014-07-28 16:49 ` [Ocfs2-devel] " David Sterba
2014-07-28 16:49 ` David Sterba
2013-12-13 11:06 ` Christoph Hellwig [this message]
2013-12-13 11:06 ` Christoph Hellwig
2013-12-13 11:06 ` Christoph Hellwig
2013-12-12 15:26 ` [PATCH 3/4 v3] btrfs: set FIEMAP_EXTENT_DATA_COMPRESSED for compressed extents David Sterba
2013-12-12 22:20 ` Andreas Dilger
2013-12-12 15:26 ` [PATCH 4/4 v3] Documentation/fiemap: Document the DATA_COMPRESSED flag David Sterba
2013-12-12 22:22 ` [PATCH 0/4 v3] fiemap: introduce EXTENT_DATA_COMPRESSED flag Andreas Dilger
2013-12-12 22:22 ` [Ocfs2-devel] " Andreas Dilger
2013-12-12 22:22 ` Andreas Dilger
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=20131213110608.GC14884@infradead.org \
--to=hch@infradead.org \
--cc=adilger@dilger.ca \
--cc=david@fromorbit.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nilfs@vger.kernel.org \
--cc=mfasheh@suse.com \
--cc=ocfs2-devel@oss.oracle.com \
--cc=viro@zeniv.linux.org.uk \
--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.