Re: [PATCH 1/2] ext4: Remove arbitrary block value in __es_remove_extent()

["Lukáš Czerner" <lczerner@redhat.com>]

On Thu, 17 Apr 2014, Theodore Ts'o wrote:

> Date: Thu, 17 Apr 2014 12:19:44 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org
> Subject: Re: [PATCH 1/2] ext4: Remove arbitrary block value in
>     __es_remove_extent()
> 
> On Tue, Apr 15, 2014 at 08:08:21PM +0200, Lukas Czerner wrote:
> > In __es_remove_extent() we're storing seemingly arbitrary value
> > 0x7FDEADBEEF into block variable. I assume that the reason is just to
> > initialize the variable before the use because the actual value does not
> > matter at this point.
> > 
> > Just remove the arbitrary value and initialized block variable to zero
> > which is much less suspicious.
> 
> The whole point was for the value to be suspicious.  That way, if
> there is a bug, and we try to use that value, it is a large enough
> value that for most storage devices, we will get an I/O error (because
> the disk isn't that big :-) referencing that block value, and we'll
> have a bit of a hint where that suspicious value might have come from.

Aside from the fact that this is totally undocumented and there is
not even comment on what is that all about in couple of years we
might actually get file systems big enough that this would not be an
I/O error anymore (that might be a bit of a stretch).

But mainly this value is only going to be used if it is delayed
extent or a hole which implies that it has not been mapped and
pblock does not contain anything valid. And if we really screwed it
up and tried to use pblock of extent which is a hole or delayed
extent, then it would not help us anyway since the only place that
we actually set this is when splitting extent on removal.

Now I can see that in ext4_da_map_blocks() we're actually using ~0
value for the pblock which is a bit better I think as long as we're
using this reliably. So I'll resend the patch which will make sure
that we're using ~0 reliably when storin delayed, or hole extents in
the extent status tree. Does that make sense ?

-Lukas

> 
>        	      	     	   		   - Ted