Re: Where are the specific WARNS=n defined?
On Tue, 23 Aug 2011, Mouse wrote: [...] gcc errors due to comparison of signed and unsigned values. It is best to fix the errors. What errors? It is not necessarily an error to compare signed and unsigned values. In my experience, that warning produces so many more false positives than useful warnings that I normally shut it off entirely. and that one time that using it might have warned you about a serious vulnerability? the compiler is producing a warning that something you did may be unsafe.. since you are cleverer than the compiler, it is simple for you to examine it and tell the compiler that it is ok, by using a cast or the appropriate type. iain
Re: Where are the specific WARNS=n defined?
In article 201108230521.baa12...@sparkle.rodents-montreal.org, Mouse mo...@rodents-montreal.org wrote: [...] gcc errors due to comparison of signed and unsigned values. It is best to fix the errors. What errors? It is not necessarily an error to compare signed and unsigned values. In my experience, that warning produces so many more false positives than useful warnings that I normally shut it off entirely. And it is not an error to put assignments in conditionals, or not place parentheses to clarify operator precedence, etc. It is a warning; the compiler is telling you that it is unclear what you are trying to do, and you should check to make sure that this is what you want to do (and let the compiler know by making your intentions clear). For some of us this is helpful. The compiler writers try to help protect programmers against common mistakes. If you don't like the warnings you are free to turn them off. christos
Re: Where are the specific WARNS=n defined?
[...] gcc errors due to comparison of signed and unsigned values. It is best to fix the errors. In my experience, that warning produces so many more false positives than useful warnings that I normally shut it off entirely. and that one time that using it might have warned you about a serious vulnerability? When was that? Except for a few that also provoked, or would have provoked, the warning about how a conditional's value is constant due to limited range of data type, I can't recall ever finding a bug that -Wsign-compare warned about (or would have warned about). Ever. In anyone's code. Yes, it's possible there is such an occasion lurking in my future. It's also possible I've forgotten about one in the past. But I judge the expected cost of possibly having to track down such a bug directly to be well below the expected cost (both immediate and in down-the-road maintenance) of pervasive manual uglification of code to fix non-errors. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: Where are the specific WARNS=n defined?
It is not necessarily an error to compare signed and unsigned values. [...] And it is not an error to put assignments in conditionals, or not place parentheses to clarify operator precedence, etc. It is a warning [...]. For some of us this is helpful. The compiler writers try to help protect programmers against common mistakes. If you don't like the warnings you are free to turn them off. That's what I do - along with a handful of other such warnings. The question asked what the appropriate action was, whether to turn the warnings off (the way real kernel compiles apparently do anyway) or uglify the code to work around the warning [ok, my phrasing]. I believe the former is better, because in my experience the mistake the warning warns about is anything but common. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: Where are the specific WARNS=n defined?
On Tue, Aug 23, 2011 at 03:11:27AM -0400, Mouse wrote: [...] gcc errors due to comparison of signed and unsigned values. It is best to fix the errors. In my experience, that warning produces so many more false positives than useful warnings that I normally shut it off entirely. and that one time that using it might have warned you about a serious vulnerability? When was that? Except for a few that also provoked, or would have provoked, the warning about how a conditional's value is constant due to limited range of data type, I can't recall ever finding a bug that -Wsign-compare warned about (or would have warned about). Ever. In anyone's code. The most recent one I ran into (that's easily documentable and had clear adverse consequences) was PR 43413. Just FWIW. -- David A. Holland dholl...@netbsd.org
Re: Where are the specific WARNS=n defined?
On Tue, 23 Aug 2011, Christos Zoulas wrote: I'm trying to modularize a couple of drivers, and one of them is generating some gcc errors due to comparison of signed and unsigned values. The driver module is currently being compiled with WARNS=4 (just picked that up from another Makefile). Is there a more appropriate WARNS=n to use to permit the comparison? Or should I simply add CCOPTS+= -Wno-sign-compare (which is used when the driver is being compiled as part of a kernel)? It is best to fix the errors. Yeah, OK! :) Fixing, will commit shortly. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: Adding pulse support to gpio(4), gpioctl(8)
On Tue, Aug 23, 2011 at 08:24:03AM +0200, Marc Balmer wrote: Furthermore I am thinking if it would be useful if more than one process could open a gpio device, as a long as they use different pins, e.g. one process controlls a stepper motor using some of the pins while another process drives some LEDs. Perhaps something like this? gpioctl gpio0 attach gpio 0 0xf (attaches gpio1 on pins 0..3 of gpio0) gpioctl gpio0 attach gpio 4 0xf (attaches gpio2 on pins 4..7 of gpio0) gpioctl gpio0 attach gpio 1 0xc (fails w/ EBUSY) gpioctl gpio0 attach gpio 3 0x7 (fails w/ EBUSY) Then the stepper-motor process operates exclusively on /dev/gpio1, and the LED process exclusively on /dev/gpio2. Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 344-0444 x24
Re: Adding pulse support to gpio(4), gpioctl(8)
Am 24.08.11 00:00, schrieb Mindaugas Rasiukevicius:
Marc Balmer mbal...@netbsd.org wrote:
Here comes the third iteration of the gpio(4) patch. In addition to
whjat I already described, u_int_XXX types have been replaced by
uint_XXX and gpio(4) is made MPSAFE. Comments?
Furthermore I am thinking if it would be useful if more than one process
could open a gpio device, as a long as they use different pins, e.g. one
process controlls a stepper motor using some of the pins while another
process drives some LEDs.
There is nothing what prevents from multiple threads calling gpioioctl(),
which is obviously not MP-safe. As soon as you will start fixing this, it
will bring you back to the point I have already stated - the design needs
to be MP-safe in general.
Well, you need to open it first, before you can to ioctl, and if only
one process can open it, only one process can ioctl it, right?
+mutex_enter(sc-sc_mtx);
+if (sc-sc_opened)
+return EBUSY;
This leaks the lock.
Yes, this part is utterly wrong.
int sc_opened;
+kmutex_t sc_mtx;
Preferred suffix is _lock, rather than _mtx, so please use sc_lock.
Yes, no problem.
+sc-sc_pins[pin].pin_ticks_on = tvtohz(pulse-gp_pulse_on);
+sc-sc_pins[pin].pin_ticks_off = tvtohz(pulse-gp_pulse_off);
+if (sc-sc_pins[pin].pin_ticks_on == 0
+|| sc-sc_pins[pin].pin_ticks_off == 0) {
+sc-sc_pins[pin].pin_ticks_on = hz / 2;
+sc-sc_pins[pin].pin_ticks_off = hz / 2;
+}
Use gpio_pin_t *gpin = sc-sc_pins[pin]; and gpin variable instead of
sc-sc_pins[pin] each time. Apart from better readability, GCC will most
likely generate a better code.
Sure, no problem either.
Re: Adding pulse support to gpio(4), gpioctl(8)
Marc Balmer mbal...@netbsd.org wrote: There is nothing what prevents from multiple threads calling gpioioctl (), which is obviously not MP-safe. As soon as you will start fixing this, it will bring you back to the point I have already stated - the design needs to be MP-safe in general. Well, you need to open it first, before you can to ioctl, and if only one process can open it, only one process can ioctl it, right? Wrong. Multiple threads can ioctl and nobody prevents one from having a single process with multiple threads (pthreads, if you like). -- Mindaugas
deadlock on flt_noram5
Hi I experience kernel hangs, which seems to be related to memory getting scarce. getty on console is unresponsive, but I least I can drop into ddb. db ps PIDLID S CPU FLAGS STRUCT LWP * NAME WAIT 270301 3 0 4 cbc6fa80 cron flt_noram5 248501 3 0 4 cbc8d840 sh flt_noram5 246431 3 0 4 cbc6f300 ld flt_noram5 (...) 8058 1 3 0 4 cb29c280 sh flt_noram5 (...) 516 1 3 0 4 cb0470c0 cron flt_noram5 (...) 229 1 3 0 4 cac4f580 perfused flt_noram5 Reading the sources, it seems that we sleep on flt_noram5 when we are out of memory, but that nothing in the kernel is there to awake us from flt_noram5, both on -current and netbsd-5 Is that a known problem? -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: deadlock on flt_noram5
Hi I experience kernel hangs, which seems to be related to memory getting scarce. getty on console is unresponsive, but I least I can drop into ddb. show uvm might be interesting. db ps PIDLID S CPU FLAGS STRUCT LWP * NAME WAIT 270301 3 0 4 cbc6fa80 cron flt_noram5 248501 3 0 4 cbc8d840 sh flt_noram5 246431 3 0 4 cbc6f300 ld flt_noram5 (...) 8058 1 3 0 4 cb29c280 sh flt_noram5 (...) 516 1 3 0 4 cb0470c0 cron flt_noram5 (...) 229 1 3 0 4 cac4f580 perfused flt_noram5 it can be a deadlock if this perfused thread is effectively blocking the pagedaemon's work. Reading the sources, it seems that we sleep on flt_noram5 when we are out of memory, but that nothing in the kernel is there to awake us from flt_noram5, both on -current and netbsd-5 wakeup(uvmexp.free) will wake it up. YAMAMOTO Takashi Is that a known problem? -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: Adding pulse support to gpio(4), gpioctl(8)
Well, you need to open it first, before you can to ioctl, and if only one process can open it, only one process can ioctl it, right? Wrong. Agreed. Multiple threads can ioctl and nobody prevents one from having a single process with multiple threads (pthreads, if you like). Not only that, but even without threading, there are at least two ways I can think of offhand that a file descriptor, once opened, can end up in multiple processes' open file tables: fork() and SCM_RIGHTS. (There are probably others, too.) /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: deadlock on flt_noram5
db show uvm Current UVM status: pagesize=4096 (0x1000), pagemask=0xfff, pageshift=12 63217 VM pages: 37312 active, 18283 inactive, 3084 wired, 1 free pages 35865 anon, 18751 file, 3808 exec freemin=256, free-target=341, wired-max=21072 faults=920374, traps=810647, intrs=7767403, ctxswitch=12850739 softint=16186151, syscalls=897784267, swapins=23, swapouts=40 fault counts: noram=4, noanon=0, pgwait=0, pgrele=0 ok relocks(total)=4706(4711), anget(retrys)=36(1693), amapcopy=96542 neighbor anon/obj pg=119132/1323748, gets(lock/unlock)=336489/3015 cases: anon=220693, anoncow=36623, obj=275901, prcopy=60586, przero=315025 daemon and swap counts: woke=47, revs=33, scans=53717, obscans=81, anscans=33755 busy=0, freed=32656, reactivate=15379, deactivate=87720 pageouts=4244, pending=29740, nswget=1702 nswapdev=2, swpgavail=98303 swpages=98303, swpginuse=33791, swpgonly=30938, paging=1180 my guess is: - processes, including your file server, are waiting for completion of paging i/o. - paging i/o never completes as they are served by your file server. ie. it isn't a new problem. YAMAMOTO Takashi
Re: deadlock on flt_noram5
YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote: - processes, including your file server, are waiting for completion of paging i/o. - paging i/o never completes as they are served by your file server. Yes, this is my conclusion too. This suggests that we cannot make puffs_vnop_fsync/flushvncache/dosetattr synchronous to fix the race with puffs_vnop_getattr. This one is really getting tricky, as we have no way to know when SETATTR from puffs_vnop_fsync/ completes. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
