Re: Threads related SIGSEGV in random.c (diff, v2)
Hi.
Any news on that?
On Friday, September 21, 2012, Alexey Suslikov wrote:
On Fri, Sep 21, 2012 at 10:36 AM, Alexey Suslikov
alexey.susli...@gmail.com javascript:; wrote:
On Wed, Sep 19, 2012 at 10:24 PM, Ted Unangst
t...@tedunangst.comjavascript:;
wrote:
On Wed, Sep 19, 2012 at 18:50, Alexey Suslikov wrote:
On Wednesday, September 19, 2012, Theo de Raadt wrote:
arc4random() is also thread-safe (it has interal locking) and very
desirable for other reasons. But no way to save state.
The last part of this is intentional. Saving the state of pseudo
random number generators is a stupid concept from the 80's.
I see many rng functions behaving very differently. Is it a good idea
to create a common locking layer on top of need-to-be-safe rng
functions? Or we should deal only with original problem (and only
port random.c code from netbsd)?
just slap a mutex around it.
With the diff below Kannel no longer crashes. Only protecting random()
for now.
Make random() thread-safe by surrounding real call with a mutex locking.
Found by and diff from Roman Kravchuk. Mainly from NetBSD.
Sorry. Here is correct diff.
We kinda unsure about the approach. For now, we follow arc4random pattern.
Should we use generic _thread_mutex_lock/_thread_mutex_unlock instead?
Index: lib/libc/include/thread_private.h
===
RCS file: /cvs/src/lib/libc/include/thread_private.h,v
retrieving revision 1.25
diff -u -p -r1.25 thread_private.h
--- lib/libc/include/thread_private.h 16 Oct 2011 06:29:56 -
1.25
+++ lib/libc/include/thread_private.h 21 Sep 2012 07:59:34 -
@@ -172,4 +172,16 @@ void _thread_arc4_unlock(void);
_thread_arc4_unlock();\
} while (0)
+void _thread_random_lock(void);
+void _thread_random_unlock(void);
+
+#define _RANDOM_LOCK() do {\
+ if (__isthreaded) \
+ _thread_random_lock(); \
+ } while (0)
+#define _RANDOM_UNLOCK() do {\
+ if (__isthreaded) \
+ _thread_random_unlock();\
+ } while (0)
+
#endif /* _THREAD_PRIVATE_H_ */
Index: lib/libc/stdlib/random.c
===
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.17
diff -u -p -r1.17 random.c
--- lib/libc/stdlib/random.c1 Jun 2012 01:01:57 - 1.17
+++ lib/libc/stdlib/random.c21 Sep 2012 07:59:35 -
@@ -35,6 +35,7 @@
#include stdio.h
#include stdlib.h
#include unistd.h
+#include thread_private.h
/*
* random.c:
@@ -376,21 +377,38 @@ setstate(char *arg_state)
*
* Returns a 31-bit random number.
*/
-long
-random(void)
+static long
+random_unlocked(void)
{
int32_t i;
+ int32_t *f, *r;
if (rand_type == TYPE_0)
i = state[0] = (state[0] * 1103515245 + 12345)
0x7fff;
else {
- *fptr += *rptr;
- i = (*fptr 1) 0x7fff; /* chucking least random
bit */
- if (++fptr = end_ptr) {
- fptr = state;
- ++rptr;
- } else if (++rptr = end_ptr)
- rptr = state;
+ /*
+* Use local variables rather than static variables for
speed.
+*/
+ f = fptr; r = rptr;
+ *f += *r;
+ i = (*f 1) 0x7fff; /* chucking least random
bit */
+ if (++f = end_ptr) {
+ f = state;
+ ++r;
+ } else if (++r = end_ptr)
+ r = state;
+ fptr = f; rptr = r;
}
return((long)i);
+}
+
+long
+random(void)
+{
+ long r;
+
+ _RANDOM_LOCK();
+ r = random_unlocked();
+ _RANDOM_UNLOCK();
+ return (r);
}
Index: lib/libc/thread/unithread_malloc_lock.c
===
RCS file: /cvs/src/lib/libc/thread/unithread_malloc_lock.c,v
retrieving revision 1.8
diff -u -p -r1.8 unithread_malloc_lock.c
--- lib/libc/thread/unithread_malloc_lock.c 13 Jun 2008 21:18:43 -
1.8
+++ lib/libc/thread/unithread_malloc_lock.c 21 Sep 2012 07:59:35 -
@@ -21,6 +21,12 @@ WEAK_PROTOTYPE(_thread_arc4_unlock);
WEAK_ALIAS(_thread_arc4_lock);
WEAK_ALIAS(_thread_arc4_unlock);
+WEAK_PROTOTYPE(_thread_random_lock);
+WEAK_PROTOTYPE(_thread_random_unlock);
+
+WEAK_ALIAS(_thread_random_lock);
Re-evaluation of smtpd queue entries
Hi! When setting up your first host or hosts to use smtpd it is inevitable to make some mistakes here and there, which e.g. could cause delivery attempts of an email to the wrong host (relay via boo.hoo). Now, that is obviously not a problem per se, but after fixing the config, I cannot seem to re-evaluate those entries, causing them to be forever locked in the queue. So far, I've ended up manually editing the envelopes and fixing the issues I've found. Is this expected or unexpected behaviour? Maybe some rfc even mandate that routing decision should be taken on first arrival, and may not change later? /Alexander
Re: Fw: scaffolding formwork
Thanks ! we may interest in engineered concrete fromwork www.hengxingpvc.com
Re: Re-evaluation of smtpd queue entries
On Wed, Sep 26, 2012 at 11:20:31AM +0200, Alexander Hall wrote: Hi! Hi, When setting up your first host or hosts to use smtpd it is inevitable to make some mistakes here and there, which e.g. could cause delivery attempts of an email to the wrong host (relay via boo.hoo). Now, that is obviously not a problem per se, but after fixing the config, I cannot seem to re-evaluate those entries, causing them to be forever locked in the queue. So far, I've ended up manually editing the envelopes and fixing the issues I've found. Is this expected or unexpected behaviour? Maybe some rfc even mandate that routing decision should be taken on first arrival, and may not change later? This is an expected behaviour. I'll expand on that since this has been discussed a few times in private and I think it's worth being referenced in list archives ;-) First of all, to clarify the situation, OpenSMTPD does cope with changes and will dynamically lookup hosts, routes, MX, etc ... The re-evaluation thing has to do with a very specific case: what should the daemon do when a configuration changes in a way that a relay node has become a destination node, or the way around. There is no way to handle re-evaluation in a way that removes all ambiguity, and there is no way to do it reliably. Re-evaluation can lead to lost mails: rejecting mail that had been acknowledged as accepted, not notifying sender, not sending to all recipients expanded from an alias, etc, etc ... There can be no correct answer to the following questions: - do we process aliases if a relayed mail is now local ? - how can we ensure the original sender gets notified if we now reject ? - do we cancel all previously expanded aliases if we no longer expand ? - do we do it if we switched from a local node to a relay ? There are tons of questions like these where there is no valid answer. Re-evaluation is complex, it will add a lot of code to make OpenSMTPD smart, behind the admins backs, and we know in advance that it cannot be done right because what re-evaluation tries to fix is an admin issue not something that can be fixed with code. Currently OpenSMTPD provides a predictable behavior: mails are handled using the decision that was taken when they were committed to queue. If you change the configuration in a way that's compatible, no worries. Otherwise, it will not work around the admin fuckup and will probably bounce but at least there will be not undefined behaviour, someone will get notified and we'll be able to explain exactly what has happened despite the configuration change. That being said, what I'm against is OpenSMTPD trying to take smart decision by itself. I have plans in the future to extend smtpctl so that an admin can fix the aftermaths of a config fuckup *manually* ;-) -- Gilles Chehade https://www.poolp.org @poolpOrg
Re: Re-evaluation of smtpd queue entries
On 09/26/12 12:50, Gilles Chehade wrote: On Wed, Sep 26, 2012 at 11:20:31AM +0200, Alexander Hall wrote: Hi! Hi, When setting up your first host or hosts to use smtpd it is inevitable to make some mistakes here and there, which e.g. could cause delivery attempts of an email to the wrong host (relay via boo.hoo). Now, that is obviously not a problem per se, but after fixing the config, I cannot seem to re-evaluate those entries, causing them to be forever locked in the queue. So far, I've ended up manually editing the envelopes and fixing the issues I've found. Is this expected or unexpected behaviour? Maybe some rfc even mandate that routing decision should be taken on first arrival, and may not change later? This is an expected behaviour. I'll expand on that since this has been discussed a few times in private and I think it's worth being referenced in list archives ;-) First of all, to clarify the situation, OpenSMTPD does cope with changes and will dynamically lookup hosts, routes, MX, etc ... The re-evaluation thing has to do with a very specific case: what should the daemon do when a configuration changes in a way that a relay node has become a destination node, or the way around. There is no way to handle re-evaluation in a way that removes all ambiguity, and there is no way to do it reliably. Re-evaluation can lead to lost mails: rejecting mail that had been acknowledged as accepted, not notifying sender, not sending to all recipients expanded from an alias, etc, etc ... There can be no correct answer to the following questions: - do we process aliases if a relayed mail is now local ? - how can we ensure the original sender gets notified if we now reject ? - do we cancel all previously expanded aliases if we no longer expand ? - do we do it if we switched from a local node to a relay ? There are tons of questions like these where there is no valid answer. Re-evaluation is complex, it will add a lot of code to make OpenSMTPD smart, behind the admins backs, and we know in advance that it cannot be done right because what re-evaluation tries to fix is an admin issue not something that can be fixed with code. Currently OpenSMTPD provides a predictable behavior: mails are handled using the decision that was taken when they were committed to queue. If you change the configuration in a way that's compatible, no worries. Otherwise, it will not work around the admin fuckup and will probably bounce but at least there will be not undefined behaviour, someone will get notified and we'll be able to explain exactly what has happened despite the configuration change. That being said, what I'm against is OpenSMTPD trying to take smart decision by itself. I have plans in the future to extend smtpctl so that an admin can fix the aftermaths of a config fuckup *manually* ;-) Yeah this all makes sense. Thanks for the thorough explanation. /Alexander
make -j and errors
I've been thinking some more about it. POSIX says very little about parallel makes. The more I think about it, the more I think gnu-make's approach on this is stupid: if a job errors out in a fatal way, what do we gain if we keep going ? Especially for high -j values, the quicker we die, the better, as far as the error is concerned. (note that, in sequential mode, the first fatal error will kill us, so there's no point in considering further stuff running). but what about commands that take a long time to run ? Well, make already has a standard mechanism to flag those, that's called .PRECIOUS So, instead of the -dq quick death debugging option, I suggest we move to the following semantics: in case of an error, send ^C to all jobs making targets that are not tagged .PRECIOUS, wait for everything to come back, and that's it...
Re: make -j and errors
On Wed, Sep 26, 2012 at 06:21:34PM +0200, Marc Espie wrote: I've been thinking some more about it. POSIX says very little about parallel makes. The more I think about it, the more I think gnu-make's approach on this is stupid: if a job errors out in a fatal way, what do we gain if we keep going ? Especially for high -j values, the quicker we die, the better, as far as the error is concerned. (note that, in sequential mode, the first fatal error will kill us, so there's no point in considering further stuff running). but what about commands that take a long time to run ? Well, make already has a standard mechanism to flag those, that's called .PRECIOUS So, instead of the -dq quick death debugging option, I suggest we move to the following semantics: in case of an error, send ^C to all jobs making targets that are not tagged .PRECIOUS, wait for everything to come back, and that's it... Sounds like a rational approach to me. Ken
Re: make -j and errors
On Wed, Sep 26, 2012 at 06:21:34PM +0200, Marc Espie wrote: but what about commands that take a long time to run ? Well, make already has a standard mechanism to flag those, that's called .PRECIOUS What if most everything takes a fairly long time to run? Say, largish C++ sources or whatever? Mark every target as .PRECIOUS? So, instead of the -dq quick death debugging option, I suggest we move to the following semantics: in case of an error, send ^C to all jobs making targets that are not tagged .PRECIOUS, wait for everything to come back, and that's it... I think currently running jobs should be allowed to finish, as the default behavior. Why would you want to stop (at least potentially) successful work in progress? Is it because some very prominent things are done using a build-from-scratch every time? For those things, I agree that forcibly dying ASAP would save time and frustration. But for the more correct use of being able to fix a problem and have make pick up where it left off on the next invocation, your idea does not save time and frustration. If that's the case, then things that need a full build from scratch should know to invoke make with the proper flags. Kanpai! Darrin
Re: Threads related SIGSEGV in random.c (diff, v2)
On Wed, Sep 26, 2012 at 11:18, Alexey Suslikov wrote: Hi. Any news on that? Can we do it without the local variables for speed part? I am not interested in making this function faster.
Re: make -j and errors
On Wed, Sep 26, 2012 at 18:21, Marc Espie wrote: I've been thinking some more about it. POSIX says very little about parallel makes. The more I think about it, the more I think gnu-make's approach on this is stupid: if a job errors out in a fatal way, what do we gain if we keep going ? Especially for high -j values, the quicker we die, the better, as far as the error is concerned. (note that, in sequential mode, the first fatal error will kill us, so there's no point in considering further stuff running). I don't see what we gain by killing jobs. If the scheduler dice had come down differently, maybe those jobs would finish. Here's a downside, albeit maybe a stretch. What if the job doesn't like being killed? You're changing behavior here. Previously, the only way a job was interrupted was if the operator did it. In that case, I will pick up the pieces. I think letting the running jobs finish is actually a better match to the sequential make's behavior.
gem(4): simplify gem_attach_pci() variant detection code a bit
Simplify the gem(4) variant detection code a bit. OK? Index: if_gem_pci.c === RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v retrieving revision 1.31 diff -u -p -r1.31 if_gem_pci.c --- if_gem_pci.c15 Oct 2009 17:54:56 - 1.31 +++ if_gem_pci.c6 Mar 2011 06:50:12 - @@ -231,18 +231,10 @@ gem_attach_pci(struct device *parent, st sc-sc_variant = GEM_SUN_GEM; else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_ERINETWORK) sc-sc_variant = GEM_SUN_ERI; - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC) - sc-sc_variant = GEM_APPLE_GMAC; - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC) - sc-sc_variant = GEM_APPLE_GMAC; - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC) - sc-sc_variant = GEM_APPLE_GMAC; - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC) - sc-sc_variant = GEM_APPLE_GMAC; - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC) - sc-sc_variant = GEM_APPLE_GMAC; else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_K2_GMAC) sc-sc_variant = GEM_APPLE_K2_GMAC; + else + sc-sc_variant = GEM_APPLE_GMAC; #define PCI_GEM_BASEADDR 0x10 if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0, -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Re: make -j and errors
On Wed, Sep 26, 2012 at 12:01 PM, Ted Unangst t...@tedunangst.com wrote: I don't see what we gain by killing jobs. If the scheduler dice had come down differently, maybe those jobs would finish. Here's a downside, albeit maybe a stretch. What if the job doesn't like being killed? You're changing behavior here. Previously, the only way a job was interrupted was if the operator did it. In that case, I will pick up the pieces. I think letting the running jobs finish is actually a better match to the sequential make's behavior. +1
Re: Threads related SIGSEGV in random.c (diff, v2)
On Wed, Sep 26, 2012 at 9:51 PM, Ted Unangst t...@tedunangst.com wrote:
On Wed, Sep 26, 2012 at 11:18, Alexey Suslikov wrote:
Hi.
Any news on that?
Can we do it without the local variables for speed part? I am not
interested in making this function faster.
Removing only local variables part reverts us to previous
behavior (i.e. crashes).
However, leaving current code as is but adding only local
variables (see below) passes our test with no crashes.
I'm starting to believe that static globals are not good.
Can somebody help us with writing threaded test case?
As I mentioned above, we use Kannel port as a test which
is somewhat hard to share.
Alexey
Index: lib/libc/stdlib/random.c
===
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.17
diff -u -p -r1.17 random.c
--- lib/libc/stdlib/random.c1 Jun 2012 01:01:57 - 1.17
+++ lib/libc/stdlib/random.c26 Sep 2012 20:30:46 -
@@ -380,17 +380,20 @@ long
random(void)
{
int32_t i;
+ int32_t *f, *r;
if (rand_type == TYPE_0)
i = state[0] = (state[0] * 1103515245 + 12345) 0x7fff;
else {
- *fptr += *rptr;
- i = (*fptr 1) 0x7fff; /* chucking least random bit */
- if (++fptr = end_ptr) {
- fptr = state;
- ++rptr;
- } else if (++rptr = end_ptr)
- rptr = state;
+ f = fptr; r = rptr;
+ *f += *r;
+ i = (*f 1) 0x7fff; /* chucking least random bit */
+ if (++f = end_ptr) {
+ f = state;
+ ++r;
+ } else if (++r = end_ptr)
+ r = state;
+ fptr = f; rptr = r;
}
return((long)i);
}
ksh: tab completion fix (again)
I sent this diff in 2011 (?) with a bunch of other diffs.
It fixes tab completion for filenames containing special
characters like [], () and so.
This code affects interactive mode only, and I don't know a way
to do automated testing of this. Suggestions welcome.
Index: edit.c
===
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.35
diff -u -r1.35 edit.c
--- edit.c 10 Sep 2012 01:25:30 - 1.35
+++ edit.c 26 Sep 2012 21:32:42 -
@@ -348,7 +348,7 @@
{
char *toglob;
char **words;
- int nwords, i, idx, escaping;
+ int nwords;
XPtrV w;
struct source *s, *sold;
@@ -357,20 +357,6 @@
toglob = add_glob(str, slen);
- /* remove all escaping backward slashes */
- escaping = 0;
- for (i = 0, idx = 0; toglob[i]; i++) {
- if (toglob[i] == '\\' !escaping) {
- escaping = 1;
- continue;
- }
-
- toglob[idx] = toglob[i];
- idx++;
- if (escaping) escaping = 0;
- }
- toglob[idx] = '\0';
-
/*
* Convert foo* (toglob) to an array of strings (words)
*/
@@ -378,7 +364,7 @@
s = pushs(SWSTR, ATEMP);
s-start = s-str = toglob;
source = s;
- if (yylex(ONEWORD) != LWORD) {
+ if (yylex(ONEWORD|UNESCAPE) != LWORD) {
source = sold;
internal_errorf(0, fileglob: substitute error);
return 0;
@@ -394,15 +380,12 @@
if (nwords == 1) {
struct stat statb;
- /* Check if globbing failed (returned glob pattern),
-* but be careful (E.g. toglob == ab* when the file
-* ab* exists is not an error).
-* Also, check for empty result - happens if we tried
-* to glob something which evaluated to an empty
-* string (e.g., $FOO when there is no FOO, etc).
+ /* Check if file exists, also, check for empty
+* result - happens if we tried to glob something
+* which evaluated to an empty string (e.g.,
+* $FOO when there is no FOO, etc).
*/
- if ((strcmp(words[0], toglob) == 0
- stat(words[0], statb) 0) ||
+if ((lstat(words[0], statb) 0) ||
words[0][0] == '\0') {
x_free_words(nwords, words);
words = NULL;
Index: lex.c
===
RCS file: /cvs/src/bin/ksh/lex.c,v
retrieving revision 1.45
diff -u -r1.45 lex.c
--- lex.c 9 Mar 2011 09:30:39 - 1.45
+++ lex.c 26 Sep 2012 21:32:43 -
@@ -299,6 +299,10 @@
}
/* FALLTHROUGH */
default:
+ if (cf UNESCAPE) {
+ *wp++ = QCHAR, *wp++ = c;
+ break;
+ }
Xcheck(ws, wp);
if (c) { /* trailing \ is lost */
*wp++ = CHAR, *wp++ = '\\';
Index: lex.h
===
RCS file: /cvs/src/bin/ksh/lex.h,v
retrieving revision 1.11
diff -u -r1.11 lex.h
--- lex.h 29 May 2006 18:22:24 - 1.11
+++ lex.h 26 Sep 2012 21:32:43 -
@@ -113,6 +113,7 @@
#define CMDWORD BIT(8) /* parsing simple command (alias related) */
#define HEREDELIM BIT(9) /* parsing ,- delimiter */
#define HEREDOC BIT(10)/* parsing heredoc */
+#define UNESCAPE BIT(11) /* remove backslashes */
#defineHERES 10 /* max in line */
--
open source wizard
Re: make -j and errors
On Wed, Sep 26, 2012 at 12:41:10PM -0700, Philip Guenther wrote: On Wed, Sep 26, 2012 at 12:01 PM, Ted Unangst t...@tedunangst.com wrote: I don't see what we gain by killing jobs. If the scheduler dice had come down differently, maybe those jobs would finish. Here's a downside, albeit maybe a stretch. What if the job doesn't like being killed? You're changing behavior here. Previously, the only way a job was interrupted was if the operator did it. In that case, I will pick up the pieces. I think letting the running jobs finish is actually a better match to the sequential make's behavior. +1 Actually, I do see what we gain by killing jobs. You probably don't see the difference, because you run short stuff with not enough jobs. But for long running stuff, you will sometimes have an error, and notice it only a few minutes afterwards, 5000 lines of scrollback later, when they other jobs that were running finally reach completion... I see this all the time when working on large stuff. Seriously, try both. You have access to the quick death behavior right now. Try it on your normal work. Try the normal behavior as well. Yep, there's the downside that you have to pick up the pieces... happens almost never to me. Not any more often than the normal pick up the pieces because the build crashed, the makefile is bad, and you have to erase lots of shit because you restart. The way I see it, there are actually arguments both ways. I'm not talking hard killing of job, just SIGINT... build programs usually react correctly to SIGINT (gcc does)... and we're also talking about error detection and fixing. Play with both, please. This is definitely not a theoretical argument I'm trying to make...
Alert! Your email will be blacklisted soon.
Dear tech@openbsd.org, We received complaints about spam coming from your network. Spam bots are sending bulk emails, for the security reasons your email will be blacklisted. To avoid blacklisting please check your Sent folder for unknown emails and prove that you are human by entering this code 1000 here. Your email will be recorded and spam flag will be removed. No other data will be collected. Thank you for cooperation. SpamCOP SBL.
Re: ksh: tab completion fix (again)
On Thu, Sep 27, 2012 at 01:42:25AM +0400, Alexander Polakov wrote: I sent this diff in 2011 (?) with a bunch of other diffs. It fixes tab completion for filenames containing special characters like [], () and so. This code affects interactive mode only, and I don't know a way to do automated testing of this. Suggestions welcome. Seems to be working for me! Tested with the below: $ touch bad\]file $ rm batab - which expands to- $ rm bad\]file $ -- open source wizard
Re: Threads related SIGSEGV in random.c (diff, v2)
On Thu, Sep 27, 2012 at 00:18, Alexey Suslikov wrote: On Wed, Sep 26, 2012 at 9:51 PM, Ted Unangst t...@tedunangst.com wrote: On Wed, Sep 26, 2012 at 11:18, Alexey Suslikov wrote: Hi. Any news on that? Can we do it without the local variables for speed part? I am not interested in making this function faster. Removing only local variables part reverts us to previous behavior (i.e. crashes). However, leaving current code as is but adding only local variables (see below) passes our test with no crashes. If that's the case, then the locking wasn't done correctly. Switching to locals just means you're going to get weird results. I'm starting to believe that static globals are not good. Well, no, but they're what we're stuck with. As for Kannel, patching it to use rand_r may be a better solution. That's the official bad random number generator for threads in posix.
Re: make -j and errors
On Thu, Sep 27, 2012 at 00:05, Marc Espie wrote: You probably don't see the difference, because you run short stuff with not enough jobs. But for long running stuff, you will sometimes have an error, and notice it only a few minutes afterwards, 5000 lines of scrollback later, when they other jobs that were running finally reach completion... I see this all the time when working on large stuff. Yeah, and I'm ok with that. Then I run make again without -j, look at the error and fix it.
hardware VLAN tagging for vr(4)
Hi all.
This diff adds hardware 802.1q VLAN tagging support to vr(4) (just
tag/untag, it doesn't do anything with the VLAN CAM filters).
As far as I know, the capability is only in the VT6105M Rhine III chip
which is used, amongst other places, in the pcengines ALIX machines.
If anyone has a vr(4) (any revision), especially if you use VLANs,
I'd be interested to hear if this works for you.
I'm not a kernel guy, so it's quite possible I made some basic mistake
in there somewhere, but it works for me on an ALIX:
vr0 at pci0 dev 9 function 0 VIA VT6105M RhineIII rev 0x96: irq 10, address
00:0d:b9:7e:21:5c
ukphy0 at vr0 phy 1: Generic IEEE 802.3u media interface, rev. 3: OUI 0x004063,
model 0x0034
$ ifconfig vr0 hwfeatures | grep hwfeatures
hwfeatures=8037CSUM_IPv4,CSUM_TCPv4,CSUM_UDPv4,VLAN_MTU,VLAN_HWTAGGING,WOL
As an aside, I found what I believe is a latent bug in the existing
driver. When it's done with the TX descriptor, it sets the owner
bit to tell the chip that it's good to go:
#define VR_TXSTAT_OWN 0x8000
#define VR_TXOWN(x) x-vr_ptr-vr_status
VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
which expands to:
cur_tx-vr_ptr-vr_status = htole32(0x8000);
which zeroes the 31 least significant bits in the first word of the
TX descriptor. Sadly, this includes the VLAN ID, which caused me some
head-scratching trying to figure out why my tagged frames were all in
VLAN 0.
On the plus side, nothing else currently uses those bits, so right
now it doesn't matter. My questions is: why hide that behind a macro?
or at least, if you're going to, why not go the whole hog and put the
entire thing in there?
#define VR_TXOWN(x) ((x)-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN))
Thanks.
Index: sys/dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.114
diff -u -p -r1.114 if_vr.c
--- sys/dev/pci/if_vr.c 30 Jan 2012 09:11:30 - 1.114
+++ sys/dev/pci/if_vr.c 26 Sep 2012 23:13:45 -
@@ -62,6 +62,7 @@
*/
#include bpfilter.h
+#include vlan.h
#include sys/param.h
#include sys/systm.h
@@ -83,6 +84,11 @@
#include net/if_dl.h
#include net/if_media.h
+#if NVLAN 0
+#include net/if_types.h
+#include net/if_vlan_var.h
+#endif
+
#if NBPFILTER 0
#include net/bpf.h
#endif
@@ -633,6 +639,12 @@ vr_attach(struct device *parent, struct
if (sc-vr_quirks VR_Q_CSUM)
ifp-if_capabilities |= IFCAP_CSUM_IPv4|IFCAP_CSUM_TCPv4|
IFCAP_CSUM_UDPv4;
+
+#if NVLAN 0
+ /* if the hardware can do VLAN tagging, say so. */
+ if (sc-vr_quirks VR_Q_HWTAG)
+ ifp-if_capabilities |= IFCAP_VLAN_HWTAGGING;
+#endif
#ifndef SMALL_KERNEL
if (sc-vr_revid = REV_ID_VT3065_A) {
ifp-if_capabilities |= IFCAP_WOL;
@@ -880,6 +892,23 @@ vr_rxeof(struct vr_softc *sc)
cur_rx-vr_map-dm_mapsize, BUS_DMASYNC_POSTREAD);
bus_dmamap_unload(sc-sc_dmat, cur_rx-vr_map);
+#if NVLAN 0
+ /*
+* If there's a tagged packet, the 802.1q header will be at the
+* 4-byte boundary following the CRC. There will be 2 bytes
+* TPID (0x8100) and 2 bytes TCI (including VLAN ID).
+* This isn't in the data sheet.
+*/
+ if (rxctl VR_RXCTL_TAG) {
+ u_int16_t vtag;
+ int offset = ((total_len + 3) ~3) + 2;
+
+ bcopy(m-m_data + offset, vtag, sizeof(vtag));
+ m-m_pkthdr.ether_vtag = ntohs(vtag);
+ m-m_flags |= M_VLANTAG;
+ }
+#endif
+
/*
* The VIA Rhine chip includes the CRC with every
* received frame, and there's no way to turn this
@@ -1002,7 +1031,7 @@ vr_txeof(struct vr_softc *sc)
sc-vr_flags |= VR_F_RESTART;
break;
}
- VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
+ VR_TXOWN(cur_tx) |= htole32(VR_TXSTAT_OWN);
CSR_WRITE_4(sc, VR_TXADDR, cur_tx-vr_paddr);
break;
}
@@ -1170,6 +1199,16 @@ vr_encap(struct vr_softc *sc, struct vr_
struct mbuf *m_new = NULL;
u_int32_t vr_flags = 0, vr_status = 0;
+#if NVLAN 0
+ /* Tell chip to insert VLAN tag if needed. */
+ if (m_head-m_flags M_VLANTAG) {
+ u_int32_t vtag = m_head-m_pkthdr.ether_vtag;
+ vtag = (vtag VR_TXSTAT_PQSHIFT) VR_TXSTAT_PQMASK;
+ vr_status |= vtag;
+ vr_flags |= VR_TXCTL_INSERTTAG;
+ }
+#endif
+
if (sc-vr_quirks VR_Q_CSUM) {
if (m_head-m_pkthdr.csum_flags M_IPV4_CSUM_OUT)
vr_flags |=
Incompatibility between 'rthread' and 'uthread' implementation of function 'pthread_stackseg_np'.
The 'rthread' implementation of function 'pthread_stackseq_np' returns
stack information including the default guard page in 'stack_t *sinfo'
whereas the 'uthread' implementation of that function does not include
the default guard page. The following patch makes the 'rthread'
implementation behave the same way the 'uthread' implementation behaves
updating that function to stop returning references to protected memory.
Index: rthread_np.c
===
RCS file: /cvs/src/lib/librthread/rthread_np.c,v
retrieving revision 1.7
diff -u -r1.7 rthread_np.c
--- rthread_np.c18 Feb 2012 21:12:09 - 1.7
+++ rthread_np.c26 Sep 2012 22:30:46 -
@@ -71,6 +71,9 @@
#endif
sinfo-ss_sp = base;
sinfo-ss_size = thread-stack-len;
+ if (thread-stack-guardsize != 1)
+ sinfo-ss_size -= thread-stack-guardsize;
+
sinfo-ss_flags = 0;
ret = 0;
} else if (thread == _initial_thread) {
Re: Intel azalia(4) and MSI
On 2012/09/24 03:49, Brad Smith wrote: I've always wondered why this workaround was not removed once MSI support was added. This was added before we had MSI support to workaround some Intel azalia(4) being setup by the BIOS as far as I know to use MSI and thus interrupts on system that were affected by this did not work at all for azalia(4). If you have Intel azalia(4) could you please test this. Seems alright on X220. OpenBSD 5.2-current (GENERIC.MP) #0: Tue Sep 25 12:57:38 BST 2012 st...@bamboo.spacehopper.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 8466853888 (8074MB) avail mem = 8219013120 (7838MB) mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xdae9c000 (66 entries) bios0: vendor LENOVO version 8DET63WW (1.33 ) date 07/19/2012 bios0: LENOVO 4287CTO acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SLIC SSDT SSDT SSDT HPET APIC MCFG ECDT ASF! TCPA SSDT SSDT UEFI UEFI UEFI acpi0: wakeup devices LID_(S3) SLPB(S3) IGBE(S4) EXP4(S4) EXP7(S4) EHC1(S3) EHC2(S3) HDEF(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i7-2640M CPU @ 2.80GHz, 2791.35 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF cpu0: 256KB 64b/line 8-way L2 cache cpu0: apic clock running at 99MHz cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM) i7-2640M CPU @ 2.80GHz, 2790.94 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF cpu1: 256KB 64b/line 8-way L2 cache cpu2 at mainbus0: apid 2 (application processor) cpu2: Intel(R) Core(TM) i7-2640M CPU @ 2.80GHz, 2790.94 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF cpu2: 256KB 64b/line 8-way L2 cache cpu3 at mainbus0: apid 3 (application processor) cpu3: Intel(R) Core(TM) i7-2640M CPU @ 2.80GHz, 2790.94 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF cpu3: 256KB 64b/line 8-way L2 cache ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpiec0 at acpi0 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (PEG_) acpiprt2 at acpi0: bus 2 (EXP1) acpiprt3 at acpi0: bus 3 (EXP2) acpiprt4 at acpi0: bus 5 (EXP4) acpiprt5 at acpi0: bus 13 (EXP5) acpiprt6 at acpi0: bus 14 (EXP7) acpicpu0 at acpi0: C3, C1, PSS acpicpu1 at acpi0: C3, C1, PSS acpicpu2 at acpi0: C3, C1, PSS acpicpu3 at acpi0: C3, C1, PSS acpipwrres0 at acpi0: PUBS acpitz0 at acpi0: critical temperature is 99 degC acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: SLPB acpibat0 at acpi0: BAT0 model 42T4861 serial 12519 type LION oem SANYO acpibat1 at acpi0: BAT1 not present acpiac0 at acpi0: AC unit online acpithinkpad0 at acpi0 acpidock0 at acpi0: GDCK not docked (0) cpu0: Enhanced SpeedStep 2791 MHz: speeds: 2801, 2800, 2600, 2400, 2200, 2000, 1800, 1600, 1400, 1200, 1000, 800 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 Intel Core 2G Host rev 0x09 vga1 at pci0 dev 2 function 0 Intel HD Graphics 3000 rev 0x09 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) intagp0 at vga1 agp0 at intagp0: aperture at 0xe000, size 0x1000 inteldrm0 at vga1: apic 2 int 16 drm0 at inteldrm0 Intel 6 Series MEI rev 0x04 at pci0 dev 22 function 0 not configured puc0 at pci0 dev 22 function 3 Intel 6 Series KT rev 0x04: ports: 1 com com2 at puc0 port 0 apic 2 int 19: ns16550a, 16 byte fifo com2: probed fifo depth: 0 bytes em0 at pci0 dev 25 function 0 Intel 82579LM rev 0x04: msi, address f0:de:f1:f9:a7:52 ehci0 at pci0 dev 26 function 0 Intel 6 Series USB rev 0x04: apic 2 int 16 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 Intel EHCI root hub rev 2.00/1.00 addr 1 azalia0 at pci0 dev 27 function 0 Intel 6 Series HD Audio rev 0x04: msi azalia0: codecs: Conexant/0x506e, Intel/0x2805, using Conexant/0x506e audio0 at azalia0 ppb0 at pci0 dev 28 function 0 Intel 6 Series PCIE rev 0xb4: msi pci1 at ppb0 bus 2 ppb1 at pci0 dev 28 function 1 Intel 6 Series PCIE rev 0xb4: msi pci2 at ppb1 bus 3 iwn0 at pci2 dev 0 function 0 Intel Centrino
Re: Threads related SIGSEGV in random.c (diff, v2)
On Thu, 27 Sep 2012, Alexey Suslikov wrote: Removing only local variables part reverts us to previous behavior (i.e. crashes). My guess is your program is calling srandom(), srandomdev(), initstate() or setstate() as well. Your diff doesn't protect the alteration of state, end_ptr, fptr, and rptr on those paths, so a call to initstate() while another thread is in random() can walk fptr and/or rptr out of the state array. Add the necessary locking in them and run your tests again. If not, well, crank up your debugging skills. What was the line of code that actually triggered the crash? Where did the bogus pointer come from? I'm starting to believe that static globals are not good. They are incredibly good at what they do. If you're trying to say that they fundamentally can't be thread-safe, you'll need some extraordinary evidence for such a claim. Philip Guenther
