Re: [Ocfs2-devel] [PATCH 1/2] fiemap: Add new extent flag FIEMAP_EXTENT_SHARED

Re: [Ocfs2-devel] [PATCH 1/2] fiemap: Add new extent flag FIEMAP_EXTENT_SHARED

[Mark Fasheh]

On Wed, Oct 21, 2009 at 06:05:08PM -0700, Sunil Mushran wrote:
> Patch adds flag FIEMAP_EXTENT_SHARED to denote extents that are
> shared with other inodes.

This looks pretty sane to me. Thanks Sunil.


> Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>

Acked-by: Mark Fasheh <mfasheh@suse.com>
	--Mark

--
Mark Fasheh

Re: [Ocfs2-devel] [PATCH 1/2] fiemap: Add new extent flag FIEMAP_EXTENT_SHARED

[Jamie Lokier]

Mark Fasheh wrote:
> On Wed, Oct 21, 2009 at 06:05:08PM -0700, Sunil Mushran wrote:
> > Patch adds flag FIEMAP_EXTENT_SHARED to denote extents that are
> > shared with other inodes.
> 
> This looks pretty sane to me. Thanks Sunil.

It might be better to have a flag FIEMAP_EXTENT_NOT_SHARED.

Apps which need to know the data is not shared can't safely check
!(flags & FIEMAP_EXTENT_SHARED) because of old kernels which don't set
the flag.

-- Jamie

Re: [Ocfs2-devel] [PATCH 1/2] fiemap: Add new extent flag FIEMAP_EXTENT_SHARED

[Sunil Mushran]

Jamie Lokier wrote:
> It might be better to have a flag FIEMAP_EXTENT_NOT_SHARED.
>
> Apps which need to know the data is not shared can't safely check
> !(flags & FIEMAP_EXTENT_SHARED) because of old kernels which don't set
> the flag.

I'm probably not understanding your point. Won't the older kernels not set
FIEMAP_EXTENT_NOT_SHARED too.

If this is important, we could always add this flag in the stable kernel
trees. Both sharing of extents and fiemap are relatively new to the kernel.

Re: [Ocfs2-devel] [PATCH 1/2] fiemap: Add new extent flag FIEMAP_EXTENT_SHARED

[Brad Boyer]

On Mon, Oct 26, 2009 at 10:29:36AM -0700, Sunil Mushran wrote:
> Jamie Lokier wrote:
> >It might be better to have a flag FIEMAP_EXTENT_NOT_SHARED.
> >
> >Apps which need to know the data is not shared can't safely check
> >!(flags & FIEMAP_EXTENT_SHARED) because of old kernels which don't set
> >the flag.
> 
> I'm probably not understanding your point. Won't the older kernels not set
> FIEMAP_EXTENT_NOT_SHARED too.
> 
> If this is important, we could always add this flag in the stable kernel
> trees. Both sharing of extents and fiemap are relatively new to the kernel.

I think the concern is that we are going to be getting "wrong" data when
using an old kernel, and the question is which type of error is safer. If
the flag is FIEMAP_EXTENT_SHARED, then programs running against an old
kernel may think that shared extents are not shared. If the flag is
instead FIEMAP_EXTENT_NOT_SHARED, then the program may think an extent is
shared when it is not.

The real question is which information is more important, to know that the
extent is shared or if the extent definitely is not shared. I would think
that having FIEMAP_EXTENT_SHARED is more consistent with the rest of the
defined bits, particularly since the program couldn't actually do anything
that relies completely on this data anyway since it could change at any
moment. It's not like the results of a map are static.

	Brad Boyer
	flar@allandria.com