Skip to contentSkip to conversation

shaper.c: fix locking

PATCH In review Christoph Hellwig 3 participants 5 messages May 27, 2005 – Jun 3, 2005

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 shaper.c: fix locking

o use a semaphore instead of an opencoded and racy lock
o move locking out of shaper_kick and into the callers - most just released the lock before calling shaper_kick
o remove in_interrupt() tests. from ->close we can always block, from ->hard_start_xmit and timer context never

Signed-off-by Christoph Hellwig <[email protected]>
include/linux/if_shaper.h +1 -2
Index: linux-2.6/include/linux/if_shaper.h
===================================================================
@@ -23,7 +23,7 @@
 	__u32 shapeclock;
 	unsigned long recovery;	/* Time we can next clock a packet out on
 				   an empty queue */
-        unsigned long locked;
+	struct semaphore sem;
         struct net_device_stats stats;
 	struct net_device *dev;
 	int  (*hard_start_xmit) (struct sk_buff *skb,

@@ -38,7 +38,6 @@
 	int (*hard_header_cache)(struct neighbour *neigh, struct hh_cache *hh);
 	void (*header_cache_update)(struct hh_cache *hh, struct net_device *dev, unsigned char *  haddr);
 	struct net_device_stats* (*get_stats)(struct net_device *dev);
-	wait_queue_head_t  wait_queue;
 	struct timer_list timer;
 };
drivers/net/shaper.c +19 -67

147 lines not on this page · raw message

maintainer
#1 lore raw

From: Christoph Hellwig <[email protected]>
Subject: [PATCH] shaper.c: fix locking
Date: Fri, 27 May 2005 13:54:50 +0200

Quoted 5 lines

o use a semaphore instead of an opencoded and racy lock
o move locking out of shaper_kick and into the callers - most just released the lock before calling shaper_kick
o remove in_interrupt() tests. from ->close we can always block, from ->hard_start_xmit and timer context never

Do you really want to use a semaphore for a lock taken %99 of the time in software IRQ context, which obviously cannot sleep?

author
#2 lore raw
Quoted 10 lines · David S. Miller

From: Christoph Hellwig <[email protected]>
Subject: [PATCH] shaper.c: fix locking
Date: Fri, 27 May 2005 13:54:50 +0200

Quoted 5 lines

o use a semaphore instead of an opencoded and racy lock
o move locking out of shaper_kick and into the callers - most just released the lock before calling shaper_kick
o remove in_interrupt() tests. from ->close we can always block, from ->hard_start_xmit and timer context never

Quoted 3 lines

Do you really want to use a semaphore for a lock taken %99 of the time in software IRQ context, which obviously cannot sleep?

I want to change as little as possible from the previous variant ;-)

maintainer
#3 lore raw

From: Christoph Hellwig <[email protected]>
Date: Wed, 1 Jun 2005 07:21:50 +0200

Quoted 11 lines · David S. Miller
Quoted 10 lines

From: Christoph Hellwig <[email protected]>
Subject: [PATCH] shaper.c: fix locking
Date: Fri, 27 May 2005 13:54:50 +0200

Quoted 5 lines

o use a semaphore instead of an opencoded and racy lock
o move locking out of shaper_kick and into the callers - most just released the lock before calling shaper_kick
o remove in_interrupt() tests. from ->close we can always block, from ->hard_start_xmit and timer context never

Quoted 5 lines
Quoted 3 lines

Do you really want to use a semaphore for a lock taken %99 of the time in software IRQ context, which obviously cannot sleep?

I want to change as little as possible from the previous variant ;-)

Fair enough, patch applied. If this driver breaks as a result of these changes, you get to keep the pieces ok? :-)

#4 lore raw
Quoted 2 lines · David S. Miller

Fair enough, patch applied. If this driver breaks as a result of these changes, you get to keep the pieces ok? :-)

The question is anyone really using this driver? ;->

cheers,
jamal