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

2008/6/18 Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
> "Salvatore Benedetto" <emitrax@xxxxxxxxx> wrote:
>> it looks like I hit my first bug (#2400) while unpacking a tarball
>> containing the whole haiku tree
>> (about 480MB compressed) from a usbstick while running in vmware.
>
> Good luck! :-)

I'm planning to do it over and over until it works.

>
> [...]
>> Here is a summary:
>> Index::SetTo() is called, it tries to create a Vnode object
>> Vnode vnode(fVolume, id); thus with this signature Vnode(Volume
>> *volume, ino_t id),
>> but then the destructor gets called.
>>
>> Actually, now that I look at it, it might fail because the Volume
>> constructor fails first.
>> I'm not C++ guru, but I thought that if one of the initialization
>> would fail, the object wouldn't be created without any needs to call
>> the destructor.
>
> It's allocated on the stack - the constructor cannot fail.

Hmm.. then why the destructor is called?

>
>> Anyhow, the ~Vnode gets called, that in turn calls Put()
>>         void Put()
>>        {
>>            if (fVolume)
>>                put_vnode(fVolume->FSVolume(), fID);
>>            fVolume = NULL;
>>        }
>> ...
>>    dec_vnode_ref_count(vnode, false);
>
> While Get() is called in this case, the destructor would only work
> correctly in case get_vnode() succeeded. Looks like I managed to put a
> bug in that mini class Vnode...
> If you want to fix that, please do, otherwise I can fix it on Monday.

I'll definitely try and want to figure out what's going on, and I hope
to fix it before Monday.
It'll help to get familiar with the code.


> The more interesting question would be why get_vnode() failed in the
> first place, though.

I'll look into that.

>
>> By the way, two things about atomic_add:
>> 1. As already stated in the comment above the function, it is in a
>> unusual place
>> src/add-ons/kernel/partitioning_systems/intel/intel.cpp
>
> I don't get that. What is where, and what comment describes it?

in src/add-ons/kernel/partitioning_systems/intel/intel.cpp
// TODO: This doesn't belong here!
// no atomic_add() in the boot loader
#ifdef _BOOT_MODE

inline int32
atomic_add(int32 *a, int32 num)
{
    int32 oldA = *a;
    *a += num;
    return oldA;
}

#endif

>
>> 2. How is it atomic? I don't see where the interrupts are disabled.
>
> At least you're not afraid to ask stupid questions ;-)) (sorry,
> couldn't resist :-))

:-)

> But seriously, if you're interested, have a look at the "Intel IA-32
> Software Developer's Manual".
> In short, the "lock" instruction takes care of making this atomic; it's
> the sole purpose of it.

It was 2 O'clock when I looked at the code but I saw not "lock" whatsoever,
just an inline function and a mutex acquired before calling it. Anyway
I'll defintely
read that manual.

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

Other related posts: