Re: CVS commit: src/sys

2009-05-16 Thread Andrew Doran
On Sat, May 16, 2009 at 08:29:54AM +, YAMAMOTO Takashi wrote:

 Modified Files:
   src/sys/kern: vfs_subr.c
   src/sys/sys: vnode.h
 
 Log Message:
 put a flag bit into v_usecount to prevent vtryget during getcleanvnode.
 this fixes the following deadlock.

Thanks! How about this to avoid potential sign compare issues? I forgot
to e-mail you about it earlier.

@@ -252,8 +252,8 @@ typedef struct vnode vnode_t;
 /*
  * v_usecount; see the comment in vfs_subr.c
  */
-#defineVC_XLOCK0x8000
-#defineVC_MASK 0x7fff
+#defineVC_XLOCK0x4000
+#defineVC_MASK (0x  ~VC_XLOCK)


Re: CVS commit: src/sys

2009-05-16 Thread Manuel Bouyer
On Sat, May 16, 2009 at 08:29:54AM +, YAMAMOTO Takashi wrote:
 Module Name:  src
 Committed By: yamt
 Date: Sat May 16 08:29:54 UTC 2009
 
 Modified Files:
   src/sys/kern: vfs_subr.c
   src/sys/sys: vnode.h
 
 Log Message:
 put a flag bit into v_usecount to prevent vtryget during getcleanvnode.
 this fixes the following deadlock.
 
   a thread doing getcleanvnode:
   pick a vnode
   acqure v_interlock
   v_usecount++
   call vclean
 
   now, another thread doing cache_lookup:
   picks the vnode
   vtryget succeed
   vn_lock succeed
 
   now in vclean:
   set VI_XLOCK (too late to be noticed by the competing thread)
   wait on the vnode lock (this might violate locking order)
 
 the use of a flag bit was suggested by Andrew Doran.  PR/41374.

Hi,
do you think it could also help with kern/41417 ?
cache_lookup seems also to be involved in this deadlock ...

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--


Re: CVS commit: src/sys

2009-05-16 Thread YAMAMOTO Takashi
hi,

 On Sat, May 16, 2009 at 08:29:54AM +, YAMAMOTO Takashi wrote:
 
 Modified Files:
  src/sys/kern: vfs_subr.c
  src/sys/sys: vnode.h
 
 Log Message:
 put a flag bit into v_usecount to prevent vtryget during getcleanvnode.
 this fixes the following deadlock.
 
 Thanks! How about this to avoid potential sign compare issues?

i have no problem with it.
but does it make much sense given that v_usecount is unsigned?

YAMAMOTO Takashi

 I forgot
 to e-mail you about it earlier.
 
 @@ -252,8 +252,8 @@ typedef struct vnode vnode_t;
  /*
   * v_usecount; see the comment in vfs_subr.c
   */
 -#defineVC_XLOCK0x8000
 -#defineVC_MASK 0x7fff
 +#defineVC_XLOCK0x4000
 +#defineVC_MASK (0x  ~VC_XLOCK)


Re: CVS commit: src/sys

2009-05-16 Thread YAMAMOTO Takashi
hi,

 Hi,
 do you think it could also help with kern/41417 ?
 cache_lookup seems also to be involved in this deadlock ...

it seems like a different problem.

YAMAMOTO Takashi


Re: CVS commit: src/sys

2009-05-16 Thread Manuel Bouyer
On Sat, May 16, 2009 at 11:14:49AM +, YAMAMOTO Takashi wrote:
 hi,
 
  Hi,
  do you think it could also help with kern/41417 ?
  cache_lookup seems also to be involved in this deadlock ...
 
 it seems like a different problem.

Would have been too easy ...
thanks

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--


Re: CVS commit: src/sys/kern

2009-05-16 Thread David Laight
On Sat, May 16, 2009 at 12:02:00PM +, YAMAMOTO Takashi wrote:
 Module Name:  src
 Committed By: yamt
 Date: Sat May 16 12:02:00 UTC 2009
 
 Modified Files:
   src/sys/kern: init_sysctl.c
 
 Log Message:
 sysctl_doeproc:
   - simplify.
   - KERN_PROC: fix possible stale proc pointer dereference.
   - KERN_PROC: don't do copyout with proc_lock held.

