Messages in this thread | | | Date | Thu, 16 Jul 2009 10:42:22 -0400 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 4/7] sched: Add a preempt count base offset to __might_sleep() |
| |
On Thu, Jul 16, 2009 at 04:34:53PM +0200, Peter Zijlstra wrote: > On Thu, 2009-07-16 at 16:14 +0200, Peter Zijlstra wrote: > > On Thu, 2009-07-16 at 02:28 -0400, Frederic Weisbecker wrote: > > > +++ b/include/linux/hardirq.h > > > @@ -103,6 +103,13 @@ > > > */ > > > #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != PREEMPT_INATOMIC_BASE) > > > > > > +static inline int current_preempt_equals(int preempt_offset) > > > +{ > > > + int nested = preempt_count() & ~PREEMPT_ACTIVE; > > > + > > > + return (nested == PREEMPT_INATOMIC_BASE + preempt_offset); > > > +} > > > > I'm not sure about it being in hardirq.h, I think we should keep this in > > sched.c. > > > > Also, I'm not sure about the name, but then I suck at naming too. How > > about something like: preempt_count_equals() ? > > > > Other than that the series looks nice and I've got it queued. > > I've added this on top: > > --- > Index: linux-2.6/include/linux/hardirq.h > =================================================================== > --- linux-2.6.orig/include/linux/hardirq.h > +++ linux-2.6/include/linux/hardirq.h > @@ -103,13 +103,6 @@ > */ > #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != PREEMPT_INATOMIC_BASE) > > -static inline int current_preempt_equals(int preempt_offset) > -{ > - int nested = preempt_count() & ~PREEMPT_ACTIVE; > - > - return (nested == PREEMPT_INATOMIC_BASE + preempt_offset); > -} > - > /* > * Check whether we were atomic before we did preempt_disable(): > * (used by the scheduler, *after* releasing the kernel lock) > Index: linux-2.6/kernel/sched.c > =================================================================== > --- linux-2.6.orig/kernel/sched.c > +++ linux-2.6/kernel/sched.c > @@ -9444,12 +9444,19 @@ void __init sched_init(void) > } > > #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP > +static inline int preempt_count_equals(int preempt_offset) > +{ > + int nested = preempt_count() & ~PREEMPT_ACTIVE; > + > + return (nested == PREEMPT_INATOMIC_BASE + preempt_offset); > +} > + > void __might_sleep(char *file, int line, int preempt_offset) > { > #ifdef in_atomic > static unsigned long prev_jiffy; /* ratelimiting */ > > - if ((current_preempt_equals(preempt_offset) && !irqs_disabled()) || > + if ((preempt_count_equals(preempt_offset) && !irqs_disabled()) || > system_state != SYSTEM_RUNNING || oops_in_progress) > return; > if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
Ok. Yeah the naming and the place are better there.
Thanks.
| |