Re: CVS commit: src/distrib/utils/sysinst

2012-11-21 Thread Martin Husemann
On Tue, Nov 20, 2012 at 08:17:43PM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Tue Nov 20 20:17:43 UTC 2012
 
 Modified Files:
   src/distrib/utils/sysinst: bsddisklabel.c
 
 Log Message:
 Don't enable WAPBL by default. It's far too dangerous.

Please do not make changes like this without discussion.

 (a) I can see no PRs to this effect
 (b) There is no big enough warning in the man pages
 (c) we now have a serious performance degration in the default install
 compared to the softdep world

Martin


Re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread Christos Zoulas
In article 20121121080326.gl...@snowdrop.l8s.co.uk,
David Laight  da...@l8s.co.uk wrote:
On Tue, Nov 20, 2012 at 05:52:02PM -0500, Christos Zoulas wrote:
 Module Name: src
 Committed By:christos
 Date:Tue Nov 20 22:52:02 UTC 2012
 
 Modified Files:
  src/usr.bin/pkill: pkill.c
 
 Log Message:
 Don't use p_comm since it is only 16 characters long and you can find the
 full argv[0]. It is just confusing to have a long command name, that ps
 shows as the long command name, and then when you try to kill it using
 the full command name as displayed you don't get a match. While there
 fix a format nit, and remove the main() declaration.

This might break some scripts.

Well, I could put backwards compatibility, but it is more dangerous to
do so in the long term. Kids: don't use pkill(1) in scripts.

christos



Re: CVS commit: src/distrib/utils/sysinst

2012-11-21 Thread Jonathan A. Kollasch
On Wed, Nov 21, 2012 at 12:52:04PM +0100, Martin Husemann wrote:
 On Tue, Nov 20, 2012 at 08:17:43PM +, Jonathan A. Kollasch wrote:
  Module Name:src
  Committed By:   jakllsch
  Date:   Tue Nov 20 20:17:43 UTC 2012
  
  Modified Files:
  src/distrib/utils/sysinst: bsddisklabel.c
  
  Log Message:
  Don't enable WAPBL by default. It's far too dangerous.
 
 Please do not make changes like this without discussion.

I have to admit, I committed this when I was in an unreasonable frame of
mind.

  (a) I can see no PRs to this effect

This is true, I will file one.

  (b) There is no big enough warning in the man pages

True again.

  (c) we now have a serious performance degration in the default install
  compared to the softdep world

AFAIR softdeps were never enabled by default when they were around,
it was always more of a if you want performance, enable them note
somewhere.

Jonathan Kollasch


Re: CVS commit: src/distrib/utils/sysinst

2012-11-21 Thread Martin Husemann
On Wed, Nov 21, 2012 at 11:07:04AM -0600, Jonathan A. Kollasch wrote:
 AFAIR softdeps were never enabled by default when they were around,
 it was always more of a if you want performance, enable them note
 somewhere.

I was suprised, but you are right.

Martin


re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread matthew green

 In article 20121121080326.gl...@snowdrop.l8s.co.uk,
 David Laight  da...@l8s.co.uk wrote:
 On Tue, Nov 20, 2012 at 05:52:02PM -0500, Christos Zoulas wrote:
  Module Name:   src
  Committed By:  christos
  Date:  Tue Nov 20 22:52:02 UTC 2012
  
  Modified Files:
 src/usr.bin/pkill: pkill.c
  
  Log Message:
  Don't use p_comm since it is only 16 characters long and you can find the
  full argv[0]. It is just confusing to have a long command name, that ps
  shows as the long command name, and then when you try to kill it using
  the full command name as displayed you don't get a match. While there
  fix a format nit, and remove the main() declaration.
 
 This might break some scripts.
 
 Well, I could put backwards compatibility, but it is more dangerous to
 do so in the long term. Kids: don't use pkill(1) in scripts.

this seems like a fairly big semantic change to me.

could you change it to take a new option to look in argv[0] instead
of p_comm?  p_comm is not changeable by the user.


.mrg.


re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread Christos Zoulas
On Nov 22,  4:39am, m...@eterna.com.au (matthew green) wrote:
-- Subject: re: CVS commit: src/usr.bin/pkill

| this seems like a fairly big semantic change to me.
| 
| could you change it to take a new option to look in argv[0] instead
| of p_comm?  p_comm is not changeable by the user.

You could already do this with -f. The point was to make the default case
behave intuitively. Adding an option is against that. I could add backwards
compatibility by checking both, but I think that in the long term this will
be dangerous and confusing (since you might end up killing more than you
thought you would...)

christos


