mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] spinlock: rm duplicated preempt en/disable for bottom half
@ 2011-09-27  7:56 Bob Liu
  2011-09-27  9:13 ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Bob Liu @ 2011-09-27  7:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: mingo, joe, tglx, peterz, Bob Liu

local_bh_en/disable() has already deal with en/disable preempt, so rm the
dumplicated one from spinlock_api_up/smp.h head file.

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 include/linux/spinlock_api_smp.h |    4 ----
 include/linux/spinlock_api_up.h  |    5 ++---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/linux/spinlock_api_smp.h b/include/linux/spinlock_api_smp.h
index e253ccd..ba8b8f3 100644
--- a/include/linux/spinlock_api_smp.h
+++ b/include/linux/spinlock_api_smp.h
@@ -132,7 +132,6 @@ static inline void __raw_spin_lock_irq(raw_spinlock_t *lock)
 static inline void __raw_spin_lock_bh(raw_spinlock_t *lock)
 {
 	local_bh_disable();
-	preempt_disable();
 	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
 	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
 }
@@ -174,19 +173,16 @@ static inline void __raw_spin_unlock_bh(raw_spinlock_t *lock)
 {
 	spin_release(&lock->dep_map, 1, _RET_IP_);
 	do_raw_spin_unlock(lock);
-	preempt_enable_no_resched();
 	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
 }
 
 static inline int __raw_spin_trylock_bh(raw_spinlock_t *lock)
 {
 	local_bh_disable();
-	preempt_disable();
 	if (do_raw_spin_trylock(lock)) {
 		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
 		return 1;
 	}
-	preempt_enable_no_resched();
 	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
 	return 0;
 }
diff --git a/include/linux/spinlock_api_up.h b/include/linux/spinlock_api_up.h
index af1f472..c9e4f1c 100644
--- a/include/linux/spinlock_api_up.h
+++ b/include/linux/spinlock_api_up.h
@@ -28,7 +28,7 @@
   do { preempt_disable(); __acquire(lock); (void)(lock); } while (0)
 
 #define __LOCK_BH(lock) \
-  do { local_bh_disable(); __LOCK(lock); } while (0)
+  do { local_bh_disable(); __acquire(lock); (void)(lock); } while (0)
 
 #define __LOCK_IRQ(lock) \
   do { local_irq_disable(); __LOCK(lock); } while (0)
@@ -40,8 +40,7 @@
   do { preempt_enable(); __release(lock); (void)(lock); } while (0)
 
 #define __UNLOCK_BH(lock) \
-  do { preempt_enable_no_resched(); local_bh_enable(); \
-	  __release(lock); (void)(lock); } while (0)
+  do { local_bh_enable(); __release(lock); (void)(lock); } while (0)
 
 #define __UNLOCK_IRQ(lock) \
   do { local_irq_enable(); __UNLOCK(lock); } while (0)
-- 
1.6.3.3



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spinlock: rm duplicated preempt en/disable for bottom half
  2011-09-27  7:56 [PATCH] spinlock: rm duplicated preempt en/disable for bottom half Bob Liu
@ 2011-09-27  9:13 ` Peter Zijlstra
  2011-09-27  9:40   ` Bob Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2011-09-27  9:13 UTC (permalink / raw)
  To: Bob Liu; +Cc: linux-kernel, mingo, joe, tglx

On Tue, 2011-09-27 at 15:56 +0800, Bob Liu wrote:
> local_bh_en/disable() has already deal with en/disable preempt, so rm the
> dumplicated one from spinlock_api_up/smp.h head file.

I bet you haven't tried booting this..  you just broke stuff like:

  spin_lock_bh(&foo);
  /* do crap */
  spin_unlock(&foo);
  /* do some other crap */
  local_bh_enable();


And yes that does happen.

  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spinlock: rm duplicated preempt en/disable for bottom half
  2011-09-27  9:13 ` Peter Zijlstra
@ 2011-09-27  9:40   ` Bob Liu
  2011-09-27  9:57     ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Bob Liu @ 2011-09-27  9:40 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, mingo, joe, tglx

On Tue, Sep 27, 2011 at 5:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, 2011-09-27 at 15:56 +0800, Bob Liu wrote:
>> local_bh_en/disable() has already deal with en/disable preempt, so rm the
>> dumplicated one from spinlock_api_up/smp.h head file.
>
> I bet you haven't tried booting this..  you just broke stuff like:
>

I only tried it on blackfin arch and can boot successfully
although it's a simple single-cpu ystem.

>  spin_lock_bh(&foo);
>  /* do crap */
>  spin_unlock(&foo);
>  /* do some other crap */
>  local_bh_enable();
>
>
> And yes that does happen.

Could you please give an example?  I did a simple search but no result.
I thought the user should use spin_lock_bh()/spin_unlock_bh() in pairs.

-- 
Regards,
--Bob

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spinlock: rm duplicated preempt en/disable for bottom half
  2011-09-27  9:40   ` Bob Liu
@ 2011-09-27  9:57     ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2011-09-27  9:57 UTC (permalink / raw)
  To: Bob Liu; +Cc: Peter Zijlstra, linux-kernel, mingo, joe, tglx

Le mardi 27 septembre 2011 à 17:40 +0800, Bob Liu a écrit :
> On Tue, Sep 27, 2011 at 5:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, 2011-09-27 at 15:56 +0800, Bob Liu wrote:
> >> local_bh_en/disable() has already deal with en/disable preempt, so rm the
> >> dumplicated one from spinlock_api_up/smp.h head file.
> >
> > I bet you haven't tried booting this..  you just broke stuff like:
> >
> 
> I only tried it on blackfin arch and can boot successfully
> although it's a simple single-cpu ystem.
> 
> >  spin_lock_bh(&foo);
> >  /* do crap */
> >  spin_unlock(&foo);
> >  /* do some other crap */
> >  local_bh_enable();
> >
> >
> > And yes that does happen.
> 
> Could you please give an example?  I did a simple search but no result.
> I thought the user should use spin_lock_bh()/spin_unlock_bh() in pairs.
> 

You probably can find such use in network stack.

net/ipv4/inet_hashtables.c  function  __inet_hash_connect()




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-09-27  9:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-27  7:56 [PATCH] spinlock: rm duplicated preempt en/disable for bottom half Bob Liu
2011-09-27  9:13 ` Peter Zijlstra
2011-09-27  9:40   ` Bob Liu
2011-09-27  9:57     ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®