IIRC this used to work because it locked the userspace buffer into
physical memory earlier.
I've not looked at the change, but if you release proc_lock it is
very difficult to ensure you see every process [1], and that the
count of processes is correct.

[1] consider what happens if the proc table has to be extended,
or when a process is in a fork/exit loop.

David

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


Re: CVS commit: src/sys/kern

2009-05-16 Thread YAMAMOTO Takashi
hi,

 On Sat, May 16, 2009 at 12:02:00PM +, YAMAMOTO Takashi wrote:
 Module Name: src
 Committed By:yamt
 Date:Sat May 16 12:02:00 UTC 2009
 
 Modified Files:
  src/sys/kern: init_sysctl.c
 
 Log Message:
 sysctl_doeproc:
  - simplify.
  - KERN_PROC: fix possible stale proc pointer dereference.
  - KERN_PROC: don't do copyout with proc_lock held.
 
 IIRC this used to work because it locked the userspace buffer into
 physical memory earlier.

vslock stuff has always been broken.

 I've not looked at the change,

i guess you should look at the change.

 but if you release proc_lock it is
 very difficult to ensure you see every process [1], and that the
 count of processes is correct.
 
 [1] consider what happens if the proc table has to be extended,
 or when a process is in a fork/exit loop.

right.
is it a problem?

YAMAMOTO Takashi

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


Re: CVS commit: src/sys

2009-05-16 Thread Alan Barrett
On Sat, 16 May 2009, Andrew Doran wrote:
 Thanks! How about this to avoid potential sign compare issues? I forgot
 to e-mail you about it earlier.
 
 @@ -252,8 +252,8 @@ typedef struct vnode vnode_t;
  /*
   * v_usecount; see the comment in vfs_subr.c
   */
 -#defineVC_XLOCK0x8000
 -#defineVC_MASK 0x7fff
 +#defineVC_XLOCK0x4000
 +#defineVC_MASK (0x  ~VC_XLOCK)

It may be easier and safer to use 0x7fffUL.

--apb (Alan Barrett)


Re: CVS commit: src/sys

2009-05-16 Thread David Young
On Sat, May 16, 2009 at 10:05:36PM +0300, Alan Barrett wrote:
 On Sat, 16 May 2009, Andrew Doran wrote:
  Thanks! How about this to avoid potential sign compare issues? I forgot
  to e-mail you about it earlier.
  
  @@ -252,8 +252,8 @@ typedef struct vnode vnode_t;
   /*
* v_usecount; see the comment in vfs_subr.c
*/
  -#defineVC_XLOCK0x8000
  -#defineVC_MASK 0x7fff
  +#defineVC_XLOCK0x4000
  +#defineVC_MASK (0x  ~VC_XLOCK)
 
 It may be easier and safer to use 0x7fffUL.

Or __BIT()/__BITS() et cetera.

Dave

-- 
David Young OJC Technologies
dyo...@ojctech.com  Urbana, IL * (217) 278-3933


re: CVS commit: src/sys/arch/sparc/sparc

2009-05-16 Thread matthew green

   Module Name: src
   Committed By:cegger
   Date:Sat May 16 17:01:16 UTC 2009
   
   Modified Files:
src/sys/arch/sparc/sparc: machdep.c process_machdep.c svr4_machdep.c
vm_machdep.c
   
   Log Message:
   KNF, same object code generated.


why are you doing this?  are you planning on touching all of these
files or are you doing a KNF rototill that you shouldn't?


.mrg.


Re: CVS commit: src/sys/arch/sparc/sparc

2009-05-16 Thread Izumi Tsutsui
Modified Files:
   src/sys/arch/sparc/sparc: machdep.c process_machdep.c svr4_machdep.c
   vm_machdep.c

Log Message:
KNF, same object code generated.
 
 
 why are you doing this?  are you planning on touching all of these
 files or are you doing a KNF rototill that you shouldn't?

The log message should be:
remove extra whitespace accidentally added on bcopy - memcpy changes

---
Izumi Tsutsui