* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080321003604.GC20788@khazad-dum.debian.net> @ 2008-03-21 1:08 ` Andrew Morton [not found] ` <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Morton @ 2008-03-21 1:08 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: David Brownell, Richard Purdie, linux-kernel, Ingo Molnar, Geert Uytterhoeven, netdev, Martin Schwidefsky, Heiko Carstens, linux-usb, linux-wireless, video4linux-list, Stefan Richter, lm-sensors On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote: > Well, so far so good for LEDs, but what about the other users of in_atomic > that apparently should not be doing it either? Ho hum. Lots of cc's added. ./arch/x86/mm/pageattr.c Looks wrong. ./arch/m68k/atari/time.c Possibly buggy: deadlockable ./sound/core/seq/seq_virmidi.c Possibly buggy ./net/iucv/iucv.c ./kernel/power/process.c Just a debug check. ./drivers/s390/char/sclp_tty.c Possibly buggy: deadlockable ./drivers/s390/char/sclp_vt220.c Possibly buggy: deadlockable ./drivers/s390/net/netiucv.c Possibly buggy: deadlockable ./drivers/char/isicom.c Possibly buggy: deadlockable ./drivers/usb/misc/sisusbvga/sisusb_con.c Possibly buggy: deadlockable ./drivers/net/usb/pegasus.c Possibly buggy: deadlockable (I assume) ./drivers/net/wireless/airo.c Possibly buggy: deadlockable ./drivers/net/wireless/rt2x00/rt73usb.c Possibly buggy: deadlockable (I assume) ./drivers/net/wireless/rt2x00/rt2500usb.c Possibly buggy: deadlockable (I assume) ./drivers/net/wireless/hostap/hostap_ioctl.c Possibly buggy: deadlockable (I assume) ./drivers/net/wireless/zd1211rw/zd_usb.c Possibly buggy: deadlockable (I assume) ./drivers/net/irda/sir_dev.c Possibly buggy: deadlockable ./drivers/net/netxen/netxen_nic_niu.c Possibly buggy: deadlockable ./drivers/net/netxen/netxen_nic_init.c Possibly buggy: deadlockable ./drivers/ieee1394/ieee1394_transactions.c Possibly buggy: deadlockable ./drivers/video/amba-clcd.c Possibly buggy: deadlockable ./drivers/i2c/i2c-core.c Possibly buggy: deadlockable The usual pattern for most of the above is if (!in_atomic()) do_something_which_might_sleep(); problem is, in_atomic() returns false inside spinlock on non-preptible kernels. So if anyone calls those functions inside spinlock they will incorrectly schedule and another task can then come in and try take the already-held lock. Now, it happens that in_atomic() returns true on non-preemtible kernels when running in interrupt or softirq context. But if the above code really is using in_atomic() to detect am-i-called-from-interrupt and NOT am-i-called-from-inside-spinlock, they should be using in_irq(), in_softirq() or in_interrupt(). ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> @ 2008-03-21 1:31 ` Alan Stern [not found] ` <Pine.LNX.4.44L0.0803202126240.11234-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> 2008-03-21 9:21 ` Stefan Richter 2008-03-21 17:04 ` David Brownell 2 siblings, 1 reply; 17+ messages in thread From: Alan Stern @ 2008-03-21 1:31 UTC (permalink / raw) To: Andrew Morton Cc: Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Thu, 20 Mar 2008, Andrew Morton wrote: > On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote: > > > Well, so far so good for LEDs, but what about the other users of in_atomic > > that apparently should not be doing it either? > > Ho hum. Lots of cc's added. ... > The usual pattern for most of the above is > > if (!in_atomic()) > do_something_which_might_sleep(); > > problem is, in_atomic() returns false inside spinlock on non-preptible > kernels. So if anyone calls those functions inside spinlock they will > incorrectly schedule and another task can then come in and try take the > already-held lock. > > Now, it happens that in_atomic() returns true on non-preemtible kernels > when running in interrupt or softirq context. But if the above code really > is using in_atomic() to detect am-i-called-from-interrupt and NOT > am-i-called-from-inside-spinlock, they should be using in_irq(), > in_softirq() or in_interrupt(). Presumably most of these places are actually trying to detect am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? Why doesn't it do that in non-preemptible kernels? For that matter, isn't it also the sort of thing that might_sleep() is supposed to check? But looking at the definitions in include/linux/kernel.h, it appears that might_sleep() does nothing at all when neither CONFIG_PREEMPT_VOLUNTARY nor CONFIG_DEBUG_SPINLOCK_SLEEP is set. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <Pine.LNX.4.44L0.0803202126240.11234-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <Pine.LNX.4.44L0.0803202126240.11234-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> @ 2008-03-21 1:36 ` Michael Buesch [not found] ` <200803210236.52063.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Michael Buesch @ 2008-03-21 1:36 UTC (permalink / raw) To: Alan Stern Cc: Andrew Morton, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Friday 21 March 2008 02:31:44 Alan Stern wrote: > On Thu, 20 Mar 2008, Andrew Morton wrote: > > > On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote: > > > > > Well, so far so good for LEDs, but what about the other users of in_atomic > > > that apparently should not be doing it either? > > > > Ho hum. Lots of cc's added. > > ... > > > The usual pattern for most of the above is > > > > if (!in_atomic()) > > do_something_which_might_sleep(); > > > > problem is, in_atomic() returns false inside spinlock on non-preptible > > kernels. So if anyone calls those functions inside spinlock they will > > incorrectly schedule and another task can then come in and try take the > > already-held lock. > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > when running in interrupt or softirq context. But if the above code really > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > in_softirq() or in_interrupt(). > > Presumably most of these places are actually trying to detect > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? No, I think there is no such check in the kernel. Most likely for performance reasons, as it would require a global flag that is set on each spinlock. You simply must always _know_, if you are allowed to sleep or not. This is done by defining an API. The call-context is part of any kernel API. -- Greetings Michael. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <200803210236.52063.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <200803210236.52063.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> @ 2008-03-21 2:27 ` Andrew Morton [not found] ` <20080320192719.6a32386e.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Morton @ 2008-03-21 2:27 UTC (permalink / raw) To: Michael Buesch Cc: Alan Stern, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Fri, 21 Mar 2008 02:36:51 +0100 Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> wrote: > On Friday 21 March 2008 02:31:44 Alan Stern wrote: > > On Thu, 20 Mar 2008, Andrew Morton wrote: > > > > > On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote: > > > > > > > Well, so far so good for LEDs, but what about the other users of in_atomic > > > > that apparently should not be doing it either? > > > > > > Ho hum. Lots of cc's added. > > > > ... > > > > > The usual pattern for most of the above is > > > > > > if (!in_atomic()) > > > do_something_which_might_sleep(); > > > > > > problem is, in_atomic() returns false inside spinlock on non-preptible > > > kernels. So if anyone calls those functions inside spinlock they will > > > incorrectly schedule and another task can then come in and try take the > > > already-held lock. > > > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > > when running in interrupt or softirq context. But if the above code really > > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > > in_softirq() or in_interrupt(). > > > > Presumably most of these places are actually trying to detect > > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? > > No, I think there is no such check in the kernel. Most likely for performance > reasons, as it would require a global flag that is set on each spinlock. Yup. non-preemptible kernels avoid the inc/dec of current_thread_info->preempt_count on spin_lock/spin_unlock > You simply must always _know_, if you are allowed to sleep or not. This is > done by defining an API. The call-context is part of any kernel API. Yup. 99.99% of kernel code manages to do this... -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20080320192719.6a32386e.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080320192719.6a32386e.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> @ 2008-03-21 3:07 ` Alan Stern [not found] ` <Pine.LNX.4.44L0.0803202301140.16520-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> 2008-03-21 13:47 ` Heiko Carstens 1 sibling, 1 reply; 17+ messages in thread From: Alan Stern @ 2008-03-21 3:07 UTC (permalink / raw) To: Andrew Morton Cc: Michael Buesch, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Thu, 20 Mar 2008, Andrew Morton wrote: > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > > > when running in interrupt or softirq context. But if the above code really > > > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > > > in_softirq() or in_interrupt(). > > > > > > Presumably most of these places are actually trying to detect > > > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? > > > > No, I think there is no such check in the kernel. Most likely for performance > > reasons, as it would require a global flag that is set on each spinlock. > > Yup. non-preemptible kernels avoid the inc/dec of > current_thread_info->preempt_count on spin_lock/spin_unlock So then what's the point of having in_atomic() at all? Is it nothing more than a shorthand form of (in_irq() | in_softirq() | in_interrupt())? In short, you are saying that there is _no_ reliable way to determine am-i-called-from-inside-spinlock. Well, why isn't there? Would it be so terrible if non-preemptible kernels did adjust preempt_count on spin_lock/unlock? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <Pine.LNX.4.44L0.0803202301140.16520-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <Pine.LNX.4.44L0.0803202301140.16520-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> @ 2008-03-21 3:17 ` Andrew Morton 0 siblings, 0 replies; 17+ messages in thread From: Andrew Morton @ 2008-03-21 3:17 UTC (permalink / raw) To: Alan Stern Cc: Michael Buesch, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Thu, 20 Mar 2008 23:07:16 -0400 (EDT) Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote: > On Thu, 20 Mar 2008, Andrew Morton wrote: > > > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > > > > when running in interrupt or softirq context. But if the above code really > > > > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > > > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > > > > in_softirq() or in_interrupt(). > > > > > > > > Presumably most of these places are actually trying to detect > > > > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? > > > > > > No, I think there is no such check in the kernel. Most likely for performance > > > reasons, as it would require a global flag that is set on each spinlock. > > > > Yup. non-preemptible kernels avoid the inc/dec of > > current_thread_info->preempt_count on spin_lock/spin_unlock > > So then what's the point of having in_atomic() at all? Is it nothing > more than a shorthand form of (in_irq() | in_softirq() | > in_interrupt())? in_atomic() is for core kernel use only. Because in special circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on non-preemptible kernels to tell the per-arch fault handler that it was invoked by copy_*_user() inside kmap_atomic(), and it must fail. > In short, you are saying that there is _no_ reliable way to determine > am-i-called-from-inside-spinlock. That's correct. > Well, why isn't there? The reasons I identified: it adds additional overhead and it encourages poorly-thought-out design. Now we _could_ change kernel design principles from caller-knows-whats-going-on over to callee-works-out-whats-going-on. But that would affect more than this particular thing. > Would it be > so terrible if non-preemptible kernels did adjust preempt_count on > spin_lock/unlock? The vast, vast majority of kernel code has managed to get through life without needing this hidden-argument-passing. The handful of errant callsites should be able to do so as well... -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080320192719.6a32386e.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 2008-03-21 3:07 ` Alan Stern @ 2008-03-21 13:47 ` Heiko Carstens [not found] ` <20080321134750.GB4128-Pmgahw53EmNLmI7Nx2oIsGnsbthNF6/HVpNB7YpNyf8@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Heiko Carstens @ 2008-03-21 13:47 UTC (permalink / raw) To: Andrew Morton Cc: Michael Buesch, Alan Stern, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Thu, Mar 20, 2008 at 07:27:19PM -0700, Andrew Morton wrote: > On Fri, 21 Mar 2008 02:36:51 +0100 Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> wrote: > > On Friday 21 March 2008 02:31:44 Alan Stern wrote: > > > On Thu, 20 Mar 2008, Andrew Morton wrote: > > > > On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote: > > > > > > > > > Well, so far so good for LEDs, but what about the other users of in_atomic > > > > > that apparently should not be doing it either? > > > > > > > > Ho hum. Lots of cc's added. > > > > > > ... > > > > > > > The usual pattern for most of the above is > > > > > > > > if (!in_atomic()) > > > > do_something_which_might_sleep(); > > > > > > > > problem is, in_atomic() returns false inside spinlock on non-preptible > > > > kernels. So if anyone calls those functions inside spinlock they will > > > > incorrectly schedule and another task can then come in and try take the > > > > already-held lock. > > > > > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > > > when running in interrupt or softirq context. But if the above code really > > > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > > > in_softirq() or in_interrupt(). > > > > > > Presumably most of these places are actually trying to detect > > > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? > > > > No, I think there is no such check in the kernel. Most likely for performance > > reasons, as it would require a global flag that is set on each spinlock. > > Yup. non-preemptible kernels avoid the inc/dec of > current_thread_info->preempt_count on spin_lock/spin_unlock > > > You simply must always _know_, if you are allowed to sleep or not. This is > > done by defining an API. The call-context is part of any kernel API. > > Yup. 99.99% of kernel code manages to do this... This is difficult for console drivers. They get called and are supposed to print something and don't have the slightest clue which context they are running in and if they are allowed to schedule. This is the problem with e.g. s390's sclp driver. If there are no write buffers available anymore it tries to allocate memory if schedule is allowed or otherwise has to wait until finally a request finished and memory is available again. And now we have to always busy wait if we are out of buffers, since we cannot tell which context we are in? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20080321134750.GB4128-Pmgahw53EmNLmI7Nx2oIsGnsbthNF6/HVpNB7YpNyf8@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080321134750.GB4128-Pmgahw53EmNLmI7Nx2oIsGnsbthNF6/HVpNB7YpNyf8@public.gmane.org> @ 2008-03-21 16:54 ` Greg KH [not found] ` <20080321165405.GC5766-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Greg KH @ 2008-03-21 16:54 UTC (permalink / raw) To: Heiko Carstens Cc: Andrew Morton, Michael Buesch, Alan Stern, Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Fri, Mar 21, 2008 at 02:47:50PM +0100, Heiko Carstens wrote: > On Thu, Mar 20, 2008 at 07:27:19PM -0700, Andrew Morton wrote: > > On Fri, 21 Mar 2008 02:36:51 +0100 Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> wrote: > > > On Friday 21 March 2008 02:31:44 Alan Stern wrote: > > > > On Thu, 20 Mar 2008, Andrew Morton wrote: > > > > > On Thu, 20 Mar 2008 21:36:04 -0300 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote: > > > > > > > > > > > Well, so far so good for LEDs, but what about the other users of in_atomic > > > > > > that apparently should not be doing it either? > > > > > > > > > > Ho hum. Lots of cc's added. > > > > > > > > ... > > > > > > > > > The usual pattern for most of the above is > > > > > > > > > > if (!in_atomic()) > > > > > do_something_which_might_sleep(); > > > > > > > > > > problem is, in_atomic() returns false inside spinlock on non-preptible > > > > > kernels. So if anyone calls those functions inside spinlock they will > > > > > incorrectly schedule and another task can then come in and try take the > > > > > already-held lock. > > > > > > > > > > Now, it happens that in_atomic() returns true on non-preemtible kernels > > > > > when running in interrupt or softirq context. But if the above code really > > > > > is using in_atomic() to detect am-i-called-from-interrupt and NOT > > > > > am-i-called-from-inside-spinlock, they should be using in_irq(), > > > > > in_softirq() or in_interrupt(). > > > > > > > > Presumably most of these places are actually trying to detect > > > > am-i-allowed-to-sleep. Isn't that what in_atomic() is supposed to do? > > > > > > No, I think there is no such check in the kernel. Most likely for performance > > > reasons, as it would require a global flag that is set on each spinlock. > > > > Yup. non-preemptible kernels avoid the inc/dec of > > current_thread_info->preempt_count on spin_lock/spin_unlock > > > > > You simply must always _know_, if you are allowed to sleep or not. This is > > > done by defining an API. The call-context is part of any kernel API. > > > > Yup. 99.99% of kernel code manages to do this... > > This is difficult for console drivers. They get called and are supposed to > print something and don't have the slightest clue which context they are > running in and if they are allowed to schedule. > This is the problem with e.g. s390's sclp driver. If there are no write > buffers available anymore it tries to allocate memory if schedule is allowed > or otherwise has to wait until finally a request finished and memory is > available again. > And now we have to always busy wait if we are out of buffers, since we > cannot tell which context we are in? This is the reason why the drivers/usb/misc/sisusbvga driver is trying to test for in_atomic: /* We can't handle console calls in non-schedulable * context due to our locks and the USB transport. * So we simply ignore them. This should only affect * some calls to printk. */ if (in_atomic()) return NULL; So how should this be "fixed" if in_atomic() is not a valid test? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20080321165405.GC5766-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080321165405.GC5766-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> @ 2008-03-21 19:59 ` Andrew Morton [not found] ` <20080321125950.a5b38bda.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Morton @ 2008-03-21 19:59 UTC (permalink / raw) To: Greg KH Cc: heiko.carstens-tA70FqPdS9bQT0dZR+AlfA, mb-fseUSCV1ubazQB+pC5nmwQ, stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz, hmh-N3TV7GIv+o9fyO9Q7EP/yw, david-b-yBeKhBN/0LDR7s880joybQ, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-X9Un+BFzKDI, geert-Td1EMuHUCqxL1ZNQvxDV9g, netdev-u79uwXL29TY76Z2rM5mHXA, schwidefsky-tA70FqPdS9bQT0dZR+AlfA, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Fri, 21 Mar 2008 09:54:05 -0700 Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote: > > > > You simply must always _know_, if you are allowed to sleep or not. This is > > > > done by defining an API. The call-context is part of any kernel API. > > > > > > Yup. 99.99% of kernel code manages to do this... > > > > This is difficult for console drivers. They get called and are supposed to > > print something and don't have the slightest clue which context they are > > running in and if they are allowed to schedule. > > This is the problem with e.g. s390's sclp driver. If there are no write > > buffers available anymore it tries to allocate memory if schedule is allowed > > or otherwise has to wait until finally a request finished and memory is > > available again. > > And now we have to always busy wait if we are out of buffers, since we > > cannot tell which context we are in? > > This is the reason why the drivers/usb/misc/sisusbvga driver is trying > to test for in_atomic: > /* We can't handle console calls in non-schedulable > * context due to our locks and the USB transport. > * So we simply ignore them. This should only affect > * some calls to printk. > */ > if (in_atomic()) > return NULL; > > > So how should this be "fixed" if in_atomic() is not a valid test? Well. The kernel has traditionally assumed that console writes are atomic. But we now have complex sleepy drivers acting as consoles. Presumably this means that large amounts of device driver code, page allocator code, etc cannot have printks in them without going recursive. Except printk itself internally handles that, due to its need to be able to handle printk-from-interrupt-when-this-cpu-is-already-running-printk. The typical fix is for these console drivers to just assume that they cannot sleep: pass GFP_ATOMIC down into the device driver code. But I bet the device driver code was designed assuming that it could sleep, oops-bad-we-lose. And it's not just sleep-in-spinlock. If any of that device driver code uses alloc_pages(GFP_KERNEL) then it can deadlock if we do a printk from within the page allocator (and hence a lot of the block and storage layer). Because in those code paths we must use GFP_NOFS or GFP_NOIO to allocate memory. So I think the right fix here is to switch those drivers to being unconditionally atomic: don't schedule, don't take mutexes, don't use __GFP_WAIT allocations. They could of course be switched to using kmalloc(GFP_ATOMIC)+memcpy()+schedule_task(). That's rather slow, but this is not a performance-sensitive area. But more seriously, this could lead to messages getting lost from a dying machine. One possibility would be to do current->called_for_console_output=1 and then test that in various places. But a) ugh and b) that's only useful for memory allocations - it doesn't help if sleeping locks need to be taken. Another possibility might be: if (current->called_for_console_output == false) { mutex_lock(lock); } else { if (!mutex_trylock(lock)) return -EAGAIN; } and then teach the console-calling code to requeue the message for later. But that's hard, because the straightforward implementation would result in the output being queued for _all_ the currently active consoles, but some of them might already have displayed this output - there's only one log_buf. An interesting problem ;) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20080321125950.a5b38bda.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080321125950.a5b38bda.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> @ 2008-03-21 20:16 ` Michael Buesch [not found] ` <200803212116.49462.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Michael Buesch @ 2008-03-21 20:16 UTC (permalink / raw) To: Andrew Morton Cc: Greg KH, heiko.carstens-tA70FqPdS9bQT0dZR+AlfA, stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz, hmh-N3TV7GIv+o9fyO9Q7EP/yw, david-b-yBeKhBN/0LDR7s880joybQ, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-X9Un+BFzKDI, geert-Td1EMuHUCqxL1ZNQvxDV9g, netdev-u79uwXL29TY76Z2rM5mHXA, schwidefsky-tA70FqPdS9bQT0dZR+AlfA, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Friday 21 March 2008 20:59:50 Andrew Morton wrote: > They could of course be switched to using > kmalloc(GFP_ATOMIC)+memcpy()+schedule_task(). That's rather slow, but this > is not a performance-sensitive area. But more seriously, this could lead > to messages getting lost from a dying machine. Well, IMO drivers that need to sleep to transmit some data (to whatever, the screen or something) are not useful for debugging a crashing kernel anyway. Or how high is the possibility that it'd survive the actual sleep in the memory allocation? I'd say almost zero. So that schedule_task() is not that bad. -- Greetings Michael. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <200803212116.49462.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <200803212116.49462.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org> @ 2008-03-21 20:20 ` Michael Buesch 0 siblings, 0 replies; 17+ messages in thread From: Michael Buesch @ 2008-03-21 20:20 UTC (permalink / raw) To: Andrew Morton Cc: Greg KH, heiko.carstens-tA70FqPdS9bQT0dZR+AlfA, stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz, hmh-N3TV7GIv+o9fyO9Q7EP/yw, david-b-yBeKhBN/0LDR7s880joybQ, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-X9Un+BFzKDI, geert-Td1EMuHUCqxL1ZNQvxDV9g, netdev-u79uwXL29TY76Z2rM5mHXA, schwidefsky-tA70FqPdS9bQT0dZR+AlfA, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Friday 21 March 2008 21:16:48 Michael Buesch wrote: > On Friday 21 March 2008 20:59:50 Andrew Morton wrote: > > They could of course be switched to using > > kmalloc(GFP_ATOMIC)+memcpy()+schedule_task(). That's rather slow, but this > > is not a performance-sensitive area. But more seriously, this could lead > > to messages getting lost from a dying machine. > > Well, IMO drivers that need to sleep to transmit some data (to whatever, > the screen or something) are not useful for debugging a crashing kernel anyway. > Or how high is the possibility that it'd survive the actual sleep in the > memory allocation? I'd say almost zero. > So that schedule_task() is not that bad. and transmit_data_func() { if (!oops_in_progress) { schedule_transmission_for_later(); } else { /* We crash anyway, so we don't care about * possible deadlocks from memory alloc sleeps * or whatever. */ close_eyes_and_transmit_it_now(); } } -- Greetings Michael. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 2008-03-21 1:31 ` Alan Stern @ 2008-03-21 9:21 ` Stefan Richter [not found] ` <47E37E04.3080303-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org> 2008-03-21 17:04 ` David Brownell 2 siblings, 1 reply; 17+ messages in thread From: Stefan Richter @ 2008-03-21 9:21 UTC (permalink / raw) To: Andrew Morton Cc: Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, lm-sensors-GZX6beZjE8VD60Wz+7aTrA Andrew Morton wrote: > ./drivers/ieee1394/ieee1394_transactions.c > > Possibly buggy: deadlockable That's in hpsb_get_tlabel(), an exported symbol of the ieee1394 core. The in_atomic() there didn't cause problems yet and is unlikely to do so in the future, because there are no plans for substantial changes to the whole drivers/ieee1394/ anymore (because of drivers/firewire/). Nevertheless I shall look into replacing the in_atomic() by in_softirq() or something like that. Touching this legacy code is dangerous though. Some background: This in_atomic() is just one symptom of one of the fundamental design flaws of the ieee1394 stack: The "tlabels" (transaction labels, a limited resource) are acquired not only in process context but also in soft IRQ context --- but they are released only in process context. Unsurprisingly (in hindsight), the stack used to run out of tlabels simply because the tlabel consumers were scheduled more frequently than the tlabel recycler. This resulted in IO failures in sbp2 and eth1394. This is one of the design problems which inspired the submission of a new alternative driver stack. (Though this particular one of the ieee1394 stack's problems could of course also be solved by a rework of the stack --- with a respective need of resources for testing and some danger of regressions.) In the meantime (Linux 2.6.19 and 2.6.22) I added workarounds in sbp2 and eth1394 to deal with temporary lack of of tlabels. Alas I just recently received a report that eth1394's workaround is unsuccessful on non-preemptible uniprocessor kernels. I suspect the same issue exists with sbp2's workaround, it just isn't as likely to happen there. The new drivers/firewire/ recycle tlabels in bottom halves context and in timer context, which is the appropriate approach. Alas drivers/firewire/ don't have an eth1394 equivalent yet... -- Stefan Richter -=====-==--- --== =-=-= http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <47E37E04.3080303-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <47E37E04.3080303-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org> @ 2008-03-21 9:27 ` Stefan Richter 2008-03-21 12:37 ` Henrique de Moraes Holschuh 1 sibling, 0 replies; 17+ messages in thread From: Stefan Richter @ 2008-03-21 9:27 UTC (permalink / raw) To: Andrew Morton Cc: Henrique de Moraes Holschuh, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, lm-sensors-GZX6beZjE8VD60Wz+7aTrA I wrote: > Andrew Morton wrote: >> ./drivers/ieee1394/ieee1394_transactions.c >> >> Possibly buggy: deadlockable > > That's in hpsb_get_tlabel(), an exported symbol of the ieee1394 core. > > The in_atomic() there didn't cause problems yet and is unlikely to do so > in the future, because there are no plans for substantial changes to the > whole drivers/ieee1394/ anymore (because of drivers/firewire/). > > Nevertheless I shall look into replacing the in_atomic() by in_softirq() > or something like that. Or extend the API to have separate calls for callers which can sleep and callers which can't. But that may be thwarted by deep call chains. > Touching this legacy code is dangerous though. -- Stefan Richter -=====-==--- --== =-=-= http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <47E37E04.3080303-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org> 2008-03-21 9:27 ` Stefan Richter @ 2008-03-21 12:37 ` Henrique de Moraes Holschuh 2008-03-21 13:16 ` Stefan Richter 1 sibling, 1 reply; 17+ messages in thread From: Henrique de Moraes Holschuh @ 2008-03-21 12:37 UTC (permalink / raw) To: Stefan Richter Cc: Andrew Morton, David Brownell, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Fri, 21 Mar 2008, Stefan Richter wrote: > and eth1394 to deal with temporary lack of of tlabels. Alas I just > recently received a report that eth1394's workaround is unsuccessful on > non-preemptible uniprocessor kernels. I suspect the same issue exists Which, I think, is exactly the config where in_atomic() can't be used to mean "in_scheduleable_context()" ? -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c 2008-03-21 12:37 ` Henrique de Moraes Holschuh @ 2008-03-21 13:16 ` Stefan Richter [not found] ` <47E3B528.6010200-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Stefan Richter @ 2008-03-21 13:16 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Andrew Morton, David Brownell, Richard Purdie, linux-kernel, Ingo Molnar, Geert Uytterhoeven, netdev, Martin Schwidefsky, Heiko Carstens, linux-usb, linux-wireless, video4linux-list, lm-sensors Henrique de Moraes Holschuh wrote: > On Fri, 21 Mar 2008, Stefan Richter wrote: >> and eth1394 to deal with temporary lack of of tlabels. Alas I just >> recently received a report that eth1394's workaround is unsuccessful on >> non-preemptible uniprocessor kernels. > > Which, I think, is exactly the config where in_atomic() can't be used to > mean "in_scheduleable_context()" ? That's coincidence. The mentioned workaround fails this way: - tlabel consumer eth1394 (IPv4 over FireWire) grabs lots of tlabels in soft IRQ context. - tlabel recycler khpsbpkt (a kthread of ieee1394) sleeps even though it could start putting tlabels back into the pool. - eth1394 can't get tlabels anymore, stops the transmit queue, schedules a workqueue job. - eth1394's workqueue job (run by the events kthread) tries to acquire a tlabel. It does so in non-atomic context and hence sleeps in hpsb_get_tlabel() until the tlabel pool is nonempty again. It would then wake up the eth1394 transmit queue again. - Normally, khpsbpkt would have been woken up by now and would have released a lot of now unused tlabels back into the pool again. However, on UP preempt_none kernels, khpsbpkt continues to sleep. (The 1394 stack's lower level runing in IRQ context or perhaps tasklet context wakes up khpsbpkt.) - Since it doesn't get a tlabel, eth1394's workqueue jobs sleeps forever as well. Result is that all other tasks of the shared workqueue can't be serviced, notably the keyboard is stuck, and that the eth1394 connection breaks down. (I haven't started working on a fix, or opened a bugzilla ticket for it yet. The reporter currently switched his kernel to PREEMPT which is not affected.) IOW: The failure in the workaround is *not* about the in_atomic() being the wrong question asked in hpsb_get_tlabel() --- no, ieee1394's in_atomic() abuse works just fine even on UP PREEMPT_NONE. Instead, the failure is about kthreads not being scheduled in the way that I thought they would. -- Stefan Richter -=====-==--- --== =-=-= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <47E3B528.6010200-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>]
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <47E3B528.6010200-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org> @ 2008-03-22 11:29 ` Stefan Richter 0 siblings, 0 replies; 17+ messages in thread From: Stefan Richter @ 2008-03-22 11:29 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Henrique de Moraes Holschuh, Andrew Morton, David Brownell, Richard Purdie, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, lm-sensors-GZX6beZjE8VD60Wz+7aTrA I wrote: >>> and eth1394 to deal with temporary lack of of tlabels. Alas I just >>> recently received a report that eth1394's workaround is unsuccessful >>> on non-preemptible uniprocessor kernels. > (I haven't started working on a fix, or opened a bugzilla > ticket for it yet. The reporter currently switched his kernel to > PREEMPT which is not affected.) now logged as http://bugzilla.kernel.org/show_bug.cgi?id=10306 > The failure in the workaround is *not* about the in_atomic() being the > wrong question asked in hpsb_get_tlabel() --- no, ieee1394's in_atomic() > abuse works just fine even on UP PREEMPT_NONE. Instead, the failure is > about kthreads not being scheduled in the way that I thought they would. -- Stefan Richter -=====-==--- --== =-==- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: use of preempt_count instead of in_atomic() at leds-gpio.c [not found] ` <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 2008-03-21 1:31 ` Alan Stern 2008-03-21 9:21 ` Stefan Richter @ 2008-03-21 17:04 ` David Brownell 2 siblings, 0 replies; 17+ messages in thread From: David Brownell @ 2008-03-21 17:04 UTC (permalink / raw) To: Andrew Morton Cc: Henrique de Moraes Holschuh, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Geert Uytterhoeven, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky, Heiko Carstens, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-wireless-u79uwXL29TY76Z2rM5mHXA, video4linux-list-H+wXaHxf7aLQT0dZR+AlfA, Stefan Richter, lm-sensors-GZX6beZjE8VD60Wz+7aTrA On Thursday 20 March 2008, Andrew Morton wrote: > ./drivers/net/usb/pegasus.c > > Possibly buggy: deadlockable (I assume) Looks just unecessary to me ... ethtool MII ops get called from a task context, as I recall, and other drivers just rely on that. - Dave ========= CUT HERE Remove superfluous in-atomic() check; ethtool MII ops are called from task context. Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> --- drivers/net/usb/pegasus.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) --- g26.orig/drivers/net/usb/pegasus.c 2008-03-21 08:53:28.000000000 -0700 +++ g26/drivers/net/usb/pegasus.c 2008-03-21 08:54:07.000000000 -0700 @@ -1128,12 +1128,8 @@ pegasus_get_settings(struct net_device * { pegasus_t *pegasus; - if (in_atomic()) - return 0; - pegasus = netdev_priv(dev); mii_ethtool_gset(&pegasus->mii, ecmd); ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-03-22 11:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080316184349.GA28543@khazad-dum.debian.net>
[not found] ` <200803161246.23909.david-b@pacbell.net>
[not found] ` <20080318001429.896acf51.akpm@linux-foundation.org>
[not found] ` <20080320225612.GB20788@khazad-dum.debian.net>
[not found] ` <20080320164741.734e838c.akpm@linux-foundation.org>
[not found] ` <20080321003604.GC20788@khazad-dum.debian.net>
2008-03-21 1:08 ` use of preempt_count instead of in_atomic() at leds-gpio.c Andrew Morton
[not found] ` <20080320180802.426ad2d1.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2008-03-21 1:31 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803202126240.11234-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-21 1:36 ` Michael Buesch
[not found] ` <200803210236.52063.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
2008-03-21 2:27 ` Andrew Morton
[not found] ` <20080320192719.6a32386e.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2008-03-21 3:07 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803202301140.16520-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-21 3:17 ` Andrew Morton
2008-03-21 13:47 ` Heiko Carstens
[not found] ` <20080321134750.GB4128-Pmgahw53EmNLmI7Nx2oIsGnsbthNF6/HVpNB7YpNyf8@public.gmane.org>
2008-03-21 16:54 ` Greg KH
[not found] ` <20080321165405.GC5766-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2008-03-21 19:59 ` Andrew Morton
[not found] ` <20080321125950.a5b38bda.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2008-03-21 20:16 ` Michael Buesch
[not found] ` <200803212116.49462.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
2008-03-21 20:20 ` Michael Buesch
2008-03-21 9:21 ` Stefan Richter
[not found] ` <47E37E04.3080303-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>
2008-03-21 9:27 ` Stefan Richter
2008-03-21 12:37 ` Henrique de Moraes Holschuh
2008-03-21 13:16 ` Stefan Richter
[not found] ` <47E3B528.6010200-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>
2008-03-22 11:29 ` Stefan Richter
2008-03-21 17:04 ` David Brownell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).