lkml.org 
[lkml]   [2006]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[RFC PATCH 2.6.17-mm1 4/3] ieee1394: convert ieee1394_transactions from semaphores to waitqueue
There were 63 instances of counting semaphores used for each hpsb_host.
All of them are now replaced by a single wait_queue.

Note, it is rather unlikely that a process has to wait for a transaction
label to become free. This condition occurs only if more than 64
transactions are initiated before the receiving node is able to complete
any of them.

A slight change in behaviour is introduced: Allocation of a tlabel will
fail if a signal was received while the process had to sleep until a
tlabel becomes available. Previously, the process slept uninterruptibly
on the semaphore until availability of a tlabel. Furthermore, error
return values of hpsb_get_tlabel() are now -ERESTARTSYS or -EAGAIN
instead of 1. However an error return value has always been defined as
"non-zero" in the API and all in-tree callers respect that.

The macro HPSB_TPOOL_INIT is no longer necessary since struct hpsb_host
is kzalloc'd.

The spinlock-protected code which accesses tlabel pools can use the
non-atomic variants of bitmap manipulation functions.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/ieee1394/hosts.c | 3
drivers/ieee1394/ieee1394_transactions.c | 82 ++++++++++++-----------
drivers/ieee1394/ieee1394_types.h | 10 --
3 files changed, 46 insertions(+), 49 deletions(-)

Index: linux-2.6.17-mm1/drivers/ieee1394/ieee1394_types.h
===================================================================
--- linux-2.6.17-mm1.orig/drivers/ieee1394/ieee1394_types.h 2006-06-24 11:11:55.000000000 +0200
+++ linux-2.6.17-mm1/drivers/ieee1394/ieee1394_types.h 2006-06-24 18:03:24.000000000 +0200
@@ -23,18 +23,8 @@ struct hpsb_tlabel_pool {
u32 next :6 __attribute__((packed));
u32 allocations :26 __attribute__((packed));
#endif
- struct semaphore count;
};

