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

[Theodore Ts'o <tytso@mit.edu>]

On Fri, Apr 18, 2014 at 11:22:12AM +0200, Lukáš Czerner wrote:
> 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).

Well, do you have some suggestions about what might be a good place to
document something like this?  My assumption is that it's something
that would be used by developers after a bug had been reported, so
presumably it would be someplace in the source code.

And I used "0x7FDEADBEEF" deliberately so that it would be a extremely
unlikely we would have file systems that big (we're approximately 512
PB, and honestly, if we had fixed all of the scaling limits such that
it was sane to think someone would want to be using ext4 with a file
system that big --- well, that would be a very nice problem to have
:-)

> 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 ?

So the technique that we're using in mballoc.c is that we use
different illegal flag values depending on where the bad value was
introduced:

% grep "debug value" fs/ext4/mballoc.c
	ex.fe_logical = 0xDEADFA11; /* debug value */
		ex.fe_logical = 0xDEADC0DE; /* debug value */
				ex.fe_logical = 0xDEADF00D; /* debug value */

I think it might be useful to do the same for the physical blocks in
the extent_status tree.

	  				- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html