Re: CVS commit: src/sys/kern

2012-01-22 Thread David Laight
On Sun, Jan 22, 2012 at 03:48:51AM +, 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.

OTOH I've actually NFI what the code in this file is for!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2012-01-22 Thread Mindaugas Rasiukevicius
David Laight da...@l8s.co.uk wrote:
 On Sun, Jan 22, 2012 at 03:48:51AM +, 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


Re: CVS commit: src

2012-01-22 Thread Izumi Tsutsui
 Module Name:  src
 Committed By: christos
 Date: Sun Jan 22 18:36:19 UTC 2012
 
 Modified Files:
   src/bin/csh: csh.c
   src/distrib/sets/lists/base: ad.mips64eb ad.mips64el md.amd64
   md.sparc64 shl.mi
   src/distrib/sets/lists/comp: ad.mips64eb ad.mips64el md.amd64
   md.sparc64 shl.mi
   src/include: stdio.h
   src/lib/libc: shlib_version
   src/lib/libc/compat: Makefile Makefile.inc
   src/lib/libc/stdio: fgetpos.c findfp.c fmemopen.c fopen.c freopen.c
   fseek.3 fseeko.c fsetpos.c ftell.c ftello.c funopen.3 funopen.c
   local.h stdio.c
   src/tests/fs/nfs/nfsservice: mountd.c
 Added Files:
   src/lib/libc/compat/include: stdio.h
   src/lib/libc/compat/stdio: Makefile.inc compat_fgetpos.c
   compat_fsetpos.c
 
 Log Message:
 From tsutsui@: make fpos_t a complex object that keeps track of the parse

Not tsutsui@ but tnozaki@ .

---
Izumi Tsutsui