-#define HPSB_TPOOL_INIT(_tp) \
-do { \
- bitmap_zero((_tp)->pool, 64); \
- (_tp)->next = 0; \
- (_tp)->allocations = 0; \
- sema_init(&(_tp)->count, 63); \
-} while (0)
-
-
typedef u32 quadlet_t;
typedef u64 octlet_t;
typedef u16 nodeid_t;
Index: linux-2.6.17-mm1/drivers/ieee1394/hosts.c
===================================================================
--- linux-2.6.17-mm1.orig/drivers/ieee1394/hosts.c 2006-06-23 17:58:03.000000000 +0200
+++ linux-2.6.17-mm1/drivers/ieee1394/hosts.c 2006-06-24 18:03:24.000000000 +0200
@@ -143,9 +143,6 @@ struct hpsb_host *hpsb_alloc_host(struct
for (i = 2; i < 16; i++)
h->csr.gen_timestamp[i] = jiffies - 60 * HZ;

- for (i = 0; i < ARRAY_SIZE(h->tpool); i++)
- HPSB_TPOOL_INIT(&h->tpool[i]);
-
atomic_set(&h->generation, 0);

INIT_WORK(&h->delayed_reset, delayed_reset_bus, h);
Index: linux-2.6.17-mm1/drivers/ieee1394/ieee1394_transactions.c
===================================================================
--- linux-2.6.17-mm1.orig/drivers/ieee1394/ieee1394_transactions.c 2006-06-24 11:12:01.000000000 +0200
+++ linux-2.6.17-mm1/drivers/ieee1394/ieee1394_transactions.c 2006-06-24 18:03:24.000000000 +0200
@@ -9,10 +9,9 @@
* directory of the kernel sources for details.
*/

-#include <linux/sched.h>
#include <linux/bitops.h>
-#include <linux/smp_lock.h>
-#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/wait.h>

#include <asm/errno.h>

@@ -20,8 +19,6 @@
#include "ieee1394_types.h"
#include "hosts.h"
#include "ieee1394_core.h"
-#include "highlevel.h"
-#include "nodemgr.h"
#include "ieee1394_transactions.h"

#define PREP_ASYNC_HEAD_ADDRESS(tc) \
@@ -32,6 +29,7 @@
packet->header[2] = addr & 0xffffffff

spinlock_t hpsb_tlabel_lock = SPIN_LOCK_UNLOCKED;
+static DECLARE_WAIT_QUEUE_HEAD(tlabel_wq);

static void fill_async_readquad(struct hpsb_packet *packet, u64 addr)
{
@@ -116,50 +114,35 @@ static void fill_async_stream_packet(str
packet->tcode = TCODE_ISO_DATA;
}

-/**
- * hpsb_get_tlabel - allocate a transaction label
- * @packet: the packet who's tlabel/tpool we set
- *
- * Every asynchronous transaction on the 1394 bus needs a transaction
- * label to match the response to the request. This label has to be
- * different from any other transaction label in an outstanding request to
- * the same node to make matching possible without ambiguity.
- *
- * There are 64 different tlabels, so an allocated tlabel has to be freed
- * with hpsb_free_tlabel() after the transaction is complete (unless it's
- * reused again for the same target node).
- *
- * Return value: Zero on success, otherwise non-zero. A non-zero return
- * generally means there are no available tlabels. If this is called out
- * of interrupt or atomic context, then it will sleep until can return a
- * tlabel.
- */
-int hpsb_get_tlabel(struct hpsb_packet *packet)
+/* same as hpsb_get_tlabel, except that it returns immediately */
+static int hpsb_get_tlabel_atomic(struct hpsb_packet *packet)
{
unsigned long flags;
struct hpsb_tlabel_pool *tp;
int tlabel, n = NODEID_TO_NODE(packet->node_id);

- if (unlikely(n == ALL_NODES))
+ /* Broadcast transactions are complete once the request has been sent.
+ * Use the same transaction label for all broadcast transactions. */
+ if (unlikely(n == ALL_NODES)) {
+ packet->tlabel = 0;
return 0;
- tp = &packet->host->tpool[n];
-
- if (irqs_disabled() || in_atomic()) {
- if (down_trylock(&tp->count))
- return 1;
- } else {
- down(&tp->count);
}
+ BUG_ON(n > ARRAY_SIZE(packet->host->tpool));
+ tp = &packet->host->tpool[n];

spin_lock_irqsave(&hpsb_tlabel_lock, flags);

tlabel = find_next_zero_bit(tp->pool, 64, tp->next);
if (tlabel > 63)
tlabel = find_first_zero_bit(tp->pool, 64);
+ if (tlabel > 63) {
+ spin_unlock_irqrestore(&hpsb_tlabel_lock, flags);
+ return -EAGAIN;
+ }
+ __set_bit(tlabel, tp->pool);
+
/* tp->next is 6 bits wide, thus rolls over from 63 to 0 */
tp->next = (tlabel + 1);
- /* Should _never_ happen */
- BUG_ON(test_and_set_bit(tlabel, tp->pool));
tp->allocations++;

spin_unlock_irqrestore(&hpsb_tlabel_lock, flags);
@@ -168,6 +151,33 @@ int hpsb_get_tlabel(struct hpsb_packet *
}

/**
+ * hpsb_get_tlabel - allocate a transaction label
+ * @packet: the packet who's tlabel/tpool we set
+ *
+ * Every asynchronous transaction on the 1394 bus needs a transaction
+ * label to match the response to the request. This label has to be
+ * different from any other transaction label in an outstanding request to
+ * the same node to make matching possible without ambiguity.
+ *
+ * There are 64 different tlabels, so an allocated tlabel has to be freed
+ * with hpsb_free_tlabel() after the transaction is complete (unless it's
+ * reused again for the same target node).
+ *
+ * Return value: Zero on success, otherwise non-zero. A non-zero return
+ * generally means there are no available tlabels. If this is called out
+ * of interrupt or atomic context, then it will sleep until can return a
+ * tlabel or a signal is received.
+ */
+int hpsb_get_tlabel(struct hpsb_packet *packet)
+{
+ if (irqs_disabled() || in_atomic())
+ return hpsb_get_tlabel_atomic(packet);
+
+ return wait_event_interruptible(tlabel_wq,
+ !hpsb_get_tlabel_atomic(packet));
+}
+
+/**
* hpsb_free_tlabel - free an allocated transaction label
* @packet: packet whos tlabel/tpool needs to be cleared
*
@@ -191,10 +201,10 @@ void hpsb_free_tlabel(struct hpsb_packet
BUG_ON(tlabel > 63 || tlabel < 0);

spin_lock_irqsave(&hpsb_tlabel_lock, flags);
- BUG_ON(!test_and_clear_bit(tlabel, tp->pool));
+ BUG_ON(!__test_and_clear_bit(tlabel, tp->pool));
spin_unlock_irqrestore(&hpsb_tlabel_lock, flags);

- up(&tp->count);
+ wake_up_interruptible(&tlabel_wq);
}

int hpsb_packet_success(struct hpsb_packet *packet)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-06-24 19:36    [W:0.069 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site