[PATCH] part 3 - Convert IPMI driver over to use refcounts

From: Corey Minyard
Date: Thu Sep 01 2005 - 14:14:33 EST



The IPMI driver uses read/write locks to ensure that things
exist while they are in use. This is bad from a number of
points of view. This patch removes the rwlocks and uses
refcounts and a special synced list (the entries can be
refcounted and removal is blocked while an entry is in
use).

drivers/char/ipmi/ipmi_msghandler.c | 1153 +++++++++++++++++++++---------------
1 files changed, 692 insertions(+), 461 deletions(-)

Index: linux-2.6.13/drivers/char/ipmi/ipmi_msghandler.c
===================================================================
--- linux-2.6.13.orig/drivers/char/ipmi/ipmi_msghandler.c
+++ linux-2.6.13/drivers/char/ipmi/ipmi_msghandler.c
@@ -38,7 +38,6 @@
#include <linux/sched.h>
#include <linux/poll.h>
#include <linux/spinlock.h>
-#include <linux/rwsem.h>
#include <linux/slab.h>
#include <linux/ipmi.h>
#include <linux/ipmi_smi.h>
@@ -65,9 +64,210 @@ struct proc_dir_entry *proc_ipmi_root =
the max message timer. This is in milliseconds. */
#define MAX_MSG_TIMEOUT 60000

-struct ipmi_user
+
+/*
+ * The following is an implementation of a list that allows traversals
+ * and additions/deletions at the same time. If a list item is in use
+ * while the deletion is occurring, then the deletion will block until
+ * the list item is no longer in use.
+ */
+#include <linux/completion.h>
+#include <asm/atomic.h>
+struct synced_list
+{
+ struct list_head head;
+ spinlock_t lock;
+};
+
+struct synced_list_entry
+{
+ struct list_head link;
+ atomic_t usecount;
+
+ struct list_head task_list;
+};
+
+/* Return values for match functions. */
+#define SYNCED_LIST_NO_MATCH 0
+#define SYNCED_LIST_MATCH_STOP 1
+#define SYNCED_LIST_MATCH_CONTINUE -1
+
+/* Can be used for synced list find and clear operations for finding
+ and deleting a specific entry. */
+static int match_entry(struct synced_list_entry *e, void *match_data)
+{
+ if (e == match_data)
+ return SYNCED_LIST_MATCH_STOP;
+ else
+ return SYNCED_LIST_NO_MATCH;
+}
+
+struct synced_list_entry_task_q
{
struct list_head link;
+ task_t *process;
+};
+
+static inline void init_synced_list(struct synced_list *list)
+{
+ INIT_LIST_HEAD(&list->head);
+ spin_lock_init(&list->lock);
+}
+
+/* Called with the list head lock held */
+static void synced_list_wake(struct synced_list_entry *entry)
+{
+ struct synced_list_entry_task_q *e;
+
+ if (!atomic_dec_and_test(&entry->usecount))
+ /* Another thread is using the entry, too. */
+ return;
+
+ list_for_each_entry(e, &entry->task_list, link)
+ wake_up_process(e->process);
+}
+
+/* Can only be called on entries that have been "gotten". */
+#define synced_list_put_entry(pos, head) \
+ synced_list_before_exit(pos, head)
+/* Must be called with head->lock already held. */
+#define synced_list_put_entry_nolock(pos, head) \
+ synced_list_wake(pos);
+
+#define synced_list_for_each_entry(pos, l, entry, flags) \
+ for ((spin_lock_irqsave(&(l)->lock, flags), \
+ pos = container_of((l)->head.next, typeof(*(pos)),entry.link)); \
+ (prefetch((pos)->entry.link.next), \
+ &(pos)->entry.link != (&(l)->head) \
+ ? (atomic_inc(&(pos)->entry.usecount), \
+ spin_unlock_irqrestore(&(l)->lock, flags), 1) \
+ : (spin_unlock_irqrestore(&(l)->lock, flags), 0)); \
+ (spin_lock_irqsave(&(l)->lock, flags), \
+ synced_list_wake(&(pos)->entry), \
+ pos = container_of((pos)->entry.link.next, typeof(*(pos)), \
+ entry.link)))
+
+/* If you must exit a synced_list_for_each_entry loop abnormally (with
+ a break, return, goto) then you *must* call this first, with the
+ current entry. Otherwise, the entry will be left locked. */
+static void synced_list_before_exit(struct synced_list_entry *entry,
+ struct synced_list *head)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&head->lock, flags);
+ synced_list_wake(entry);
+ spin_unlock_irqrestore(&head->lock, flags);
+}
+
+/* Can only be called in a synced list loop. This will preserve the
+ entry at least until synced_list_put_entry() is called. */
+#define synced_list_get_entry(pos) atomic_inc(&(pos)->usecount)
+
+/* Must be called with head->lock already held. */
+static void synced_list_add_nolock(struct synced_list *head,
+ struct synced_list_entry *entry)
+{
+ atomic_set(&entry->usecount, 0);
+ INIT_LIST_HEAD(&entry->task_list);
+ list_add(&entry->link, &head->head);
+}
+
+static void synced_list_add(struct synced_list *head,
+ struct synced_list_entry *entry)
+{
+ spin_lock_irq(&head->lock);
+ synced_list_add_nolock(head, entry);
+ spin_unlock_irq(&head->lock);
+}
+
+/*
+ * See the SYNCED_LIST_MATCH... defines for the return values from the
+ * "match" function. If the free function is non-NULL, it will be
+ * called with the entry after it is removed from the list. This must
+ * be called with the head->lock already held.
+ */
+static int synced_list_clear(struct synced_list *head,
+ int (*match)(struct synced_list_entry *,
+ void *),
+ void (*free)(struct synced_list_entry *),
+ void *match_data)
+{
+ struct synced_list_entry *ent, *ent2;
+ int rv = -ENODEV;
+ int mrv = SYNCED_LIST_MATCH_CONTINUE;
+
+ spin_lock_irq(&head->lock);
+ restart:
+ list_for_each_entry_safe(ent, ent2, &head->head, link) {
+ if (match) {
+ mrv = match(ent, match_data);
+ if (mrv == SYNCED_LIST_NO_MATCH)
+ continue;
+ }
+ if (atomic_read(&ent->usecount)) {
+ struct synced_list_entry_task_q e;
+ e.process = current;
+ list_add(&e.link, &ent->task_list);
+ __set_current_state(TASK_UNINTERRUPTIBLE);
+ spin_unlock_irq(&head->lock);
+ schedule();
+ spin_lock_irq(&head->lock);
+ list_del(&e.link);
+ goto restart;
+ }
+ list_del(&ent->link);
+ rv = 0;
+ if (free)
+ free(ent);
+ if (mrv == SYNCED_LIST_MATCH_STOP)
+ break;
+ }
+ spin_unlock_irq(&head->lock);
+ return rv;
+}
+
+/* Returns the entry "gotten". Note that this will always stop on a
+ match, even if the return value is SYNCED_LIST_MATCH_CONTINUE. */
+static struct synced_list_entry *
+synced_list_find_nolock(struct synced_list *head,
+ int (*match)(struct synced_list_entry *,
+ void *),
+ void *match_data)
+{
+ struct synced_list_entry *ent, *rv = NULL;
+
+ list_for_each_entry(ent, &head->head, link) {
+ if (match(ent, match_data)) {
+ rv = ent;
+ synced_list_get_entry(ent);
+ break;
+ }
+ }
+ return rv;
+}
+
+/* Like above, but claims the locks. */
+static struct synced_list_entry *
+synced_list_find(struct synced_list *head,
+ int (*match)(struct synced_list_entry *,
+ void *),
+ void *match_data)
+{
+ struct synced_list_entry *rv = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&head->lock, flags);
+ rv = synced_list_find_nolock(head, match, match_data);
+ spin_unlock_irqrestore(&head->lock, flags);
+ return rv;
+}
+
+/*
+ * The main "user" data structure.
+ */
+struct ipmi_user
+{
+ struct synced_list_entry link;

/* The upper layer that handles receive messages. */
struct ipmi_user_hndl *handler;
@@ -80,15 +280,58 @@ struct ipmi_user
int gets_events;
};

