Re: kern/35351: fileassoc lacks locking

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR kern/35351; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/35351: fileassoc lacks locking
Date: Sun, 22 Jan 2012 18:28:25 +0000

 forward two more mails to gnats
 
    ------
 
 From: David Laight <david%l8s.co.uk@localhost>
 To: Elad Efrat <elad%NetBSD.org@localhost>
 Cc: netbsd-bugs%netbsd.org@localhost
 Subject: Re: Fwd: PR/35351: fileassoc lacks locking
 Date: Sun, 22 Jan 2012 17:35:43 +0000
 Mail-Followup-To: Elad Efrat <elad%NetBSD.org@localhost>, 
netbsd-bugs%netbsd.org@localhost
 
 On Sun, Jan 22, 2012 at 11:39:29AM -0500, Elad Efrat wrote:
 > Resend to a different address.
 > 
 > ---------- Forwarded message ----------
 > From: Elad Efrat <elad%netbsd.org@localhost>
 > Date: Wed, Jan 18, 2012 at 12:48 PM
 > Subject: PR/35351: fileassoc lacks locking
 > To: gnats%netbsd.org@localhost
 > 
 > 
 > Attached is a diff that adds locking to fileassoc(9). It fixes two
 > panics that I can easily trigger through Veriexec.
 
 A quick scan makes me think it has too many mutex and condvar.
 
        David
 
 -- 
 David Laight: david%l8s.co.uk@localhost
 
 
 From: Matthew Mondor <mm_lists%pulsar-zone.net@localhost>
 To: netbsd-bugs%netbsd.org@localhost
 Subject: Re: PR/35351: fileassoc lacks locking
 Date: Sun, 22 Jan 2012 12:38:47 -0500
 
 On Sun, 22 Jan 2012 11:39:29 -0500
 Elad Efrat <elad%NetBSD.org@localhost> wrote:
 
 > Attached is a diff that adds locking to fileassoc(9). It fixes two
 > panics that I can easily trigger through Veriexec.
 > 
 > A version of it (earlier? can't remember) was discussed here:
 > 
 > ? ?http://mail-index.netbsd.org/tech-kern/2009/12/26/msg006703.html
 > 
 > I don't find the "it's not perfect" line of reasoning convincing, so
 > my recommendation is to check it in. It's filed so it doesn't get
 > lost.
 
 The only part that's not totally clear to me (from the diff alone) is
 when a hash table is rehashed, where/when the old table gets freed?
 
 +      /* Free the old hash table, and plug in the new one. */
        hashdone(tbl->tbl_hash, HASH_LIST, tbl->tbl_mask);
 -      specificdata_fini(fileassoc_domain, &tbl->tbl_data);
 -      kmem_free(tbl, sizeof(*tbl));
 +      tbl->tbl_hash = new_hash;
 +      tbl->tbl_nslots = new_nslots;
 +      tbl->tbl_mask = new_mask;
 
 Of course I may have overlooked something too.
 
 Thanks,
 -- 
 Matt