Re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread David Laight
On Wed, Nov 21, 2012 at 12:43:20PM -0500, Christos Zoulas wrote:
 On Nov 22,  4:39am, m...@eterna.com.au (matthew green) wrote:
 -- Subject: re: CVS commit: src/usr.bin/pkill
 
 | this seems like a fairly big semantic change to me.
 | 
 | could you change it to take a new option to look in argv[0] instead
 | of p_comm?  p_comm is not changeable by the user.
 
 You could already do this with -f. The point was to make the default case
 behave intuitively. Adding an option is against that. I could add backwards
 compatibility by checking both, but I think that in the long term this will
 be dangerous and confusing (since you might end up killing more than you
 thought you would...)

That (killing too much) is always true when you use something like pkill().
At least it is better that the 'ps | grep' I've seen many people do!

Is pkill() guaranteed to do an atomic traversal of the process list?
So it will kill something that keeps using fork() to change its pid.

David

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


Re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread Christos Zoulas
In article 20121121200309.gn...@snowdrop.l8s.co.uk,
David Laight  da...@l8s.co.uk wrote:
On Wed, Nov 21, 2012 at 12:43:20PM -0500, Christos Zoulas wrote:
 On Nov 22,  4:39am, m...@eterna.com.au (matthew green) wrote:
 -- Subject: re: CVS commit: src/usr.bin/pkill
 
 | this seems like a fairly big semantic change to me.
 | 
 | could you change it to take a new option to look in argv[0] instead
 | of p_comm?  p_comm is not changeable by the user.
 
 You could already do this with -f. The point was to make the default case
 behave intuitively. Adding an option is against that. I could add backwards
 compatibility by checking both, but I think that in the long term this will
 be dangerous and confusing (since you might end up killing more than you
 thought you would...)

That (killing too much) is always true when you use something like pkill().
At least it is better that the 'ps | grep' I've seen many people do!

Yes, this is why it is best to not use things that kill by name from scripts.

Is pkill() guaranteed to do an atomic traversal of the process list?
So it will kill something that keeps using fork() to change its pid.

I don't think our libkvm offers that.

christos



Re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread David Laight
On Wed, Nov 21, 2012 at 01:36:01PM -0700, Warner Losh wrote:
  Is pkill() guaranteed to do an atomic traversal of the process list?
  So it will kill something that keeps using fork() to change its pid.
  
  I don't think our libkvm offers that.
 
 How could you guarantee this short of moving this into the kernel
 so you could do all the comparisons while keeping all forks from happening?

You probably can't ...
Even in the kernel it might be 'interesting'.

David

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


Re: CVS commit: src/usr.bin/pkill

2012-11-21 Thread Warner Losh

On Nov 21, 2012, at 1:55 PM, David Laight wrote:

 On Wed, Nov 21, 2012 at 01:36:01PM -0700, Warner Losh wrote:
 Is pkill() guaranteed to do an atomic traversal of the process list?
 So it will kill something that keeps using fork() to change its pid.
 
 I don't think our libkvm offers that.
 
 How could you guarantee this short of moving this into the kernel
 so you could do all the comparisons while keeping all forks from happening?
 
 You probably can't ...
 Even in the kernel it might be 'interesting'.

Agreed.  It was a leading question.  Even with help from the kernel, it would 
be difficult

Warner

Re: CVS commit: src/lib/libpthread

2012-11-21 Thread YAMAMOTO Takashi
hi,

 Module Name:  src
 Committed By: christos
 Date: Wed Nov 21 19:19:25 UTC 2012
 
 Modified Files:
   src/lib/libpthread: pthread_int.h pthread_specific.c pthread_tsd.c
 
 Log Message:
 Replace the simple implementation of pthread_key_{create,destroy}
 and pthread_{g,s}etspecific functions, to one that invalidates
 values of keys in other threads when pthread_key_delete() is called.
 This fixes chromium, which expects pthread_key_delete() to do
 cleanup in all threads.

i think pthread_key_delete should not call dtor.

YAMAMOTO Takashi

 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.87 -r1.88 src/lib/libpthread/pthread_int.h
 cvs rdiff -u -r1.23 -r1.24 src/lib/libpthread/pthread_specific.c
 cvs rdiff -u -r1.8 -r1.9 src/lib/libpthread/pthread_tsd.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.


Re: CVS commit: src/lib/libpthread

2012-11-21 Thread Christos Zoulas
On Nov 21, 10:51pm, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

|  Replace the simple implementation of pthread_key_{create,destroy}
|  and pthread_{g,s}etspecific functions, to one that invalidates
|  values of keys in other threads when pthread_key_delete() is called.
|  This fixes chromium, which expects pthread_key_delete() to do
|  cleanup in all threads.
| 
| i think pthread_key_delete should not call dtor.

Ok, that's simple to change. But where is the documentation for how this
is supposed to work, so I can put a cross-reference to it.

christos