[haiku-gsoc] Re: [HCD]: Bfs bug #1

2008/6/18 Ingo Weinhold <ingo_weinhold@xxxxxx>:
>
> On 2008-06-18 at 16:01:11 [+0200], Salvatore Benedetto <emitrax@xxxxxxxxx>
> wrote:
>> 2008/6/18 Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
>> > The more interesting question would be why get_vnode() failed in the
>> > first place, though.
>>
>> I investigated this a bit and in the serial log I found before the crash
>> that the vnode is not becoming "unbusy"
>>
>> vnode 3:4260 is not becoming unbusy!
>> vnode 0x9149ff00
>> PANIC: ASSERT FAILED (src/system/kernel/fs/vfs.cpp:801): oldRefCount > 0
>>
>> At first, this seems the reason, but I don't know why this is
>> happening and I'll look into
>> it more.
>
> Just a hint, if you want to track this down: Turn the dprintf()s about the
> vnode not becoming unbusy into an panic()s and add ktrace_printf()s in all
> places where a node is marked busy/unbusy. When the panic is triggered it
> should be easy to find out which thread marked it busy and what it is doing.

Thanks for the hint!

>
> Anyway, we should probably use condition variables for busy vnodes, so that
> we don't run into problems with slow FSs.

Yep, I thought of that. It's better than retrying over and over to get a lock.

>
> CU, Ingo
>
>

The patch attached should prevents the kernel from crashing in this
occasion making
it more error prone. I called the boolean variable fOk. I really
didn't know how else to call
it. Can somebody review it and give me his bless so that I can commit
it please?

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

Index: src/add-ons/kernel/file_systems/bfs/Inode.h
===================================================================
--- src/add-ons/kernel/file_systems/bfs/Inode.h (revision 26005)
+++ src/add-ons/kernel/file_systems/bfs/Inode.h (working copy)
@@ -256,14 +256,16 @@
                Vnode(Volume *volume, ino_t id)
                        :
                        fVolume(volume),
-                       fID(id)
+                       fID(id),
+                       fOk(true)
                {
                }
 
                Vnode(Volume *volume, block_run run)
                        :
                        fVolume(volume),
-                       fID(volume->ToVnode(run))
+                       fID(volume->ToVnode(run)),
+                       fOk(true)
                {
                }
 
@@ -275,12 +277,16 @@
                status_t Get(Inode **_inode)
                {
                        // should we check inode against NULL here? it should 
not be necessary
-                       return get_vnode(fVolume->FSVolume(), fID, (void 
**)_inode);
+                       status_t status
+                               = get_vnode(fVolume->FSVolume(), fID, (void 
**)_inode);
+                       if (status < B_OK)
+                               fOk = false;
+                       return status;
                }
 
                void Put()
                {
-                       if (fVolume)
+                       if (fVolume && fOk)
                                put_vnode(fVolume->FSVolume(), fID);
                        fVolume = NULL;
                }
@@ -293,6 +299,7 @@
        private:
                Volume          *fVolume;
                ino_t           fID;
+               bool            fOk;
 };

Other related posts: