Re: [PATCH] ext2fs and large stores (> 1.5G)

[Ognyan Kulev]

Neal H. Walfield wrote:
> It seems your email client is broken, it does not respect my
> Mail-Copies-To header.

I usually remove all recipients other than mail lists.  I'll think about 
whether this is bad practice.

> Actually, it is worse than this, your locking is quite broken as it
> only covers a few of many cases.  Consider: a function can be using a
> page and have it taken out from underneath it because use_count is
> only used when the block is accessed via disk_image_request (and there
> are a lot of places where you use, e.g. bptr directly).  You need to
> do reference counting on all blocks and it should be done via
> boffs_ptr, bptr_offs et al., not disk_image_*.  Consider,
> record_global_poke for just one example.  As far as I can tell,
> disk_image_{request,release,clear} serve no purpose that should not be
> in the bptr_*, boffs_* family of functions.

Using some kind of locking and unlocking (request/release) of floating 
metablocks (indirect blocks) is crucial.  This guarantees that cached 
block will not be replaced by another one.  Fortunately there are only 
three places where indirect blocks are ever referred: 
getblk.c:block_getblk, getblk.c:ext2_alloc_block (to zero indirect 
block) and truncate.c:trunc_indirect.  All other places can freely use 
b{ptr,offs}* because they deal with metablocks at fixed location that 
won't be replaced suddenly with something else.

Here is the solution[1] to the "record_global_poke" problem: the .dirty 
field is replaced by .dirty_count field.  Now a block in the cache is 
never reused while "use_count || dirty_count".  This forces us to use 
the same amount of disk_cache_clear(block) calls (old name was 
disk_image_clear) as disk_cache_release(block,1).  One problem arises: 
when indirect block is made many times dirty, this will increment 
dirty_count many times, but _pokel_exec will decrement dirty_count only 
once.  To solve it, I made pokel_add to return boolean value that 
indicates whether the passed memory region is already in the pokel.  If 
it is, then disk_cache_clear is called.

[1] http://debian.fmi.uni-sofia.bg/~ogi/hurd/ext3fs/

Unfortunately the new snapshot generates some very rare and 
unreproducable data corruptions.  I'll look at these issues later.

Regards
--
Ognyan Kulev <ogi@fmi.uni-sofia.bg>, "\"Programmer\""