Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().

On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
[off-list ref] wrote:

On Sun, Mar 11, 2012 at 08:18:32PM +0530, santosh prasad nayak wrote:

quoted

On Sun, Mar 11, 2012 at 8:06 PM, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Sun, Mar 11, 2012 at 07:47:27PM +0530, santosh prasad nayak wrote:

quoted

Not to use in_atomic()  in driver code.

 Following article  inspired me to do the change.
http://lwn.net/Articles/274695/

"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 other words, in_atomic() works in a specific low-level situation,
but it was never meant to be used in a wider context. Its placement in
hardirq.h next to macros which can be used elsewhere was, thus, almost
certainly a mistake. As Alan Stern pointed out, the fact that Linux
Device Drivers recommends the use of in_atomic() will not have helped
the situation. Your editor recommends that the authors of that book be
immediately sacked. "

In the present case, we just check whether its an IRQ context or user
context. So for that
we can use "in_interrupt()".

Greg also mentions the same in the following mail.
http://www.spinics.net/lists/newbies/msg43402.html

In which case, we'll just have to do mdelay() and forget about allowing
anything else to run for the 20ms that we need to sleep.  Sucky but
that's the way things are.

 mdelay() or msleep() are there before. I did not change that.


my point is :  in_atomic()  vs "in_interrupt()".
We should avoid to use "in_atomic()" in driver code.

In the present case to check IRQ context "in_interrupt()" should be preferred.

in_interrupt() won't tell us if we're being called with spinlocks held,
which _is_ a possibility because this can be called from printk(), for
oops dumps and the like.

in_interrupt() just means that we're inside a hard or soft interrupt,
or nmi.  It says nothing about whether msleep() is possible.

in_atomic() is also not  error free.  I found following comment in
include/linux/hardirq.h.  How do you handle it in non-preemptible
kernel ?

/*
 * Are we running in atomic context?  WARNING: this macro cannot
 * always detect atomic context; in particular, it cannot know about
 * held spinlocks in non-preemptible kernels.  Thus it should not be
 * used in the general case to determine whether sleeping is possible.
 * Do not use in_atomic() in driver code.
 */
#define in_atomic()     ((preempt_count() & ~PREEMPT_ACTIVE) != 0)



regards
Santosh