+static int match_user(struct synced_list_entry *e, void *match_data)
+{
+ ipmi_user_t user = container_of(e, struct ipmi_user, link);
+ if (user == match_data)
+ return SYNCED_LIST_MATCH_STOP;
+ else
+ return SYNCED_LIST_NO_MATCH;
+}
+
+
struct cmd_rcvr
{
- struct list_head link;
+ struct synced_list_entry link;

ipmi_user_t user;
unsigned char netfn;
unsigned char cmd;
};

+static int match_rcvr_user(struct synced_list_entry *e, void *match_data)
+{
+ struct cmd_rcvr *rcvr = container_of(e, struct cmd_rcvr, link);
+ if (rcvr->user == match_data)
+ /* We want to find all of the entries that match the user. */
+ return SYNCED_LIST_MATCH_CONTINUE;
+ else
+ return SYNCED_LIST_NO_MATCH;
+}
+
+static int match_rcvr(struct synced_list_entry *e, void *match_data)
+{
+ struct cmd_rcvr *rcvr = container_of(e, struct cmd_rcvr, link);
+ struct cmd_rcvr *cmp = match_data;
+ if ((cmp->netfn == rcvr->netfn) && (cmp->cmd == rcvr->cmd))
+ return SYNCED_LIST_MATCH_STOP;
+ else
+ return SYNCED_LIST_NO_MATCH;
+}
+
+static int match_rcvr_and_user(struct synced_list_entry *e, void *match_data)
+{
+ struct cmd_rcvr *rcvr = container_of(e, struct cmd_rcvr, link);
+ struct cmd_rcvr *cmp = match_data;
+ if ((cmp->netfn == rcvr->netfn) && (cmp->cmd == rcvr->cmd)
+ && (cmp->user == rcvr->user))
+ {
+ return SYNCED_LIST_MATCH_STOP;
+ } else
+ return SYNCED_LIST_NO_MATCH;
+}
+
+
struct seq_table
{
unsigned int inuse : 1;
@@ -150,13 +393,10 @@ struct ipmi_smi
/* What interface number are we? */
int intf_num;

- /* The list of upper layers that are using me. We read-lock
- this when delivering messages to the upper layer to keep
- the user from going away while we are processing the
- message. This means that you cannot add or delete a user
- from the receive callback. */
- rwlock_t users_lock;
- struct list_head users;
+ struct kref refcount;
+
+ /* The list of upper layers that are using me. */
+ struct synced_list users;

/* Used for wake ups at startup. */
wait_queue_head_t waitq;
@@ -193,8 +433,7 @@ struct ipmi_smi

/* The list of command receivers that are registered for commands
on this interface. */
- rwlock_t cmd_rcvr_lock;
- struct list_head cmd_rcvrs;
+ struct synced_list cmd_rcvrs;

/* Events that were queues because no one was there to receive
them. */
@@ -296,16 +535,17 @@ struct ipmi_smi
unsigned int events;
};

+/* Used to mark an interface entry that cannot be used but is not a
+ * free entry, either, primarily used at creation and deletion time so
+ * a slot doesn't get reused too quickly. */
+#define IPMI_INVALID_INTERFACE_ENTRY ((ipmi_smi_t) ((long) 1))
+#define IPMI_INVALID_INTERFACE(i) (((i) == NULL) \
+ || (i == IPMI_INVALID_INTERFACE_ENTRY))
+
#define MAX_IPMI_INTERFACES 4
static ipmi_smi_t ipmi_interfaces[MAX_IPMI_INTERFACES];

-/* Used to keep interfaces from going away while operations are
- operating on interfaces. Grab read if you are not modifying the
- interfaces, write if you are. */
-static DECLARE_RWSEM(interfaces_sem);
-
-/* Directly protects the ipmi_interfaces data structure. This is
- claimed in the timer interrupt. */
+/* Directly protects the ipmi_interfaces data structure. */
static DEFINE_SPINLOCK(interfaces_lock);

/* List of watchers that want to know when smi's are added and
@@ -313,20 +553,66 @@ static DEFINE_SPINLOCK(interfaces_lock);
static struct list_head smi_watchers = LIST_HEAD_INIT(smi_watchers);
static DECLARE_RWSEM(smi_watchers_sem);

-int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
+
+static void free_recv_msg_list(struct list_head *q)
+{
+ struct ipmi_recv_msg *msg, *msg2;
+
+ list_for_each_entry_safe(msg, msg2, q, link) {
+ list_del(&msg->link);
+ ipmi_free_recv_msg(msg);
+ }
+}
+
+static void free_cmd_rcvr(struct synced_list_entry *e)
+{
+ struct cmd_rcvr *rcvr = container_of(e, struct cmd_rcvr, link);
+ kfree(rcvr);
+}
+
+static void clean_up_interface_data(ipmi_smi_t intf)
{
int i;

- down_read(&interfaces_sem);
+ free_recv_msg_list(&intf->waiting_msgs);
+ free_recv_msg_list(&intf->waiting_events);
+ synced_list_clear(&intf->cmd_rcvrs, NULL, free_cmd_rcvr, NULL);
+
+ for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) {
+ if ((intf->seq_table[i].inuse)
+ && (intf->seq_table[i].recv_msg))
+ {
+ ipmi_free_recv_msg(intf->seq_table[i].recv_msg);
+ }
+ }
+}
+
+static void intf_free(struct kref *ref)
+{
+ ipmi_smi_t intf = container_of(ref, struct ipmi_smi, refcount);
+
+ clean_up_interface_data(intf);
+ kfree(intf);
+}
+
+int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
+{
+ int i;
+ unsigned long flags;
+
down_write(&smi_watchers_sem);
list_add(&(watcher->link), &smi_watchers);
+ up_write(&smi_watchers_sem);
+ spin_lock_irqsave(&interfaces_lock, flags);
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
- if (ipmi_interfaces[i] != NULL) {
- watcher->new_smi(i);
- }
+ ipmi_smi_t intf = ipmi_interfaces[i];
+ if (IPMI_INVALID_INTERFACE(intf))
+ continue;
+ spin_unlock_irqrestore(&interfaces_lock, flags);
+ watcher->new_smi(i);
+ spin_lock_irqsave(&interfaces_lock, flags);
}
- up_write(&smi_watchers_sem);
- up_read(&interfaces_sem);
+ spin_unlock_irqrestore(&interfaces_lock, flags);
return 0;
}

@@ -451,6 +737,8 @@ unsigned int ipmi_addr_length(int addr_t
return 0;
}

+/* Note that if the message has a user, it must be called with the
+ user "gotten" by synced_list_get_entry. */
static void deliver_response(struct ipmi_recv_msg *msg)
{
if (! msg->user) {
@@ -471,8 +759,10 @@ static void deliver_response(struct ipmi
}
ipmi_free_recv_msg(msg);
} else {
- msg->user->handler->ipmi_recv_hndl(msg,
- msg->user->handler_data);
+ ipmi_user_t user = msg->user;
+ user->handler->ipmi_recv_hndl(msg,
+ user->handler_data);
+ synced_list_put_entry(&user->link, &user->intf->users);
}
}

