Re: CVS commit: src/sys/kern

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

David Laight <david%l8s.co.uk@localhost> wrote:
> On Sun, Jan 22, 2012 at 03:48:51AM +0000, Mindaugas Rasiukevicius wrote:
> > Module Name:        src
> > Committed By:       rmind
> > Date:               Sun Jan 22 03:48:51 UTC 2012
> > 
> > Modified Files:
> >     src/sys/kern: kern_fileassoc.c
> > 
> > Log Message:
> > fileassoc_file_delete: pre-check whether fileassoc was used and thus
> > avoid acquiring kernel-lock, which damages sys_unlink() performance.
> 
> Erm... looking at the file the locking in there looks decidedly dubious.
> 
> 1) There doesn't seem to be any locking on the hash table.
> 2) It isn't clear why the KERNEL_LOCK was acquired in one specific path.
> 3) If fileassoc_file_delete() is expected to remove all references for
>    a vnode, something external must have forced the state of the vnode.
>    (otherwise the stuff might be added - inc. global init - while this
>    code is being called.

Yes, locking issues are known in this code (see e.g. PR/35351) and I would
say fileassoc(9) should be disabled by default while this is resolved.

However, I do not really have much interest in fixing fileassoc(9), so my
only concern was to fix performance degradation of unlink(2) due to it.

-- 
Mindaugas