Re: CVS commit: src/sys

[Antti Kantee <pooka%NetBSD.org@localhost>]

On Mon May 04 2009 at 15:06:56 +0000, YAMAMOTO Takashi wrote:
> >> > Log Message:
> >> > when freeing cn_pnbuf, make it NULL if DIAGNOSTIC.
> >> 
> >> Even better if PNBUF_PUT() does it so that we catch all violations?
> 
> i don't like such a "magic" macro.

I don't like namei ;)

> these #ifdef indicate that cn_pnbuf == NULL is not a valid test for HASBUF.
> ie. this commit is not intended to modify the api.

Also, I think that HASBUF should be removed and we could use a NULL
pointer to signal "no buffer".  Since PNBUF_PUT() does not do anything to
HASBUF, currently it is impossible to determine if the buffer exists or
not after calling a VOP.  This is a real problem for libp2k.  I considered
using SAVESTART, but since that's not a sure way (cf. tmpfs ;) and likely
causes even more problems, I gave up.

I guess puffs now panics on DIAGNOSTIC kernels (see kern/38188).
I should've fixed it "a little" earlier, but somehow I didn't just have
the time.  I whipped up a patch, but due to the abovementioned issues
with HASBUF/cn_pnbuf I can't currently test it in userland.  And I
really really don't have time to play kernel games today (or tomorrow).
If someone can test it, please commit it.

Index: puffs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/puffs/puffs_vnops.c,v
retrieving revision 1.131
diff -p -u -r1.131 puffs_vnops.c
--- puffs_vnops.c       26 Nov 2008 20:17:33 -0000      1.131
+++ puffs_vnops.c       4 May 2009 15:29:10 -0000
@@ -581,6 +581,13 @@ puffs_vnop_lookup(void *v)
                cnp->cn_consume = MIN(lookup_msg->pvnr_cn.pkcn_consume,
                    strlen(cnp->cn_nameptr) - cnp->cn_namelen);
 
+       /*
+        * We need the name in remove and rmdir (well, rename too, but
+        * SAVESTART takes care of that)
+        */
+       if (cnp->cn_nameiop == DELETE)
+               cnp->cn_flags |= SAVENAME;
+
  out:
        if (cnp->cn_flags & ISDOTDOT)
                vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
@@ -1439,6 +1446,8 @@ puffs_vnop_remove(void *v)
        RELEPN_AND_VP(vp, pn);
 
        error = checkerr(pmp, error, __func__);
+       if (error || (cnp->cn_flags & SAVESTART) == 0)
+               PNBUF_PUT(cnp->cn_pnbuf);
        return error;
 }
 
@@ -1544,6 +1553,9 @@ puffs_vnop_rmdir(void *v)
        RELEPN_AND_VP(dvp, dpn);
        RELEPN_AND_VP(vp, pn);
 
+       if (error || (cnp->cn_flags & SAVESTART) == 0)
+               PNBUF_PUT(cnp->cn_pnbuf);
+
        return error;
 }