Re: locking patches for ufs_rename

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Mon, Sep 28, 2009 at 05:09:54AM +0000, David Holland wrote:
> The attached patches correct the locking in ufs_rename and
> ufs_wapbl_rename. They have been tested lightly and work in the simple
> cases; I will beat on them harder before committing.
> 
> The first patch is naming reform to abolish the alphabet soup of tdvp,
> tvp, tdp, ip, xp, and so forth. This has been checked to make only
> very minor changes to the generated .o files.
> 
> The second patch tidies up some formatting problems left behind by the
> longer symbol names in the first patch; this makes slightly less minor
> changes to the generated .o files.
> 
> The third patch rototills ufs_rename and ufs_wapbl_rename to behave
> sensibly with respect to locking, or at least as sensibly as is
> currently possible without reworking the VOP_LOOKUP locking model.
> 
> Since I've pretty much rewritten the whole thing the diffs may not be
> the easiest to read, so I've also included copies of the updated
> functions, plus the new ufs_parentcheck() that replaces ufs_checkpath().
> 
> Any testing or auditing/reviewing would be appreciated.

I found 2 problems that I send to david in private mail.
I did a backport of these patches to netbsd-5. Running a rsync+cvs update
on a dual-CPU system, I got:
Reader / writer lock error: lockdebug_wantlock: locking against myself

lock address : 0x00000000d7a3bd9c type     :     sleep/adaptive
initialized  : 0x00000000c036f970
shared holds :                  0 exclusive:                  1
shares wanted:                  0 exclusive:                  1
current cpu  :                  0 last held:                  0
current lwp  : 0x00000000cea78d20 last held: 0x00000000cea78d20
last locked  : 0x00000000c036d647 unlocked : 0x00000000c036d6e8
owner/count  : 0x00000000cea78d20 flags    : 0x000000000000000c

Turnstile chain at 0xc06ed7a0.
=> No active turnstile for this lock.
0xc036d647 is in vlockmgr 
(/dsk/l1/misc/bouyer/netbsd-5/src/sys/kern/vfs_subr.c:2810).
2805                    }
2806                    rw_enter(&vl->vl_lock, RW_READER);
2807                    return 0;
2808
2809            case LK_EXCLUSIVE:
2810                    if (rw_tryenter(&vl->vl_lock, RW_WRITER)) {
2811                            return 0;
2812                    }
2813                    if ((vl->vl_canrecurse || (flags & LK_CANRECURSE) != 0) 
&&
2814                        rw_write_held(&vl->vl_lock)) {
(gdb) l *(0x00000000c036d6e8)
0xc036d6e8 is in vlockmgr 
(/dsk/l1/misc/bouyer/netbsd-5/src/sys/kern/vfs_subr.c:2836).
2831                    return 0;
2832
2833            default:
2834                    panic("vlockmgr: flags %x", flags);
2835            }
2836    }

Unfortunably I couldn't get much from ddb:
db{0}> tr/a cea78d20
trace: pid 220 lid 1 at 0xce348d2c
__cpu_simple_lock(c2dd3000,0,c01002a7,0,c01002a7,0,0,0,0,0) at 
netbsd:__cpu_simple_lock+0xd

And couldn't get a core dump.

-- 
Manuel Bouyer, LIP6, Universite Paris VI.           
Manuel.Bouyer%lip6.fr@localhost
     NetBSD: 26 ans d'experience feront toujours la difference
--