[haiku-gsoc] Re: BFS bug #2

2008/6/28 Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
> "Salvatore Benedetto" <emitrax@xxxxxxxxx> wrote:
>> I'm fixing it now. Let me know if you have any suggestions about how
>> to fix it.
>
> On second thought, it's not that easy to do what I just suggested;
> Inode::_MakeSpaceForSmallData() calls Inode::Remove(), and it's
> impractical to always lock the volume lock before that.

For that matter even Inode::_RemoveAttributes calls Inode::Remove().
The public methods that calls those functions that eventually
call Inode::Remove are WriteAttribute, RemoveAttribute, SetName,
CreateName and probably others. So I don't think it's just a matter of
removing the Volume lock in the Inode::Remove function.

One thing we probably agree is that when calling Inode::Remove the
volume lock should be held, so I'm wondering if the IsLocked method
is available in that context, so we could do something like
if (!fVolume->isLocked()) {
TRACE(("Inode::Remove: called without volume lock held\n"));
return B_ERROR;
}

> Instead, I have two different suggestions:
> 1) the simple one: get rid of the volume lock in remove/rename, and
> start the transaction in rename earlier (at the place where the volume
> is locked now)

You mean to totally remove the volume lock in the bfs_rename and bfs_unlink?
So Transaction would get locked first, and then let the Inode get the
volume lock?

> 2) the needlessly complex one: make the journal lock an r/w lock, and
> make rename read lock it before actually starting a transaction (a
> transaction would always write lock).
>
> IMO 1) is the way to go, since 2) would only be an optimization for the
> error case, and can thus be neglected.

Just to make things clear, which lock are you saying should be held first?
Journaled lock, or volume lock?

:-)

>
> Bye,
>   Axel.
>


Regards,
-- 
Salvatore Benedetto (a.k.a. emitrax)
Student of Computer Engineer
University of Pisa
www.haiku-os.it

Other related posts: