Re: 4 significant issues with amd64 -current

[Sverre Froyen <sverre%viewmark.com@localhost>]

On Monday 12 May 2008, Sverre Froyen wrote:
> On Sunday 11 May 2008, Scott Ellis wrote:
>
> [snip]
>
> > Issue #3 (paraphrased below):
> >
> > breakpoint() at netbsd:breakpoint+0x5
> > panic() at netbsd:panic+0x253
> > lockdebug_wantlock() at netbsd:lockdebug_wantlock
> > rw_vector_enter() at netbsd:rw_vector_enter+0x237
> > prop_dictionary_iterator() at netbsd:prop_dictionary_iterator+0x67
> > _prop_dictionary_externalize() at
> > netbsd:_prop_dictionary_externalize+0xa2 prop_dictionary_externalize() at
> > netbsd:prop_dictionary_externalize+0x30 _prop_object_copyout_ioctl() at
> > netbsd:_prop_object_copyout_ioctl+0x121 sysmonioctl_power() at
> > netbsd:sysmonioctl_power+0xf2
> > VOP_IOCTL() at netbsd:VOP_IOCTL+0x64
> > vn_ioctl() at netbsd:vn_ioctl+0x62
> > sys_ioctl() at netbsd:sys_ioctl+0x11e
> > syscall() at netbsd:syscall+0x8f
> > db{1}>
>
> I suspect this is the same issue as in:
>
> http://mail-index.netbsd.org/current-users/2008/05/06/msg002274.html
> http://mail-index.netbsd.org/current-users/2008/05/06/msg002287.html
>
> From I quick code inspection, it looks like _prop_dictionary_externalize
> locks pd->pd_rwlock and calls prop_dictionary_iterator before unlocking.
> prop_dictionary_iterator's attempt to acquire the lock then fails.  Perhaps
> prop_dictionary_iterator needs to do the _PROP_RWLOCK_TRYRDLOCK dance?

FWIW, the attached diff "fixes" my shutdown panic and also (at least on my 
system) the envstat panic.  There is probably a better way to resolve the 
locking issue, however.

Sverre

Index: common/lib/libprop/prop_array.c
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_array.c,v
retrieving revision 1.16
diff -u -r1.16 prop_array.c
--- common/lib/libprop/prop_array.c     7 May 2008 10:16:41 -0000       1.16
+++ common/lib/libprop/prop_array.c     13 May 2008 13:35:01 -0000
@@ -497,6 +497,7 @@
 prop_array_iterator(prop_array_t pa)
 {
        struct _prop_array_iterator *pai;
+       bool acquired;
 
        if (! prop_object_is_array(pa))
                return (NULL);
@@ -508,9 +509,14 @@
        pai->pai_base.pi_reset = _prop_array_iterator_reset;
        prop_object_retain(pa);
        pai->pai_base.pi_obj = pa;
-       _PROP_RWLOCK_RDLOCK(pa->pa_rwlock);
+
+       _PROP_ASSERT(prop_object_is_array(pa));
+       acquired = _PROP_RWLOCK_TRYRDLOCK(pa->pa_rwlock);
+       _PROP_RWLOCK_OWNED(pa->pa_rwlock);
        _prop_array_iterator_reset(pai);
-       _PROP_RWLOCK_UNLOCK(pa->pa_rwlock);
+       if (acquired) {
+               _PROP_RWLOCK_UNLOCK(pa->pa_rwlock);
+       }
 
        return (&pai->pai_base);
 }
Index: common/lib/libprop/prop_dictionary.c
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_dictionary.c,v
retrieving revision 1.28
diff -u -r1.28 prop_dictionary.c
--- common/lib/libprop/prop_dictionary.c        7 May 2008 10:16:41 -0000       
1.28
+++ common/lib/libprop/prop_dictionary.c        13 May 2008 13:35:01 -0000
@@ -760,6 +760,7 @@
 prop_dictionary_iterator(prop_dictionary_t pd)
 {
        struct _prop_dictionary_iterator *pdi;
+       bool acquired;
 
        if (! prop_object_is_dictionary(pd))
                return (NULL);
@@ -771,9 +772,14 @@
        pdi->pdi_base.pi_reset = _prop_dictionary_iterator_reset;
        prop_object_retain(pd);
        pdi->pdi_base.pi_obj = pd;
-       _PROP_RWLOCK_RDLOCK(pd->pd_rwlock);
+
+       _PROP_ASSERT(prop_object_is_dictionary(pd));
+       acquired = _PROP_RWLOCK_TRYRDLOCK(pd->pd_rwlock);
+       _PROP_RWLOCK_OWNED(pd->pd_rwlock);
        _prop_dictionary_iterator_reset(pdi);
-       _PROP_RWLOCK_UNLOCK(pd->pd_rwlock);
+       if (acquired) {
+               _PROP_RWLOCK_UNLOCK(pd->pd_rwlock);
+       }
 
        return (&pdi->pdi_base);
 }