Re: CVS commit: src/distrib/utils/sysinst
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
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
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
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
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
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
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
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
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
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
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
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