@@ -547,6 +837,11 @@ static int intf_find_seq(ipmi_smi_t
&& (msg->msg.netfn == netfn)
&& (ipmi_addr_equal(addr, &(msg->addr))))
{
+ /* This is safe because removing the entry gets the
+ seq lock, and we hold the seq_lock now, so the user
+ in the recv_msg must be valid. */
+ if (msg->user)
+ synced_list_get_entry(&msg->user->link);
*recv_msg = msg;
intf->seq_table[seq].inuse = 0;
rv = 0;
@@ -607,6 +902,11 @@ static int intf_err_seq(ipmi_smi_t int
{
struct seq_table *ent = &(intf->seq_table[seq]);

+ /* This is safe because removing the entry gets the
+ seq lock, and we hold the seq_lock now, so the user
+ in the recv_msg must be valid. */
+ if (ent->recv_msg->user)
+ synced_list_get_entry(&ent->recv_msg->user->link);
ent->inuse = 0;
msg = ent->recv_msg;
rv = 0;
@@ -662,14 +962,16 @@ int ipmi_create_user(unsigned int
if (! new_user)
return -ENOMEM;

- down_read(&interfaces_sem);
- if ((if_num >= MAX_IPMI_INTERFACES) || ipmi_interfaces[if_num] == NULL)
- {
- rv = -EINVAL;
- goto out_unlock;
+ spin_lock_irqsave(&interfaces_lock, flags);
+ intf = ipmi_interfaces[if_num];
+ if ((if_num >= MAX_IPMI_INTERFACES) || IPMI_INVALID_INTERFACE(intf)) {
+ spin_unlock_irqrestore(&interfaces_lock, flags);
+ return -EINVAL;
}

- intf = ipmi_interfaces[if_num];
+ /* Note that each existing user holds a refcount to the interface. */
+ kref_get(&intf->refcount);
+ spin_unlock_irqrestore(&interfaces_lock, flags);

new_user->handler = handler;
new_user->handler_data = handler_data;
@@ -678,98 +980,61 @@ int ipmi_create_user(unsigned int

if (!try_module_get(intf->handlers->owner)) {
rv = -ENODEV;
- goto out_unlock;
+ goto out_err;
}

if (intf->handlers->inc_usecount) {
rv = intf->handlers->inc_usecount(intf->send_info);
if (rv) {
module_put(intf->handlers->owner);
- goto out_unlock;
+ goto out_err;
}
}

- write_lock_irqsave(&intf->users_lock, flags);
- list_add_tail(&new_user->link, &intf->users);
- write_unlock_irqrestore(&intf->users_lock, flags);
-
- out_unlock:
- if (rv) {
- kfree(new_user);
- } else {
- *user = new_user;
- }
+ synced_list_add(&intf->users, &new_user->link);
+ *user = new_user;
+ return 0;

- up_read(&interfaces_sem);
+ out_err:
+ kfree(new_user);
+ kref_put(&intf->refcount, intf_free);
return rv;
}

-static int ipmi_destroy_user_nolock(ipmi_user_t user)
+int ipmi_destroy_user(ipmi_user_t user)
{
int rv = -ENODEV;
- ipmi_user_t t_user;
- struct cmd_rcvr *rcvr, *rcvr2;
+ ipmi_smi_t intf = user->intf;
int i;
unsigned long flags;

- /* Find the user and delete them from the list. */
- list_for_each_entry(t_user, &(user->intf->users), link) {
- if (t_user == user) {
- list_del(&t_user->link);
- rv = 0;
- break;
- }
- }
-
- if (rv) {
- goto out_unlock;
- }
+ rv = synced_list_clear(&intf->users, match_entry, NULL, &user->link);
+ if (rv)
+ goto out;

- /* Remove the user from the interfaces sequence table. */
- spin_lock_irqsave(&(user->intf->seq_lock), flags);
+ /* Remove the user from the interface's sequence table. */
+ spin_lock_irqsave(&intf->seq_lock, flags);
for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) {
- if (user->intf->seq_table[i].inuse
- && (user->intf->seq_table[i].recv_msg->user == user))
+ if (intf->seq_table[i].inuse
+ && (intf->seq_table[i].recv_msg->user == user))
{
- user->intf->seq_table[i].inuse = 0;
+ intf->seq_table[i].inuse = 0;
}
}
- spin_unlock_irqrestore(&(user->intf->seq_lock), flags);
+ spin_unlock_irqrestore(&intf->seq_lock, flags);

/* Remove the user from the command receiver's table. */
- write_lock_irqsave(&(user->intf->cmd_rcvr_lock), flags);
- list_for_each_entry_safe(rcvr, rcvr2, &(user->intf->cmd_rcvrs), link) {
- if (rcvr->user == user) {
- list_del(&rcvr->link);
- kfree(rcvr);
- }
- }
- write_unlock_irqrestore(&(user->intf->cmd_rcvr_lock), flags);
+ synced_list_clear(&intf->cmd_rcvrs, match_rcvr_user, free_cmd_rcvr,
+ user);

- kfree(user);
+ module_put(intf->handlers->owner);
+ if (intf->handlers->dec_usecount)
+ intf->handlers->dec_usecount(intf->send_info);

- out_unlock:
-
- return rv;
-}
-
-int ipmi_destroy_user(ipmi_user_t user)
-{
- int rv;
- ipmi_smi_t intf = user->intf;
- unsigned long flags;
+ kref_put(&intf->refcount, intf_free);
+ kfree(user);

- down_read(&interfaces_sem);
- write_lock_irqsave(&intf->users_lock, flags);
- rv = ipmi_destroy_user_nolock(user);
- if (!rv) {
- module_put(intf->handlers->owner);
- if (intf->handlers->dec_usecount)
- intf->handlers->dec_usecount(intf->send_info);
- }
-
- write_unlock_irqrestore(&intf->users_lock, flags);
- up_read(&interfaces_sem);
+ out:
return rv;
}

@@ -823,24 +1088,25 @@ int ipmi_get_my_LUN(ipmi_user_t user,

int ipmi_set_gets_events(ipmi_user_t user, int val)
{
- unsigned long flags;
- struct ipmi_recv_msg *msg, *msg2;
+ unsigned long flags;
+ ipmi_smi_t intf = user->intf;
+ struct ipmi_recv_msg *msg, *msg2;

- read_lock(&(user->intf->users_lock));
- spin_lock_irqsave(&(user->intf->events_lock), flags);
+ spin_lock_irqsave(&intf->events_lock, flags);
user->gets_events = val;

if (val) {
/* Deliver any queued events. */
- list_for_each_entry_safe(msg, msg2, &(user->intf->waiting_events), link) {
+ list_for_each_entry_safe(msg, msg2, &intf->waiting_events, link) {
list_del(&msg->link);
+ /* The user better exist... */
+ synced_list_get_entry(&user->link);
msg->user = user;
deliver_response(msg);
}
}

- spin_unlock_irqrestore(&(user->intf->events_lock), flags);
- read_unlock(&(user->intf->users_lock));
+ spin_unlock_irqrestore(&intf->events_lock, flags);

return 0;
}
@@ -849,36 +1115,32 @@ int ipmi_register_for_cmd(ipmi_user_t
unsigned char netfn,
unsigned char cmd)
{
- struct cmd_rcvr *cmp;
- unsigned long flags;
- struct cmd_rcvr *rcvr;
- int rv = 0;
+ ipmi_smi_t intf = user->intf;
+ struct synced_list_entry *entry;
+ struct cmd_rcvr *rcvr;
+ int rv = 0;


rcvr = kmalloc(sizeof(*rcvr), GFP_KERNEL);
if (! rcvr)
return -ENOMEM;
+ rcvr->cmd = cmd;
+ rcvr->netfn = netfn;
+ rcvr->user = user;

- read_lock(&(user->intf->users_lock));
- write_lock_irqsave(&(user->intf->cmd_rcvr_lock), flags);
+ spin_lock_irq(&intf->cmd_rcvrs.lock);
/* Make sure the command/netfn is not already registered. */
- list_for_each_entry(cmp, &(user->intf->cmd_rcvrs), link) {
- if ((cmp->netfn == netfn) && (cmp->cmd == cmd)) {
- rv = -EBUSY;
- break;
- }
- }
-
- if (! rv) {
- rcvr->cmd = cmd;
- rcvr->netfn = netfn;
- rcvr->user = user;
- list_add_tail(&(rcvr->link), &(user->intf->cmd_rcvrs));
+ entry = synced_list_find_nolock(&intf->cmd_rcvrs, match_rcvr, rcvr);
+ if (entry) {
+ synced_list_put_entry_nolock(entry, &intf->cmd_rcvrs);
+ rv = -EBUSY;
+ goto out_unlock;
}

- write_unlock_irqrestore(&(user->intf->cmd_rcvr_lock), flags);
- read_unlock(&(user->intf->users_lock));
+ synced_list_add_nolock(&intf->cmd_rcvrs, &rcvr->link);

+ out_unlock:
+ spin_unlock_irq(&intf->cmd_rcvrs.lock);
if (rv)
kfree(rcvr);

@@ -889,31 +1151,20 @@ int ipmi_unregister_for_cmd(ipmi_user_t
unsigned char netfn,
unsigned char cmd)
{
- unsigned long flags;
- struct cmd_rcvr *rcvr;
- int rv = -ENOENT;
-
- read_lock(&(user->intf->users_lock));
- write_lock_irqsave(&(user->intf->cmd_rcvr_lock), flags);
- /* Make sure the command/netfn is not already registered. */
- list_for_each_entry(rcvr, &(user->intf->cmd_rcvrs), link) {
- if ((rcvr->netfn == netfn) && (rcvr->cmd == cmd)) {
- rv = 0;
- list_del(&rcvr->link);
- kfree(rcvr);
- break;
- }
- }
- write_unlock_irqrestore(&(user->intf->cmd_rcvr_lock), flags);
- read_unlock(&(user->intf->users_lock));
+ ipmi_smi_t intf = user->intf;
+ struct cmd_rcvr rcvr;

- return rv;
+ rcvr.cmd = cmd;
+ rcvr.netfn = netfn;
+ rcvr.user = user;
+ return synced_list_clear(&intf->cmd_rcvrs, match_rcvr_and_user,
+ free_cmd_rcvr, &rcvr);
}

void ipmi_user_set_run_to_completion(ipmi_user_t user, int val)
{
- user->intf->handlers->set_run_to_completion(user->intf->send_info,
- val);
+ ipmi_smi_t intf = user->intf;
+ intf->handlers->set_run_to_completion(intf->send_info, val);
}

static unsigned char
@@ -1010,19 +1261,19 @@ static inline void format_lan_msg(struct
supplied in certain circumstances (mainly at panic time). If
messages are supplied, they will be freed, even if an error
occurs. */
-static inline int i_ipmi_request(ipmi_user_t user,
- ipmi_smi_t intf,
- struct ipmi_addr *addr,
- long msgid,
- struct kernel_ipmi_msg *msg,
- void *user_msg_data,
- void *supplied_smi,
- struct ipmi_recv_msg *supplied_recv,
- int priority,
- unsigned char source_address,
- unsigned char source_lun,
- int retries,
- unsigned int retry_time_ms)
+static int i_ipmi_request(ipmi_user_t user,
+ ipmi_smi_t intf,
+ struct ipmi_addr *addr,
+ long msgid,
+ struct kernel_ipmi_msg *msg,
+ void *user_msg_data,
+ void *supplied_smi,
+ struct ipmi_recv_msg *supplied_recv,
+ int priority,
+ unsigned char source_address,
+ unsigned char source_lun,
+ int retries,
+ unsigned int retry_time_ms)
{
int rv = 0;
struct ipmi_smi_msg *smi_msg;
@@ -1725,11 +1976,11 @@ int ipmi_register_smi(struct ipmi_smi_ha
unsigned char version_major,
unsigned char version_minor,
unsigned char slave_addr,
- ipmi_smi_t *intf)
+ ipmi_smi_t *new_intf)
{
int i, j;
int rv;
- ipmi_smi_t new_intf;
+ ipmi_smi_t intf;
unsigned long flags;


@@ -1745,189 +1996,141 @@ int ipmi_register_smi(struct ipmi_smi_ha
return -ENODEV;
}

- new_intf = kmalloc(sizeof(*new_intf), GFP_KERNEL);
- if (!new_intf)
+ intf = kmalloc(sizeof(*intf), GFP_KERNEL);
+ if (!intf)
return -ENOMEM;
- memset(new_intf, 0, sizeof(*new_intf));
+ memset(intf, 0, sizeof(*intf));
+ intf->intf_num = -1;
+ kref_init(&intf->refcount);
+ intf->version_major = version_major;
+ intf->version_minor = version_minor;
+ for (j = 0; j < IPMI_MAX_CHANNELS; j++) {
+ intf->channels[j].address = IPMI_BMC_SLAVE_ADDR;
+ intf->channels[j].lun = 2;
+ }
+ if (slave_addr != 0)
+ intf->channels[0].address = slave_addr;
+ init_synced_list(&intf->users);
+ intf->handlers = handlers;
+ intf->send_info = send_info;
+ spin_lock_init(&intf->seq_lock);
+ for (j = 0; j < IPMI_IPMB_NUM_SEQ; j++) {
+ intf->seq_table[j].inuse = 0;
+ intf->seq_table[j].seqid = 0;
+ }
+ intf->curr_seq = 0;
+#ifdef CONFIG_PROC_FS
+ spin_lock_init(&intf->proc_entry_lock);
+#endif
+ spin_lock_init(&intf->waiting_msgs_lock);
+ INIT_LIST_HEAD(&intf->waiting_msgs);
+ spin_lock_init(&intf->events_lock);
+ INIT_LIST_HEAD(&intf->waiting_events);
+ intf->waiting_events_count = 0;
+ init_synced_list(&intf->cmd_rcvrs);
+ init_waitqueue_head(&intf->waitq);

- new_intf->proc_dir = NULL;
+ spin_lock_init(&intf->counter_lock);
+ intf->proc_dir = NULL;

rv = -ENOMEM;
-
- down_write(&interfaces_sem);
+ spin_lock_irqsave(&interfaces_lock, flags);
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
if (ipmi_interfaces[i] == NULL) {
- new_intf->intf_num = i;
- new_intf->version_major = version_major;
- new_intf->version_minor = version_minor;
- for (j = 0; j < IPMI_MAX_CHANNELS; j++) {
- new_intf->channels[j].address
- = IPMI_BMC_SLAVE_ADDR;
- new_intf->channels[j].lun = 2;
- }
- if (slave_addr != 0)
- new_intf->channels[0].address = slave_addr;
- rwlock_init(&(new_intf->users_lock));
- INIT_LIST_HEAD(&(new_intf->users));
- new_intf->handlers = handlers;
- new_intf->send_info = send_info;
- spin_lock_init(&(new_intf->seq_lock));
- for (j = 0; j < IPMI_IPMB_NUM_SEQ; j++) {
- new_intf->seq_table[j].inuse = 0;
- new_intf->seq_table[j].seqid = 0;
- }
- new_intf->curr_seq = 0;
-#ifdef CONFIG_PROC_FS
- spin_lock_init(&(new_intf->proc_entry_lock));
-#endif
- spin_lock_init(&(new_intf->waiting_msgs_lock));
- INIT_LIST_HEAD(&(new_intf->waiting_msgs));
- spin_lock_init(&(new_intf->events_lock));
- INIT_LIST_HEAD(&(new_intf->waiting_events));
- new_intf->waiting_events_count = 0;
- rwlock_init(&(new_intf->cmd_rcvr_lock));
- init_waitqueue_head(&new_intf->waitq);
- INIT_LIST_HEAD(&(new_intf->cmd_rcvrs));
-
- spin_lock_init(&(new_intf->counter_lock));
-
- spin_lock_irqsave(&interfaces_lock, flags);
- ipmi_interfaces[i] = new_intf;
- spin_unlock_irqrestore(&interfaces_lock, flags);
-
+ intf->intf_num = i;
+ /* Reserve the entry till we are done. */
+ ipmi_interfaces[i] = IPMI_INVALID_INTERFACE_ENTRY;
rv = 0;
- *intf = new_intf;
break;
}
}
+ spin_unlock_irqrestore(&interfaces_lock, flags);
+ if (rv)
+ goto out;

- downgrade_write(&interfaces_sem);
-
- if (rv == 0)
- rv = add_proc_entries(*intf, i);
-
- if (rv == 0) {
- if ((version_major > 1)
- || ((version_major == 1) && (version_minor >= 5)))
- {
- /* Start scanning the channels to see what is
- available. */
- (*intf)->null_user_handler = channel_handler;
- (*intf)->curr_channel = 0;
- rv = send_channel_info_cmd(*intf, 0);
- if (rv)
- goto out;
-
- /* Wait for the channel info to be read. */
- up_read(&interfaces_sem);
- wait_event((*intf)->waitq,
- ((*intf)->curr_channel>=IPMI_MAX_CHANNELS));
- down_read(&interfaces_sem);
-
- if (ipmi_interfaces[i] != new_intf)
- /* Well, it went away. Just return. */
- goto out;
- } else {
- /* Assume a single IPMB channel at zero. */
- (*intf)->channels[0].medium = IPMI_CHANNEL_MEDIUM_IPMB;
- (*intf)->channels[0].protocol
- = IPMI_CHANNEL_PROTOCOL_IPMB;
- }
+ /* FIXME - this is an ugly kludge, this sets the intf for the
+ caller before sending any messages with it. */
+ *new_intf = intf;
+
+ if ((version_major > 1)
+ || ((version_major == 1) && (version_minor >= 5)))
+ {
+ /* Start scanning the channels to see what is
+ available. */
+ intf->null_user_handler = channel_handler;
+ intf->curr_channel = 0;
+ rv = send_channel_info_cmd(intf, 0);
+ if (rv)
+ goto out;

- /* Call all the watcher interfaces to tell
- them that a new interface is available. */
- call_smi_watchers(i);
+ /* Wait for the channel info to be read. */
+ wait_event(intf->waitq,
+ intf->curr_channel >= IPMI_MAX_CHANNELS);
+ } else {
+ /* Assume a single IPMB channel at zero. */
+ intf->channels[0].medium = IPMI_CHANNEL_MEDIUM_IPMB;
+ intf->channels[0].protocol = IPMI_CHANNEL_PROTOCOL_IPMB;
}

- out:
- up_read(&interfaces_sem);
+ if (rv == 0)
+ rv = add_proc_entries(intf, i);

+ out:
if (rv) {
- if (new_intf->proc_dir)
- remove_proc_entries(new_intf);
- kfree(new_intf);
+ if (intf->proc_dir)
+ remove_proc_entries(intf);
+ kref_put(&intf->refcount, intf_free);
+ if (i < MAX_IPMI_INTERFACES) {
+ spin_lock_irqsave(&interfaces_lock, flags);
+ ipmi_interfaces[i] = NULL;
+ spin_unlock_irqrestore(&interfaces_lock, flags);
+ }
+ } else {
+ spin_lock_irqsave(&interfaces_lock, flags);
+ ipmi_interfaces[i] = intf;
+ spin_unlock_irqrestore(&interfaces_lock, flags);
+ call_smi_watchers(i);
}

return rv;
}

-static void free_recv_msg_list(struct list_head *q)
-{
- struct ipmi_recv_msg *msg, *msg2;
-
- list_for_each_entry_safe(msg, msg2, q, link) {
- list_del(&msg->link);
- ipmi_free_recv_msg(msg);
- }
-}
-
-static void free_cmd_rcvr_list(struct list_head *q)
-{
- struct cmd_rcvr *rcvr, *rcvr2;
-
- list_for_each_entry_safe(rcvr, rcvr2, q, link) {
- list_del(&rcvr->link);
- kfree(rcvr);
- }
-}
-
-static void clean_up_interface_data(ipmi_smi_t intf)
-{
- int i;
-
- free_recv_msg_list(&(intf->waiting_msgs));
- free_recv_msg_list(&(intf->waiting_events));
- free_cmd_rcvr_list(&(intf->cmd_rcvrs));
-
- for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) {
- if ((intf->seq_table[i].inuse)
- && (intf->seq_table[i].recv_msg))
- {
- ipmi_free_recv_msg(intf->seq_table[i].recv_msg);
- }
- }
-}
-
int ipmi_unregister_smi(ipmi_smi_t intf)
{
- int rv = -ENODEV;
int i;
struct ipmi_smi_watcher *w;
unsigned long flags;

- down_write(&interfaces_sem);
- if (list_empty(&(intf->users)))
- {
- for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
- if (ipmi_interfaces[i] == intf) {
- remove_proc_entries(intf);
- spin_lock_irqsave(&interfaces_lock, flags);
- ipmi_interfaces[i] = NULL;
- clean_up_interface_data(intf);
- spin_unlock_irqrestore(&interfaces_lock,flags);
- kfree(intf);
- rv = 0;
- goto out_call_watcher;
- }
+ spin_lock_irqsave(&interfaces_lock, flags);
+ for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
+ if (ipmi_interfaces[i] == intf) {
+ /* Set the interface number reserved until we
+ * are done. */
+ ipmi_interfaces[i] = IPMI_INVALID_INTERFACE_ENTRY;
+ intf->intf_num = -1;
+ break;
}
- } else {
- rv = -EBUSY;
}
- up_write(&interfaces_sem);
+ spin_unlock_irqrestore(&interfaces_lock,flags);

- return rv;
+ if (i == MAX_IPMI_INTERFACES)
+ return -ENODEV;

- out_call_watcher:
- downgrade_write(&interfaces_sem);
+ remove_proc_entries(intf);

/* Call all the watcher interfaces to tell them that
an interface is gone. */
down_read(&smi_watchers_sem);
- list_for_each_entry(w, &smi_watchers, link) {
+ list_for_each_entry(w, &smi_watchers, link)
w->smi_gone(i);
- }
up_read(&smi_watchers_sem);
- up_read(&interfaces_sem);
+
+ /* Allow the entry to be reused now. */
+ spin_lock_irqsave(&interfaces_lock, flags);
+ ipmi_interfaces[i] = NULL;
+ spin_unlock_irqrestore(&interfaces_lock,flags);
+
+ kref_put(&intf->refcount, intf_free);
return 0;
}

@@ -1998,14 +2201,16 @@ static int handle_ipmb_get_msg_rsp(ipmi_
static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf,
struct ipmi_smi_msg *msg)
{
- struct cmd_rcvr *rcvr;
- int rv = 0;
- unsigned char netfn;
- unsigned char cmd;
- ipmi_user_t user = NULL;
- struct ipmi_ipmb_addr *ipmb_addr;
- struct ipmi_recv_msg *recv_msg;
- unsigned long flags;
+ struct cmd_rcvr *rcvr;
+ struct cmd_rcvr crcvr;
+ int rv = 0;
+ unsigned char netfn;
+ unsigned char cmd;
+ ipmi_user_t user = NULL;
+ struct ipmi_ipmb_addr *ipmb_addr;
+ struct ipmi_recv_msg *recv_msg;
+ unsigned long flags;
+ struct synced_list_entry *entry;

if (msg->rsp_size < 10) {
/* Message not big enough, just ignore it. */
@@ -2023,16 +2228,23 @@ static int handle_ipmb_get_msg_cmd(ipmi_
netfn = msg->rsp[4] >> 2;
cmd = msg->rsp[8];

- read_lock(&(intf->cmd_rcvr_lock));
-
- /* Find the command/netfn. */
- list_for_each_entry(rcvr, &(intf->cmd_rcvrs), link) {
- if ((rcvr->netfn == netfn) && (rcvr->cmd == cmd)) {
- user = rcvr->user;
- break;
- }
- }
- read_unlock(&(intf->cmd_rcvr_lock));
+ spin_lock_irqsave(&intf->cmd_rcvrs.lock, flags);
+ crcvr.cmd = cmd;
+ crcvr.netfn = netfn;
+ entry = synced_list_find_nolock(&intf->cmd_rcvrs, match_rcvr,
+ &crcvr);
+ if (entry) {
+ rcvr = container_of(entry, struct cmd_rcvr, link);
+ user = rcvr->user;
+ synced_list_put_entry_nolock(entry, &intf->cmd_rcvrs);
+ } else
+ user = NULL;
+
+ if (user)
+ /* Safe because the user delete must delete the user
+ from this table and grab this lock. */
+ synced_list_get_entry(&user->link);
+ spin_unlock_irqrestore(&intf->cmd_rcvrs.lock, flags);

if (user == NULL) {
/* We didn't find a user, deliver an error response. */
@@ -2104,6 +2316,7 @@ static int handle_ipmb_get_msg_cmd(ipmi_
msg->rsp_size - 10);
deliver_response(recv_msg);
}
+ synced_list_put_entry(&user->link, &intf->users);
}

return rv;
@@ -2179,14 +2392,16 @@ static int handle_lan_get_msg_rsp(ipmi_s
static int handle_lan_get_msg_cmd(ipmi_smi_t intf,
struct ipmi_smi_msg *msg)
{
- struct cmd_rcvr *rcvr;
- int rv = 0;
- unsigned char netfn;
- unsigned char cmd;
- ipmi_user_t user = NULL;
- struct ipmi_lan_addr *lan_addr;
- struct ipmi_recv_msg *recv_msg;
- unsigned long flags;
+ struct cmd_rcvr *rcvr;
+ struct cmd_rcvr crcvr;
+ int rv = 0;
+ unsigned char netfn;
+ unsigned char cmd;
+ ipmi_user_t user = NULL;
+ struct ipmi_lan_addr *lan_addr;
+ struct ipmi_recv_msg *recv_msg;
+ unsigned long flags;
+ struct synced_list_entry *entry;

if (msg->rsp_size < 12) {
/* Message not big enough, just ignore it. */
@@ -2204,19 +2419,25 @@ static int handle_lan_get_msg_cmd(ipmi_s
netfn = msg->rsp[6] >> 2;
cmd = msg->rsp[10];

- read_lock(&(intf->cmd_rcvr_lock));
-
- /* Find the command/netfn. */
- list_for_each_entry(rcvr, &(intf->cmd_rcvrs), link) {
- if ((rcvr->netfn == netfn) && (rcvr->cmd == cmd)) {
- user = rcvr->user;
- break;
- }
- }
- read_unlock(&(intf->cmd_rcvr_lock));
+ spin_lock_irqsave(&intf->cmd_rcvrs.lock, flags);
+ crcvr.cmd = cmd;
+ crcvr.netfn = netfn;
+ entry = synced_list_find_nolock(&intf->cmd_rcvrs, match_rcvr,
+ &crcvr);
+ if (entry) {
+ rcvr = container_of(entry, struct cmd_rcvr, link);
+ user = rcvr->user;
+ synced_list_put_entry_nolock(entry, &intf->cmd_rcvrs);
+ } else
+ user = NULL;
+ if (user)
+ /* Safe because the user delete must delete the user
+ from this table and grab this lock. */
+ synced_list_get_entry(&user->link);
+ spin_unlock_irqrestore(&intf->cmd_rcvrs.lock, flags);

if (user == NULL) {
- /* We didn't find a user, deliver an error response. */
+ /* We didn't find a user, just give up. */
spin_lock_irqsave(&intf->counter_lock, flags);
intf->unhandled_commands++;
spin_unlock_irqrestore(&intf->counter_lock, flags);
@@ -2263,6 +2484,7 @@ static int handle_lan_get_msg_cmd(ipmi_s
msg->rsp_size - 12);
deliver_response(recv_msg);
}
+ synced_list_put_entry(&user->link, &intf->users);
}

return rv;
@@ -2286,8 +2508,6 @@ static void copy_event_into_recv_msg(str
recv_msg->msg.data_len = msg->rsp_size - 3;
}

-/* This will be called with the intf->users_lock read-locked, so no need
- to do that here. */
static int handle_read_event_rsp(ipmi_smi_t intf,
struct ipmi_smi_msg *msg)
{
@@ -2313,7 +2533,7 @@ static int handle_read_event_rsp(ipmi_sm

INIT_LIST_HEAD(&msgs);

- spin_lock_irqsave(&(intf->events_lock), flags);
+ spin_lock_irqsave(&intf->events_lock, flags);

spin_lock(&intf->counter_lock);
intf->events++;
@@ -2321,13 +2541,16 @@ static int handle_read_event_rsp(ipmi_sm

/* Allocate and fill in one message for every user that is getting
events. */
- list_for_each_entry(user, &(intf->users), link) {
+ synced_list_for_each_entry(user, &intf->users, link, flags) {
if (! user->gets_events)
continue;

recv_msg = ipmi_alloc_recv_msg();
if (! recv_msg) {
+ synced_list_before_exit(&user->link, &intf->users);
list_for_each_entry_safe(recv_msg, recv_msg2, &msgs, link) {
+ synced_list_put_entry(&recv_msg->user->link,
+ &intf->users);
list_del(&recv_msg->link);
ipmi_free_recv_msg(recv_msg);
}
@@ -2342,6 +2565,7 @@ static int handle_read_event_rsp(ipmi_sm

copy_event_into_recv_msg(recv_msg, msg);
recv_msg->user = user;
+ synced_list_get_entry(&user->link);
list_add_tail(&(recv_msg->link), &msgs);
}

@@ -2381,10 +2605,9 @@ static int handle_read_event_rsp(ipmi_sm
static int handle_bmc_rsp(ipmi_smi_t intf,
struct ipmi_smi_msg *msg)
{
- struct ipmi_recv_msg *recv_msg;
- int found = 0;
- struct ipmi_user *user;
- unsigned long flags;
+ struct ipmi_recv_msg *recv_msg;
+ struct synced_list_entry *entry;
+ unsigned long flags;

recv_msg = (struct ipmi_recv_msg *) msg->user_data;
if (recv_msg == NULL)
@@ -2397,15 +2620,8 @@ static int handle_bmc_rsp(ipmi_smi_t
}

/* Make sure the user still exists. */
- list_for_each_entry(user, &(intf->users), link) {
- if (user == recv_msg->user) {
- /* Found it, so we can deliver it */
- found = 1;
- break;
- }
- }
-
- if ((! found) && recv_msg->user) {
+ entry = synced_list_find(&intf->users, match_user, recv_msg->user);
+ if ((! entry) && recv_msg->user) {
/* The user for the message went away, so give up. */
spin_lock_irqsave(&intf->counter_lock, flags);
intf->unhandled_local_responses++;
@@ -2486,7 +2702,8 @@ static int handle_new_recv_msg(ipmi_smi_
{
/* It's a response to a response we sent. For this we
deliver a send message response to the user. */
- struct ipmi_recv_msg *recv_msg = msg->user_data;
+ struct ipmi_recv_msg *recv_msg = msg->user_data;
+ struct synced_list_entry *entry;

requeue = 0;
if (msg->rsp_size < 2)
@@ -2498,13 +2715,20 @@ static int handle_new_recv_msg(ipmi_smi_
/* Invalid channel number */
goto out;

- if (recv_msg) {
- recv_msg->recv_type = IPMI_RESPONSE_RESPONSE_TYPE;
- recv_msg->msg.data = recv_msg->msg_data;
- recv_msg->msg.data_len = 1;
- recv_msg->msg_data[0] = msg->rsp[2];
- deliver_response(recv_msg);
- }
+ if (!recv_msg)
+ goto out;
+
+ /* Make sure the user still exists. */
+ entry = synced_list_find(&intf->users, match_user,
+ recv_msg->user);
+ if (! entry)
+ goto out;
+
+ recv_msg->recv_type = IPMI_RESPONSE_RESPONSE_TYPE;
+ recv_msg->msg.data = recv_msg->msg_data;
+ recv_msg->msg.data_len = 1;
+ recv_msg->msg_data[0] = msg->rsp[2];
+ deliver_response(recv_msg);
} else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
&& (msg->rsp[1] == IPMI_GET_MSG_CMD))
{
@@ -2570,14 +2794,11 @@ void ipmi_smi_msg_received(ipmi_smi_t
int rv;


- /* Lock the user lock so the user can't go away while we are
- working on it. */
- read_lock(&(intf->users_lock));
-
if ((msg->data_size >= 2)
&& (msg->data[0] == (IPMI_NETFN_APP_REQUEST << 2))
&& (msg->data[1] == IPMI_SEND_MSG_CMD)
- && (msg->user_data == NULL)) {
+ && (msg->user_data == NULL))
+ {
/* This is the local response to a command send, start
the timer for these. The user_data will not be
NULL if this is a response send, and we will let
@@ -2612,16 +2833,16 @@ void ipmi_smi_msg_received(ipmi_smi_t
}

ipmi_free_smi_msg(msg);
- goto out_unlock;
+ goto out;
}

/* To preserve message order, if the list is not empty, we
tack this message onto the end of the list. */
- spin_lock_irqsave(&(intf->waiting_msgs_lock), flags);
- if (!list_empty(&(intf->waiting_msgs))) {
- list_add_tail(&(msg->link), &(intf->waiting_msgs));
- spin_unlock(&(intf->waiting_msgs_lock));
- goto out_unlock;
+ spin_lock_irqsave(&intf->waiting_msgs_lock, flags);
+ if (!list_empty(&intf->waiting_msgs)) {
+ list_add_tail(&msg->link, &intf->waiting_msgs);
+ spin_unlock(&intf->waiting_msgs_lock);
+ goto out;
}
spin_unlock_irqrestore(&(intf->waiting_msgs_lock), flags);

@@ -2629,29 +2850,28 @@ void ipmi_smi_msg_received(ipmi_smi_t
if (rv > 0) {
/* Could not handle the message now, just add it to a
list to handle later. */
- spin_lock(&(intf->waiting_msgs_lock));
- list_add_tail(&(msg->link), &(intf->waiting_msgs));
- spin_unlock(&(intf->waiting_msgs_lock));
+ spin_lock(&intf->waiting_msgs_lock);
+ list_add_tail(&msg->link, &intf->waiting_msgs);
+ spin_unlock(&intf->waiting_msgs_lock);
} else if (rv == 0) {
ipmi_free_smi_msg(msg);
}

- out_unlock:
- read_unlock(&(intf->users_lock));
+ out:
+ return;
}

void ipmi_smi_watchdog_pretimeout(ipmi_smi_t intf)
{
- ipmi_user_t user;
+ unsigned long flags;
+ ipmi_user_t user;

- read_lock(&(intf->users_lock));
- list_for_each_entry(user, &(intf->users), link) {
+ synced_list_for_each_entry(user, &intf->users, link, flags) {
if (! user->handler->ipmi_watchdog_pretimeout)
continue;

user->handler->ipmi_watchdog_pretimeout(user->handler_data);
}
- read_unlock(&(intf->users_lock));
}

static void
@@ -2691,8 +2911,70 @@ smi_from_recv_msg(ipmi_smi_t intf, struc
return smi_msg;
}

-static void
-ipmi_timeout_handler(long timeout_period)
+static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent,
+ struct list_head *timeouts, long timeout_period,
+ int slot, unsigned long *flags)
+{
+ struct ipmi_recv_msg *msg;
+
+ if (!ent->inuse)
+ return;
+
+ ent->timeout -= timeout_period;
+ if (ent->timeout > 0)
+ return;
+
+ if (ent->retries_left == 0) {
+ /* The message has used all its retries. */
+ ent->inuse = 0;
+ msg = ent->recv_msg;
+ /* This is safe because removing the entry gets the
+ seq lock, and we hold the seq_lock now, so the user
+ in the recv_msg must be valid. */
+ if (msg->user)
+ synced_list_get_entry(&msg->user->link);
+ list_add_tail(&msg->link, timeouts);
+ spin_lock(&intf->counter_lock);
+ if (ent->broadcast)
+ intf->timed_out_ipmb_broadcasts++;
+ else if (ent->recv_msg->addr.addr_type == IPMI_LAN_ADDR_TYPE)
+ intf->timed_out_lan_commands++;
+ else
+ intf->timed_out_ipmb_commands++;
+ spin_unlock(&intf->counter_lock);
+ } else {
+ struct ipmi_smi_msg *smi_msg;
+ /* More retries, send again. */
+
+ /* Start with the max timer, set to normal
+ timer after the message is sent. */
+ ent->timeout = MAX_MSG_TIMEOUT;
+ ent->retries_left--;
+ spin_lock(&intf->counter_lock);
+ if (ent->recv_msg->addr.addr_type == IPMI_LAN_ADDR_TYPE)
+ intf->retransmitted_lan_commands++;
+ else
+ intf->retransmitted_ipmb_commands++;
+ spin_unlock(&intf->counter_lock);
+
+ smi_msg = smi_from_recv_msg(intf, ent->recv_msg, slot,
+ ent->seqid);
+ if (! smi_msg)
+ return;
+
+ spin_unlock_irqrestore(&intf->seq_lock, *flags);
+ /* Send the new message. We send with a zero
+ * priority. It timed out, I doubt time is
+ * that critical now, and high priority
+ * messages are really only for messages to the
+ * local MC, which don't get resent. */
+ intf->handlers->sender(intf->send_info,
+ smi_msg, 0);
+ spin_lock_irqsave(&intf->seq_lock, *flags);
+ }
+}
+
+static void ipmi_timeout_handler(long timeout_period)
{
ipmi_smi_t intf;
struct list_head timeouts;
@@ -2706,14 +2988,14 @@ ipmi_timeout_handler(long timeout_period
spin_lock(&interfaces_lock);
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
intf = ipmi_interfaces[i];
- if (intf == NULL)
+ if (IPMI_INVALID_INTERFACE(intf))
continue;
-
- read_lock(&(intf->users_lock));
+ kref_get(&intf->refcount);
+ spin_unlock(&interfaces_lock);

/* See if any waiting messages need to be processed. */
- spin_lock_irqsave(&(intf->waiting_msgs_lock), flags);
- list_for_each_entry_safe(smi_msg, smi_msg2, &(intf->waiting_msgs), link) {
+ spin_lock_irqsave(&intf->waiting_msgs_lock, flags);
+ list_for_each_entry_safe(smi_msg, smi_msg2, &intf->waiting_msgs, link) {
if (! handle_new_recv_msg(intf, smi_msg)) {
list_del(&smi_msg->link);
ipmi_free_smi_msg(smi_msg);
@@ -2723,73 +3005,23 @@ ipmi_timeout_handler(long timeout_period
break;
}
}
- spin_unlock_irqrestore(&(intf->waiting_msgs_lock), flags);
+ spin_unlock_irqrestore(&intf->waiting_msgs_lock, flags);

/* Go through the seq table and find any messages that
have timed out, putting them in the timeouts
list. */
- spin_lock_irqsave(&(intf->seq_lock), flags);
- for (j = 0; j < IPMI_IPMB_NUM_SEQ; j++) {
- struct seq_table *ent = &(intf->seq_table[j]);
- if (!ent->inuse)
- continue;
-
- ent->timeout -= timeout_period;
- if (ent->timeout > 0)
- continue;
-
- if (ent->retries_left == 0) {
- /* The message has used all its retries. */
- ent->inuse = 0;
- msg = ent->recv_msg;
- list_add_tail(&(msg->link), &timeouts);
- spin_lock(&intf->counter_lock);
- if (ent->broadcast)
- intf->timed_out_ipmb_broadcasts++;
- else if (ent->recv_msg->addr.addr_type
- == IPMI_LAN_ADDR_TYPE)
- intf->timed_out_lan_commands++;
- else
- intf->timed_out_ipmb_commands++;
- spin_unlock(&intf->counter_lock);
- } else {
- struct ipmi_smi_msg *smi_msg;
- /* More retries, send again. */
+ spin_lock_irqsave(&intf->seq_lock, flags);
+ for (j = 0; j < IPMI_IPMB_NUM_SEQ; j++)
+ check_msg_timeout(intf, &(intf->seq_table[j]),
+ &timeouts, timeout_period, j,
+ &flags);
+ spin_unlock_irqrestore(&intf->seq_lock, flags);

- /* Start with the max timer, set to normal
- timer after the message is sent. */
- ent->timeout = MAX_MSG_TIMEOUT;
- ent->retries_left--;
- spin_lock(&intf->counter_lock);
- if (ent->recv_msg->addr.addr_type
- == IPMI_LAN_ADDR_TYPE)
- intf->retransmitted_lan_commands++;
- else
- intf->retransmitted_ipmb_commands++;
- spin_unlock(&intf->counter_lock);
- smi_msg = smi_from_recv_msg(intf,
- ent->recv_msg, j, ent->seqid);
- if (! smi_msg)
- continue;
-
- spin_unlock_irqrestore(&(intf->seq_lock),flags);
- /* Send the new message. We send with a zero
- * priority. It timed out, I doubt time is
- * that critical now, and high priority
- * messages are really only for messages to the
- * local MC, which don't get resent. */
- intf->handlers->sender(intf->send_info,
- smi_msg, 0);
- spin_lock_irqsave(&(intf->seq_lock), flags);
- }
- }
- spin_unlock_irqrestore(&(intf->seq_lock), flags);
-
- list_for_each_entry_safe(msg, msg2, &timeouts, link) {
+ list_for_each_entry_safe(msg, msg2, &timeouts, link)
handle_msg_timeout(msg);
- }

- read_unlock(&(intf->users_lock));
+ kref_put(&intf->refcount, intf_free);
+ spin_lock(&interfaces_lock);
}
spin_unlock(&interfaces_lock);
}
@@ -2802,7 +3034,7 @@ static void ipmi_request_event(void)
spin_lock(&interfaces_lock);
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
intf = ipmi_interfaces[i];
- if (intf == NULL)
+ if (IPMI_INVALID_INTERFACE(intf))
continue;

intf->handlers->request_events(intf->send_info);
@@ -2964,7 +3196,7 @@ static void send_panic_events(char *str)
/* For every registered interface, send the event. */
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
intf = ipmi_interfaces[i];
- if (intf == NULL)
+ if (IPMI_INVALID_INTERFACE(intf))
continue;

/* Send the event announcing the panic. */
@@ -2995,7 +3227,7 @@ static void send_panic_events(char *str)
int j;

intf = ipmi_interfaces[i];
- if (intf == NULL)
+ if (IPMI_INVALID_INTERFACE(intf))
continue;

/* First job here is to figure out where to send the
@@ -3131,7 +3363,7 @@ static int panic_event(struct notifier_b
/* For every registered interface, set it to run to completion. */
for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
intf = ipmi_interfaces[i];
- if (intf == NULL)
+ if (IPMI_INVALID_INTERFACE(intf))
continue;

intf->handlers->set_run_to_completion(intf->send_info, 1);
@@ -3160,9 +3392,8 @@ static int ipmi_init_msghandler(void)
printk(KERN_INFO "ipmi message handler version "
IPMI_DRIVER_VERSION "\n");

- for (i = 0; i < MAX_IPMI_INTERFACES; i++) {
+ for (i = 0; i < MAX_IPMI_INTERFACES; i++)
ipmi_interfaces[i] = NULL;
- }

#ifdef CONFIG_PROC_FS
proc_ipmi_root = proc_mkdir("ipmi", NULL);