[PATCH 00/23] ipmi: Remove SRCU and handle the fallout

[PATCH 00/23] ipmi: Remove SRCU and handle the fallout

[Corey Minyard]

This patch set switches from SRCU to using normal locks.  This required
pulling most of the IPMI message handler up to run in kernel threads,
which was a good thing, anyway.  It also handles some fallout from
that change.

I didn't have a way to thoroughly test the IPMI driver, most of the
work for this was creating a test framework, which required work in
QEMU, the OpenIPMI simulator, and a few other things.  But that's
working now, and it shook out surprisingly few bugs, one in the new
changes and one driver unload issue.

Anyway, here's the first candidate for this.

[PATCH 01/23] ipmi:msghandler: Use READ_ONCE on run_to_completion

[Corey Minyard]

It needs to be read only once because it's used in lock/unlock
scenarios.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 1e5313748f8b..77f0f41f2e3d 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -1882,13 +1882,12 @@ static void smi_send(struct ipmi_smi *intf,
 		     const struct ipmi_smi_handlers *handlers,
 		     struct ipmi_smi_msg *smi_msg, int priority)
 {
-	int run_to_completion = intf->run_to_completion;
+	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	unsigned long flags = 0;
 
 	if (!run_to_completion)
 		spin_lock_irqsave(&intf->xmit_msgs_lock, flags);
 	smi_msg = smi_add_send_msg(intf, smi_msg, priority);
-
 	if (!run_to_completion)
 		spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
 
@@ -4753,10 +4752,10 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
  */
 static void handle_new_recv_msgs(struct ipmi_smi *intf)
 {
-	struct ipmi_smi_msg  *smi_msg;
-	unsigned long        flags = 0;
-	int                  rv;
-	int                  run_to_completion = intf->run_to_completion;
+	struct ipmi_smi_msg *smi_msg;
+	unsigned long flags = 0;
+	int rv;
+	int run_to_completion = READ_ONCE(intf->run_to_completion);
 
 	/* See if any waiting messages need to be processed. */
 	if (!run_to_completion)
@@ -4813,7 +4812,7 @@ static void smi_recv_work(struct work_struct *t)
 {
 	unsigned long flags = 0; /* keep us warning-free. */
 	struct ipmi_smi *intf = from_work(intf, t, recv_work);
-	int run_to_completion = intf->run_to_completion;
+	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	struct ipmi_smi_msg *newmsg = NULL;
 
 	/*
@@ -4843,9 +4842,9 @@ static void smi_recv_work(struct work_struct *t)
 			intf->curr_msg = newmsg;
 		}
 	}
-
 	if (!run_to_completion)
 		spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
+
 	if (newmsg)
 		intf->handlers->sender(intf->send_info, newmsg);
 
@@ -4859,7 +4858,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
 			   struct ipmi_smi_msg *msg)
 {
 	unsigned long flags = 0; /* keep us warning-free. */
-	int run_to_completion = intf->run_to_completion;
+	int run_to_completion = READ_ONCE(intf->run_to_completion);
 
 	/*
 	 * To preserve message order, we keep a queue and deliver from
-- 
2.43.0

[PATCH 02/23] ipmi:msghandler: Rename recv_work to smi_work

[Corey Minyard]

It handles both receive and transmit functions, make the name generic.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 77f0f41f2e3d..22813b1598b0 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -41,7 +41,7 @@
 
 static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
 static int ipmi_init_msghandler(void);
-static void smi_recv_work(struct work_struct *t);
+static void smi_work(struct work_struct *t);
 static void handle_new_recv_msgs(struct ipmi_smi *intf);
 static void need_waiter(struct ipmi_smi *intf);
 static int handle_one_recv_msg(struct ipmi_smi *intf,
@@ -504,7 +504,7 @@ struct ipmi_smi {
 	spinlock_t       waiting_rcv_msgs_lock;
 	struct list_head waiting_rcv_msgs;
 	atomic_t	 watchdog_pretimeouts_to_deliver;
-	struct work_struct recv_work;
+	struct work_struct smi_work;
 
 	spinlock_t             xmit_msgs_lock;
 	struct list_head       xmit_msgs;
@@ -704,7 +704,7 @@ static void clean_up_interface_data(struct ipmi_smi *intf)
 	struct cmd_rcvr  *rcvr, *rcvr2;
 	struct list_head list;
 
-	cancel_work_sync(&intf->recv_work);
+	cancel_work_sync(&intf->smi_work);
 
 	free_smi_msg_list(&intf->waiting_rcv_msgs);
 	free_recv_msg_list(&intf->waiting_events);
@@ -3602,7 +3602,7 @@ int ipmi_add_smi(struct module         *owner,
 	intf->curr_seq = 0;
 	spin_lock_init(&intf->waiting_rcv_msgs_lock);
 	INIT_LIST_HEAD(&intf->waiting_rcv_msgs);
-	INIT_WORK(&intf->recv_work, smi_recv_work);
+	INIT_WORK(&intf->smi_work, smi_work);
 	atomic_set(&intf->watchdog_pretimeouts_to_deliver, 0);
 	spin_lock_init(&intf->xmit_msgs_lock);
 	INIT_LIST_HEAD(&intf->xmit_msgs);
@@ -4808,10 +4808,10 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
 	}
 }
 
-static void smi_recv_work(struct work_struct *t)
+static void smi_work(struct work_struct *t)
 {
 	unsigned long flags = 0; /* keep us warning-free. */
-	struct ipmi_smi *intf = from_work(intf, t, recv_work);
+	struct ipmi_smi *intf = from_work(intf, t, smi_work);
 	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	struct ipmi_smi_msg *newmsg = NULL;
 
@@ -4883,9 +4883,9 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
 		spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
 
 	if (run_to_completion)
-		smi_recv_work(&intf->recv_work);
+		smi_work(&intf->smi_work);
 	else
-		queue_work(system_bh_wq, &intf->recv_work);
+		queue_work(system_bh_wq, &intf->smi_work);
 }
 EXPORT_SYMBOL(ipmi_smi_msg_received);
 
@@ -4895,7 +4895,7 @@ void ipmi_smi_watchdog_pretimeout(struct ipmi_smi *intf)
 		return;
 
 	atomic_set(&intf->watchdog_pretimeouts_to_deliver, 1);
-	queue_work(system_bh_wq, &intf->recv_work);
+	queue_work(system_bh_wq, &intf->smi_work);
 }
 EXPORT_SYMBOL(ipmi_smi_watchdog_pretimeout);
 
@@ -5064,7 +5064,7 @@ static bool ipmi_timeout_handler(struct ipmi_smi *intf,
 				       flags);
 	}
 
-	queue_work(system_bh_wq, &intf->recv_work);
+	queue_work(system_bh_wq, &intf->smi_work);
 
 	return need_timer;
 }
-- 
2.43.0

[PATCH 03/23] ipmi:msghandler: Move timer handling into a work queue

[Corey Minyard]

Get all operations that manipulate the interface list into thread
context.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 22813b1598b0..e08ec9918a32 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -5083,8 +5083,11 @@ static struct timer_list ipmi_timer;
 
 static atomic_t stop_operation;
 
-static void ipmi_timeout(struct timer_list *unused)
+static void ipmi_timeout_work(struct work_struct *work)
 {
+	if (atomic_read(&stop_operation))
+		return;
+
 	struct ipmi_smi *intf;
 	bool need_timer = false;
 	int index;
@@ -5111,6 +5114,16 @@ static void ipmi_timeout(struct timer_list *unused)
 		mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES);
 }
 
+static DECLARE_WORK(ipmi_timer_work, ipmi_timeout_work);
+
+static void ipmi_timeout(struct timer_list *unused)
+{
+	if (atomic_read(&stop_operation))
+		return;
+
+	queue_work(system_bh_wq, &ipmi_timer_work);
+}
+
 static void need_waiter(struct ipmi_smi *intf)
 {
 	/* Racy, but worst case we start the timer twice. */
@@ -5538,6 +5551,7 @@ static void __exit cleanup_ipmi(void)
 		 */
 		atomic_set(&stop_operation, 1);
 		del_timer_sync(&ipmi_timer);
+		cancel_work_sync(&ipmi_timer_work);
 
 		initialized = false;
 
-- 
2.43.0

[PATCH 04/23] ipmi:msghandler: Deliver user messages in a work queue

[Corey Minyard]

This simplifies the locking and lets us remove some weird event
handling code.  deliver_response() and friends can now be called
from an atomic context.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 41 ++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index e08ec9918a32..fc939b5bb7f8 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -495,6 +495,12 @@ struct ipmi_smi {
 	struct seq_table seq_table[IPMI_IPMB_NUM_SEQ];
 	int curr_seq;
 
+	/*
+	 * Messages queued for deliver to the user.
+	 */
+	struct mutex user_msgs_mutex;
+	struct list_head user_msgs;
+
 	/*
 	 * Messages queued for delivery.  If delivery fails (out of memory
 	 * for instance), They will stay in here to be processed later in a
@@ -525,7 +531,6 @@ struct ipmi_smi {
 	spinlock_t       events_lock; /* For dealing with event stuff. */
 	struct list_head waiting_events;
 	unsigned int     waiting_events_count; /* How many events in queue? */
-	char             delivering_events;
 	char             event_msg_printed;
 
 	/* How many users are waiting for events? */
@@ -945,9 +950,13 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 		struct ipmi_user *user = acquire_ipmi_user(msg->user, &index);
 
 		if (user) {
-			atomic_dec(&user->nr_msgs);
-			user->handler->ipmi_recv_hndl(msg, user->handler_data);
+			/* Deliver it in smi_work. */
+			kref_get(&user->refcount);
+			mutex_lock(&intf->user_msgs_mutex);
+			list_add_tail(&msg->link, &intf->user_msgs);
+			mutex_unlock(&intf->user_msgs_mutex);
 			release_ipmi_user(user, index);
+			queue_work(system_bh_wq, &intf->smi_work);
 		} else {
 			/* User went away, give up. */
 			ipmi_free_recv_msg(msg);
@@ -1610,13 +1619,6 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 		atomic_dec(&intf->event_waiters);
 	}
 
-	if (intf->delivering_events)
-		/*
-		 * Another thread is delivering events for this, so
-		 * let it handle any new events.
-		 */
-		goto out;
-
 	/* Deliver any queued events. */
 	while (user->gets_events && !list_empty(&intf->waiting_events)) {
 		list_for_each_entry_safe(msg, msg2, &intf->waiting_events, link)
@@ -1627,17 +1629,11 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 			intf->event_msg_printed = 0;
 		}
 
-		intf->delivering_events = 1;
-		spin_unlock_irqrestore(&intf->events_lock, flags);
-
 		list_for_each_entry_safe(msg, msg2, &msgs, link) {
 			msg->user = user;
 			kref_get(&user->refcount);
 			deliver_local_response(intf, msg);
 		}
-
-		spin_lock_irqsave(&intf->events_lock, flags);
-		intf->delivering_events = 0;
 	}
 
  out:
@@ -3590,6 +3586,8 @@ int ipmi_add_smi(struct module         *owner,
 	}
 	if (slave_addr != 0)
 		intf->addrinfo[0].address = slave_addr;
+	INIT_LIST_HEAD(&intf->user_msgs);
+	mutex_init(&intf->user_msgs_mutex);
 	INIT_LIST_HEAD(&intf->users);
 	atomic_set(&intf->nr_users, 0);
 	intf->handlers = handlers;
@@ -4814,6 +4812,7 @@ static void smi_work(struct work_struct *t)
 	struct ipmi_smi *intf = from_work(intf, t, smi_work);
 	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	struct ipmi_smi_msg *newmsg = NULL;
+	struct ipmi_recv_msg *msg, *msg2;
 
 	/*
 	 * Start the next message if available.
@@ -4851,6 +4850,16 @@ static void smi_work(struct work_struct *t)
 	rcu_read_unlock();
 
 	handle_new_recv_msgs(intf);
+
+	mutex_lock(&intf->user_msgs_mutex);
+	list_for_each_entry_safe(msg, msg2, &intf->user_msgs, link) {
+		struct ipmi_user *user = msg->user;
+
+		atomic_dec(&user->nr_msgs);
+		user->handler->ipmi_recv_hndl(msg, user->handler_data);
+		kref_put(&user->refcount, free_user);
+	}
+	mutex_unlock(&intf->user_msgs_mutex);
 }
 
 /* Handle a new message from the lower layer. */
-- 
2.43.0

[PATCH 05/23] ipmi_msghandler: Change the events lock to a mutex

[Corey Minyard]

It can only be called from thread context now.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index fc939b5bb7f8..09e56e4141b7 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -528,7 +528,7 @@ struct ipmi_smi {
 	 * Events that were queues because no one was there to receive
 	 * them.
 	 */
-	spinlock_t       events_lock; /* For dealing with event stuff. */
+	struct mutex     events_mutex; /* For dealing with event stuff. */
 	struct list_head waiting_events;
 	unsigned int     waiting_events_count; /* How many events in queue? */
 	char             event_msg_printed;
@@ -1594,7 +1594,6 @@ EXPORT_SYMBOL(ipmi_set_maintenance_mode);
 
 int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 {
-	unsigned long        flags;
 	struct ipmi_smi      *intf = user->intf;
 	struct ipmi_recv_msg *msg, *msg2;
 	struct list_head     msgs;
@@ -1606,7 +1605,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 
 	INIT_LIST_HEAD(&msgs);
 
-	spin_lock_irqsave(&intf->events_lock, flags);
+	mutex_lock(&intf->events_mutex);
 	if (user->gets_events == val)
 		goto out;
 
@@ -1637,7 +1636,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 	}
 
  out:
-	spin_unlock_irqrestore(&intf->events_lock, flags);
+	mutex_unlock(&intf->events_mutex);
 	release_ipmi_user(user, index);
 
 	return 0;
@@ -3605,7 +3604,7 @@ int ipmi_add_smi(struct module         *owner,
 	spin_lock_init(&intf->xmit_msgs_lock);
 	INIT_LIST_HEAD(&intf->xmit_msgs);
 	INIT_LIST_HEAD(&intf->hp_xmit_msgs);
-	spin_lock_init(&intf->events_lock);
+	mutex_init(&intf->events_mutex);
 	spin_lock_init(&intf->watch_lock);
 	atomic_set(&intf->event_waiters, 0);
 	intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME;
@@ -4391,7 +4390,6 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	struct list_head     msgs;
 	struct ipmi_user     *user;
 	int rv = 0, deliver_count = 0, index;
-	unsigned long        flags;
 
 	if (msg->rsp_size < 19) {
 		/* Message is too small to be an IPMB event. */
@@ -4406,7 +4404,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 
 	INIT_LIST_HEAD(&msgs);
 
-	spin_lock_irqsave(&intf->events_lock, flags);
+	mutex_lock(&intf->events_mutex);
 
 	ipmi_inc_stat(intf, events);
 
@@ -4481,7 +4479,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	}
 
  out:
-	spin_unlock_irqrestore(&intf->events_lock, flags);
+	mutex_unlock(&intf->events_mutex);
 
 	return rv;
 }
-- 
2.43.0

[PATCH 06/23] ipmi:msghandler: Use the system_wq, not system_bh_wq

[Corey Minyard]

Everything can be run in thread context now, don't use the bh one.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 09e56e4141b7..c060be93562d 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -956,7 +956,7 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 			list_add_tail(&msg->link, &intf->user_msgs);
 			mutex_unlock(&intf->user_msgs_mutex);
 			release_ipmi_user(user, index);
-			queue_work(system_bh_wq, &intf->smi_work);
+			queue_work(system_wq, &intf->smi_work);
 		} else {
 			/* User went away, give up. */
 			ipmi_free_recv_msg(msg);
@@ -4892,7 +4892,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
 	if (run_to_completion)
 		smi_work(&intf->smi_work);
 	else
-		queue_work(system_bh_wq, &intf->smi_work);
+		queue_work(system_wq, &intf->smi_work);
 }
 EXPORT_SYMBOL(ipmi_smi_msg_received);
 
@@ -4902,7 +4902,7 @@ void ipmi_smi_watchdog_pretimeout(struct ipmi_smi *intf)
 		return;
 
 	atomic_set(&intf->watchdog_pretimeouts_to_deliver, 1);
-	queue_work(system_bh_wq, &intf->smi_work);
+	queue_work(system_wq, &intf->smi_work);
 }
 EXPORT_SYMBOL(ipmi_smi_watchdog_pretimeout);
 
@@ -5071,7 +5071,7 @@ static bool ipmi_timeout_handler(struct ipmi_smi *intf,
 				       flags);
 	}
 
-	queue_work(system_bh_wq, &intf->smi_work);
+	queue_work(system_wq, &intf->smi_work);
 
 	return need_timer;
 }
@@ -5128,7 +5128,7 @@ static void ipmi_timeout(struct timer_list *unused)
 	if (atomic_read(&stop_operation))
 		return;
 
-	queue_work(system_bh_wq, &ipmi_timer_work);
+	queue_work(system_wq, &ipmi_timer_work);
 }
 
 static void need_waiter(struct ipmi_smi *intf)
-- 
2.43.0

[PATCH 07/23] ipmi:msghandler: Remove srcu from the ipmi user structure

[Corey Minyard]

With the restructures done, srcu is no longer required, and it's fairly
onerous.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 310 ++++++++++++----------------
 1 file changed, 133 insertions(+), 177 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index c060be93562d..7dd264e40957 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -180,14 +180,8 @@ MODULE_PARM_DESC(max_msgs_per_user,
 struct ipmi_user {
 	struct list_head link;
 
-	/*
-	 * Set to NULL when the user is destroyed, a pointer to myself
-	 * so srcu_dereference can be used on it.
-	 */
-	struct ipmi_user *self;
-	struct srcu_struct release_barrier;
-
 	struct kref refcount;
+	refcount_t destroyed;
 
 	/* The upper layer that handles receive messages. */
 	const struct ipmi_user_hndl *handler;
@@ -200,28 +194,25 @@ struct ipmi_user {
 	bool gets_events;
 
 	atomic_t nr_msgs;
-
-	/* Free must run in process context for RCU cleanup. */
-	struct work_struct remove_work;
 };
 
-static struct workqueue_struct *remove_work_wq;
-
-static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user, int *index)
-	__acquires(user->release_barrier)
+static void free_ipmi_user(struct kref *ref)
 {
-	struct ipmi_user *ruser;
+	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
 
-	*index = srcu_read_lock(&user->release_barrier);
-	ruser = srcu_dereference(user->self, &user->release_barrier);
-	if (!ruser)
-		srcu_read_unlock(&user->release_barrier, *index);
-	return ruser;
+	vfree(user);
 }
 
-static void release_ipmi_user(struct ipmi_user *user, int index)
+static void release_ipmi_user(struct ipmi_user *user)
 {
-	srcu_read_unlock(&user->release_barrier, index);
+	kref_put(&user->refcount, free_ipmi_user);
+}
+
+static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user)
+{
+	if (!kref_get_unless_zero(&user->refcount))
+		return NULL;
+	return user;
 }
 
 struct cmd_rcvr {
@@ -327,6 +318,8 @@ struct bmc_device {
 };
 #define to_bmc_device(x) container_of((x), struct bmc_device, pdev.dev)
 
+static struct workqueue_struct *bmc_remove_work_wq;
+
 static int bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
 			     struct ipmi_device_id *id,
 			     bool *guid_set, guid_t *guid);
@@ -451,11 +444,10 @@ struct ipmi_smi {
 	struct list_head link;
 
 	/*
-	 * The list of upper layers that are using me.  seq_lock write
-	 * protects this.  Read protection is with srcu.
+	 * The list of upper layers that are using me.
 	 */
 	struct list_head users;
-	struct srcu_struct users_srcu;
+	struct mutex users_mutex;
 	atomic_t nr_users;
 	struct device_attribute nr_users_devattr;
 	struct device_attribute nr_msgs_devattr;
@@ -502,10 +494,11 @@ struct ipmi_smi {
 	struct list_head user_msgs;
 
 	/*
-	 * Messages queued for delivery.  If delivery fails (out of memory
-	 * for instance), They will stay in here to be processed later in a
-	 * periodic timer interrupt.  The workqueue is for handling received
-	 * messages directly from the handler.
+	 * Messages queued for processing.  If processing fails (out
+	 * of memory for instance), They will stay in here to be
+	 * processed later in a periodic timer interrupt.  The
+	 * workqueue is for handling received messages directly from
+	 * the handler.
 	 */
 	spinlock_t       waiting_rcv_msgs_lock;
 	struct list_head waiting_rcv_msgs;
@@ -635,8 +628,6 @@ static DEFINE_MUTEX(ipmidriver_mutex);
 
 static LIST_HEAD(ipmi_interfaces);
 static DEFINE_MUTEX(ipmi_interfaces_mutex);
-#define ipmi_interfaces_mutex_held() \
-	lockdep_is_held(&ipmi_interfaces_mutex)
 static struct srcu_struct ipmi_interfaces_srcu;
 
 /*
@@ -710,13 +701,14 @@ static void clean_up_interface_data(struct ipmi_smi *intf)
 	struct list_head list;
 
 	cancel_work_sync(&intf->smi_work);
+	/* smi_work() can no longer be in progress after this. */
 
 	free_smi_msg_list(&intf->waiting_rcv_msgs);
 	free_recv_msg_list(&intf->waiting_events);
 
 	/*
 	 * Wholesale remove all the entries from the list in the
-	 * interface and wait for RCU to know that none are in use.
+	 * interface.
 	 */
 	mutex_lock(&intf->cmd_rcvrs_mutex);
 	INIT_LIST_HEAD(&list);
@@ -760,7 +752,7 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
 	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
-			lockdep_is_held(&smi_watchers_mutex)) {
+				lockdep_is_held(&smi_watchers_mutex)) {
 		int intf_num = READ_ONCE(intf->intf_num);
 
 		if (intf_num == -1)
@@ -784,9 +776,6 @@ int ipmi_smi_watcher_unregister(struct ipmi_smi_watcher *watcher)
 }
 EXPORT_SYMBOL(ipmi_smi_watcher_unregister);
 
-/*
- * Must be called with smi_watchers_mutex held.
- */
 static void
 call_smi_watchers(int i, struct device *dev)
 {
@@ -946,17 +935,15 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 		ipmi_free_recv_msg(msg);
 		atomic_dec(&msg->user->nr_msgs);
 	} else {
-		int index;
-		struct ipmi_user *user = acquire_ipmi_user(msg->user, &index);
+		struct ipmi_user *user = acquire_ipmi_user(msg->user);
 
 		if (user) {
 			/* Deliver it in smi_work. */
-			kref_get(&user->refcount);
 			mutex_lock(&intf->user_msgs_mutex);
 			list_add_tail(&msg->link, &intf->user_msgs);
 			mutex_unlock(&intf->user_msgs_mutex);
-			release_ipmi_user(user, index);
 			queue_work(system_wq, &intf->smi_work);
+			/* User release will happen in the work queue. */
 		} else {
 			/* User went away, give up. */
 			ipmi_free_recv_msg(msg);
@@ -1201,22 +1188,13 @@ static int intf_err_seq(struct ipmi_smi *intf,
 	return rv;
 }
 
-static void free_user_work(struct work_struct *work)
-{
-	struct ipmi_user *user = container_of(work, struct ipmi_user,
-					      remove_work);
-
-	cleanup_srcu_struct(&user->release_barrier);
-	vfree(user);
-}
-
 int ipmi_create_user(unsigned int          if_num,
 		     const struct ipmi_user_hndl *handler,
 		     void                  *handler_data,
 		     struct ipmi_user      **user)
 {
 	unsigned long flags;
-	struct ipmi_user *new_user;
+	struct ipmi_user *new_user = NULL;
 	int           rv, index;
 	struct ipmi_smi *intf;
 
@@ -1239,10 +1217,6 @@ int ipmi_create_user(unsigned int          if_num,
 	if (rv)
 		return rv;
 
-	new_user = vzalloc(sizeof(*new_user));
-	if (!new_user)
-		return -ENOMEM;
-
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
 	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
 		if (intf->intf_num == if_num)
@@ -1253,16 +1227,21 @@ int ipmi_create_user(unsigned int          if_num,
 	goto out_kfree;
 
  found:
+	if (intf->in_shutdown) {
+		rv = -ENODEV;
+		goto out_kfree;
+	}
+
 	if (atomic_add_return(1, &intf->nr_users) > max_users) {
 		rv = -EBUSY;
 		goto out_kfree;
 	}
 
-	INIT_WORK(&new_user->remove_work, free_user_work);
-
-	rv = init_srcu_struct(&new_user->release_barrier);
-	if (rv)
+	new_user = vzalloc(sizeof(*new_user));
+	if (!new_user) {
+		rv = -ENOMEM;
 		goto out_kfree;
+	}
 
 	if (!try_module_get(intf->owner)) {
 		rv = -ENODEV;
@@ -1274,14 +1253,15 @@ int ipmi_create_user(unsigned int          if_num,
 
 	atomic_set(&new_user->nr_msgs, 0);
 	kref_init(&new_user->refcount);
+	refcount_set(&new_user->destroyed, 1);
+	kref_get(&new_user->refcount); /* Destroy owns a refcount. */
 	new_user->handler = handler;
 	new_user->handler_data = handler_data;
 	new_user->intf = intf;
 	new_user->gets_events = false;
 
-	rcu_assign_pointer(new_user->self, new_user);
 	spin_lock_irqsave(&intf->seq_lock, flags);
-	list_add_rcu(&new_user->link, &intf->users);
+	list_add(&new_user->link, &intf->users);
 	spin_unlock_irqrestore(&intf->seq_lock, flags);
 	if (handler->ipmi_watchdog_pretimeout)
 		/* User wants pretimeouts, so make sure to watch for them. */
@@ -1324,14 +1304,7 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
 }
 EXPORT_SYMBOL(ipmi_get_smi_info);
 
-static void free_user(struct kref *ref)
-{
-	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
-
-	/* SRCU cleanup must happen in workqueue context. */
-	queue_work(remove_work_wq, &user->remove_work);
-}
-
+/* Must be called with intf->users_mutex held. */
 static void _ipmi_destroy_user(struct ipmi_user *user)
 {
 	struct ipmi_smi  *intf = user->intf;
@@ -1341,19 +1314,8 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	struct cmd_rcvr  *rcvrs = NULL;
 	struct module    *owner;
 
-	if (!acquire_ipmi_user(user, &i)) {
-		/*
-		 * The user has already been cleaned up, just make sure
-		 * nothing is using it and return.
-		 */
-		synchronize_srcu(&user->release_barrier);
+	if (!refcount_dec_if_one(&user->destroyed))
 		return;
-	}
-
-	rcu_assign_pointer(user->self, NULL);
-	release_ipmi_user(user, i);
-
-	synchronize_srcu(&user->release_barrier);
 
 	if (user->handler->shutdown)
 		user->handler->shutdown(user->handler_data);
@@ -1364,11 +1326,11 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	if (user->gets_events)
 		atomic_dec(&intf->event_waiters);
 
-	/* Remove the user from the interface's sequence table. */
-	spin_lock_irqsave(&intf->seq_lock, flags);
-	list_del_rcu(&user->link);
+	/* Remove the user from the interface's list and sequence table. */
+	list_del(&user->link);
 	atomic_dec(&intf->nr_users);
 
+	spin_lock_irqsave(&intf->seq_lock, flags);
 	for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) {
 		if (intf->seq_table[i].inuse
 		    && (intf->seq_table[i].recv_msg->user == user)) {
@@ -1402,6 +1364,8 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 		kfree(rcvr);
 	}
 
+	release_ipmi_user(user);
+
 	owner = intf->owner;
 	kref_put(&intf->refcount, intf_free);
 	module_put(owner);
@@ -1409,9 +1373,13 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 
 void ipmi_destroy_user(struct ipmi_user *user)
 {
+	struct ipmi_smi *intf = user->intf;
+
+	mutex_lock(&intf->users_mutex);
 	_ipmi_destroy_user(user);
+	mutex_unlock(&intf->users_mutex);
 
-	kref_put(&user->refcount, free_user);
+	kref_put(&user->refcount, free_ipmi_user);
 }
 EXPORT_SYMBOL(ipmi_destroy_user);
 
@@ -1420,9 +1388,9 @@ int ipmi_get_version(struct ipmi_user *user,
 		     unsigned char *minor)
 {
 	struct ipmi_device_id id;
-	int rv, index;
+	int rv;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1431,7 +1399,7 @@ int ipmi_get_version(struct ipmi_user *user,
 		*major = ipmi_version_major(&id);
 		*minor = ipmi_version_minor(&id);
 	}
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1441,9 +1409,9 @@ int ipmi_set_my_address(struct ipmi_user *user,
 			unsigned int  channel,
 			unsigned char address)
 {
-	int index, rv = 0;
+	int rv = 0;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1453,7 +1421,7 @@ int ipmi_set_my_address(struct ipmi_user *user,
 		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		user->intf->addrinfo[channel].address = address;
 	}
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1463,9 +1431,9 @@ int ipmi_get_my_address(struct ipmi_user *user,
 			unsigned int  channel,
 			unsigned char *address)
 {
-	int index, rv = 0;
+	int rv = 0;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1475,7 +1443,7 @@ int ipmi_get_my_address(struct ipmi_user *user,
 		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		*address = user->intf->addrinfo[channel].address;
 	}
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1485,9 +1453,9 @@ int ipmi_set_my_LUN(struct ipmi_user *user,
 		    unsigned int  channel,
 		    unsigned char LUN)
 {
-	int index, rv = 0;
+	int rv = 0;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1497,7 +1465,7 @@ int ipmi_set_my_LUN(struct ipmi_user *user,
 		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		user->intf->addrinfo[channel].lun = LUN & 0x3;
 	}
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1507,9 +1475,9 @@ int ipmi_get_my_LUN(struct ipmi_user *user,
 		    unsigned int  channel,
 		    unsigned char *address)
 {
-	int index, rv = 0;
+	int rv = 0;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1519,7 +1487,7 @@ int ipmi_get_my_LUN(struct ipmi_user *user,
 		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		*address = user->intf->addrinfo[channel].lun;
 	}
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1527,17 +1495,17 @@ EXPORT_SYMBOL(ipmi_get_my_LUN);
 
 int ipmi_get_maintenance_mode(struct ipmi_user *user)
 {
-	int mode, index;
+	int mode;
 	unsigned long flags;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
 	spin_lock_irqsave(&user->intf->maintenance_mode_lock, flags);
 	mode = user->intf->maintenance_mode;
 	spin_unlock_irqrestore(&user->intf->maintenance_mode_lock, flags);
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return mode;
 }
@@ -1552,11 +1520,11 @@ static void maintenance_mode_update(struct ipmi_smi *intf)
 
 int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode)
 {
-	int rv = 0, index;
+	int rv = 0;
 	unsigned long flags;
 	struct ipmi_smi *intf = user->intf;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1586,7 +1554,7 @@ int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode)
 	}
  out_unlock:
 	spin_unlock_irqrestore(&intf->maintenance_mode_lock, flags);
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1597,9 +1565,8 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 	struct ipmi_smi      *intf = user->intf;
 	struct ipmi_recv_msg *msg, *msg2;
 	struct list_head     msgs;
-	int index;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1637,7 +1604,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val)
 
  out:
 	mutex_unlock(&intf->events_mutex);
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return 0;
 }
@@ -1682,9 +1649,9 @@ int ipmi_register_for_cmd(struct ipmi_user *user,
 {
 	struct ipmi_smi *intf = user->intf;
 	struct cmd_rcvr *rcvr;
-	int rv = 0, index;
+	int rv = 0;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1714,7 +1681,7 @@ int ipmi_register_for_cmd(struct ipmi_user *user,
 	if (rv)
 		kfree(rcvr);
 out_release:
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 
 	return rv;
 }
@@ -1728,9 +1695,9 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user,
 	struct ipmi_smi *intf = user->intf;
 	struct cmd_rcvr *rcvr;
 	struct cmd_rcvr *rcvrs = NULL;
-	int i, rv = -ENOENT, index;
+	int i, rv = -ENOENT;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -1753,7 +1720,7 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user,
 	}
 	mutex_unlock(&intf->cmd_rcvrs_mutex);
 	synchronize_rcu();
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 	while (rcvrs) {
 		smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS);
 		rcvr = rcvrs;
@@ -2408,12 +2375,12 @@ int ipmi_request_settime(struct ipmi_user *user,
 			 unsigned int     retry_time_ms)
 {
 	unsigned char saddr = 0, lun = 0;
-	int rv, index;
+	int rv;
 
 	if (!user)
 		return -EINVAL;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -2432,7 +2399,7 @@ int ipmi_request_settime(struct ipmi_user *user,
 				    retries,
 				    retry_time_ms);
 
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 	return rv;
 }
 EXPORT_SYMBOL(ipmi_request_settime);
@@ -2447,12 +2414,12 @@ int ipmi_request_supply_msgs(struct ipmi_user     *user,
 			     int                  priority)
 {
 	unsigned char saddr = 0, lun = 0;
-	int rv, index;
+	int rv;
 
 	if (!user)
 		return -EINVAL;
 
-	user = acquire_ipmi_user(user, &index);
+	user = acquire_ipmi_user(user);
 	if (!user)
 		return -ENODEV;
 
@@ -2471,7 +2438,7 @@ int ipmi_request_supply_msgs(struct ipmi_user     *user,
 				    lun,
 				    -1, 0);
 
-	release_ipmi_user(user, index);
+	release_ipmi_user(user);
 	return rv;
 }
 EXPORT_SYMBOL(ipmi_request_supply_msgs);
@@ -3058,7 +3025,7 @@ cleanup_bmc_device(struct kref *ref)
 	 * with removing the device attributes while reading a device
 	 * attribute.
 	 */
-	queue_work(remove_work_wq, &bmc->remove_work);
+	queue_work(bmc_remove_work_wq, &bmc->remove_work);
 }
 
 /*
@@ -3514,15 +3481,14 @@ static ssize_t nr_msgs_show(struct device *dev,
 			    char *buf)
 {
 	struct ipmi_smi *intf = container_of(attr,
-			 struct ipmi_smi, nr_msgs_devattr);
+					     struct ipmi_smi, nr_msgs_devattr);
 	struct ipmi_user *user;
-	int index;
 	unsigned int count = 0;
 
-	index = srcu_read_lock(&intf->users_srcu);
-	list_for_each_entry_rcu(user, &intf->users, link)
+	mutex_lock(&intf->users_mutex);
+	list_for_each_entry(user, &intf->users, link)
 		count += atomic_read(&user->nr_msgs);
-	srcu_read_unlock(&intf->users_srcu, index);
+	mutex_unlock(&intf->users_mutex);
 
 	return sysfs_emit(buf, "%u\n", count);
 }
@@ -3563,12 +3529,6 @@ int ipmi_add_smi(struct module         *owner,
 	if (!intf)
 		return -ENOMEM;
 
-	rv = init_srcu_struct(&intf->users_srcu);
-	if (rv) {
-		kfree(intf);
-		return rv;
-	}
-
 	intf->owner = owner;
 	intf->bmc = &intf->tmp_bmc;
 	INIT_LIST_HEAD(&intf->bmc->intfs);
@@ -3588,6 +3548,7 @@ int ipmi_add_smi(struct module         *owner,
 	INIT_LIST_HEAD(&intf->user_msgs);
 	mutex_init(&intf->user_msgs_mutex);
 	INIT_LIST_HEAD(&intf->users);
+	mutex_init(&intf->users_mutex);
 	atomic_set(&intf->nr_users, 0);
 	intf->handlers = handlers;
 	intf->send_info = send_info;
@@ -3688,7 +3649,6 @@ int ipmi_add_smi(struct module         *owner,
 	list_del_rcu(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
 	synchronize_srcu(&ipmi_interfaces_srcu);
-	cleanup_srcu_struct(&intf->users_srcu);
 	kref_put(&intf->refcount, intf_free);
 
 	return rv;
@@ -3754,7 +3714,7 @@ static void cleanup_smi_msgs(struct ipmi_smi *intf)
 void ipmi_unregister_smi(struct ipmi_smi *intf)
 {
 	struct ipmi_smi_watcher *w;
-	int intf_num, index;
+	int intf_num;
 
 	if (!intf)
 		return;
@@ -3780,15 +3740,14 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
 		w->smi_gone(intf_num);
 	mutex_unlock(&smi_watchers_mutex);
 
-	index = srcu_read_lock(&intf->users_srcu);
+	mutex_lock(&intf->users_mutex);
 	while (!list_empty(&intf->users)) {
-		struct ipmi_user *user =
-			container_of(list_next_rcu(&intf->users),
-				     struct ipmi_user, link);
+		struct ipmi_user *user = list_first_entry(&intf->users,
+						    struct ipmi_user, link);
 
 		_ipmi_destroy_user(user);
 	}
-	srcu_read_unlock(&intf->users_srcu, index);
+	mutex_unlock(&intf->users_mutex);
 
 	if (intf->handlers->shutdown)
 		intf->handlers->shutdown(intf->send_info);
@@ -3797,7 +3756,6 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
 
 	ipmi_bmc_unregister(intf);
 
-	cleanup_srcu_struct(&intf->users_srcu);
 	kref_put(&intf->refcount, intf_free);
 }
 EXPORT_SYMBOL(ipmi_unregister_smi);
@@ -3942,7 +3900,7 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf,
 			 * later.
 			 */
 			rv = 1;
-			kref_put(&user->refcount, free_user);
+			kref_put(&user->refcount, free_ipmi_user);
 		} else {
 			/* Extract the source address from the data. */
 			ipmb_addr = (struct ipmi_ipmb_addr *) &recv_msg->addr;
@@ -4033,7 +3991,7 @@ static int handle_ipmb_direct_rcv_cmd(struct ipmi_smi *intf,
 			 * later.
 			 */
 			rv = 1;
-			kref_put(&user->refcount, free_user);
+			kref_put(&user->refcount, free_ipmi_user);
 		} else {
 			/* Extract the source address from the data. */
 			daddr = (struct ipmi_ipmb_direct_addr *)&recv_msg->addr;
@@ -4218,7 +4176,7 @@ static int handle_lan_get_msg_cmd(struct ipmi_smi *intf,
 			 * message, so requeue it for handling later.
 			 */
 			rv = 1;
-			kref_put(&user->refcount, free_user);
+			kref_put(&user->refcount, free_ipmi_user);
 		} else {
 			/* Extract the source address from the data. */
 			lan_addr = (struct ipmi_lan_addr *) &recv_msg->addr;
@@ -4327,7 +4285,7 @@ static int handle_oem_get_msg_cmd(struct ipmi_smi *intf,
 			 * later.
 			 */
 			rv = 1;
-			kref_put(&user->refcount, free_user);
+			kref_put(&user->refcount, free_ipmi_user);
 		} else {
 			/*
 			 * OEM Messages are expected to be delivered via
@@ -4389,7 +4347,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	struct ipmi_recv_msg *recv_msg, *recv_msg2;
 	struct list_head     msgs;
 	struct ipmi_user     *user;
-	int rv = 0, deliver_count = 0, index;
+	int rv = 0, deliver_count = 0;
 
 	if (msg->rsp_size < 19) {
 		/* Message is too small to be an IPMB event. */
@@ -4412,18 +4370,20 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	 * Allocate and fill in one message for every user that is
 	 * getting events.
 	 */
-	index = srcu_read_lock(&intf->users_srcu);
-	list_for_each_entry_rcu(user, &intf->users, link) {
+	mutex_lock(&intf->users_mutex);
+	list_for_each_entry(user, &intf->users, link) {
 		if (!user->gets_events)
 			continue;
 
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
-			rcu_read_unlock();
+			mutex_unlock(&intf->users_mutex);
 			list_for_each_entry_safe(recv_msg, recv_msg2, &msgs,
 						 link) {
+				user = recv_msg->user;
 				list_del(&recv_msg->link);
 				ipmi_free_recv_msg(recv_msg);
+				kref_put(&user->refcount, free_ipmi_user);
 			}
 			/*
 			 * We couldn't allocate memory for the
@@ -4441,7 +4401,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 		kref_get(&user->refcount);
 		list_add_tail(&recv_msg->link, &msgs);
 	}
-	srcu_read_unlock(&intf->users_srcu, index);
+	mutex_unlock(&intf->users_mutex);
 
 	if (deliver_count) {
 		/* Now deliver all the messages. */
@@ -4785,23 +4745,6 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
 	}
 	if (!run_to_completion)
 		spin_unlock_irqrestore(&intf->waiting_rcv_msgs_lock, flags);
-
-	/*
-	 * If the pretimout count is non-zero, decrement one from it and
-	 * deliver pretimeouts to all the users.
-	 */
-	if (atomic_add_unless(&intf->watchdog_pretimeouts_to_deliver, -1, 0)) {
-		struct ipmi_user *user;
-		int index;
-
-		index = srcu_read_lock(&intf->users_srcu);
-		list_for_each_entry_rcu(user, &intf->users, link) {
-			if (user->handler->ipmi_watchdog_pretimeout)
-				user->handler->ipmi_watchdog_pretimeout(
-					user->handler_data);
-		}
-		srcu_read_unlock(&intf->users_srcu, index);
-	}
 }
 
 static void smi_work(struct work_struct *t)
@@ -4820,8 +4763,6 @@ static void smi_work(struct work_struct *t)
 	 * message delivery.
 	 */
 
-	rcu_read_lock();
-
 	if (!run_to_completion)
 		spin_lock_irqsave(&intf->xmit_msgs_lock, flags);
 	if (intf->curr_msg == NULL && !intf->in_shutdown) {
@@ -4845,17 +4786,32 @@ static void smi_work(struct work_struct *t)
 	if (newmsg)
 		intf->handlers->sender(intf->send_info, newmsg);
 
-	rcu_read_unlock();
-
 	handle_new_recv_msgs(intf);
 
+	/*
+	 * If the pretimout count is non-zero, decrement one from it and
+	 * deliver pretimeouts to all the users.
+	 */
+	if (atomic_add_unless(&intf->watchdog_pretimeouts_to_deliver, -1, 0)) {
+		struct ipmi_user *user;
+
+		mutex_lock(&intf->users_mutex);
+		list_for_each_entry(user, &intf->users, link) {
+			if (user->handler->ipmi_watchdog_pretimeout)
+				user->handler->ipmi_watchdog_pretimeout(
+					user->handler_data);
+		}
+		mutex_unlock(&intf->users_mutex);
+	}
+
 	mutex_lock(&intf->user_msgs_mutex);
 	list_for_each_entry_safe(msg, msg2, &intf->user_msgs, link) {
 		struct ipmi_user *user = msg->user;
 
+		list_del(&msg->link);
 		atomic_dec(&user->nr_msgs);
 		user->handler->ipmi_recv_hndl(msg, user->handler_data);
-		kref_put(&user->refcount, free_user);
+		release_ipmi_user(user);
 	}
 	mutex_unlock(&intf->user_msgs_mutex);
 }
@@ -5187,7 +5143,7 @@ static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void)
 void ipmi_free_recv_msg(struct ipmi_recv_msg *msg)
 {
 	if (msg->user && !oops_in_progress)
-		kref_put(&msg->user->refcount, free_user);
+		kref_put(&msg->user->refcount, free_ipmi_user);
 	msg->done(msg);
 }
 EXPORT_SYMBOL(ipmi_free_recv_msg);
@@ -5422,7 +5378,7 @@ static int panic_event(struct notifier_block *this,
 	has_panicked = 1;
 
 	/* For every registered interface, set it to run to completion. */
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry(intf, &ipmi_interfaces, link) {
 		if (!intf->handlers || intf->intf_num == -1)
 			/* Interface is not ready. */
 			continue;
@@ -5452,7 +5408,7 @@ static int panic_event(struct notifier_block *this,
 			intf->handlers->set_run_to_completion(intf->send_info,
 							      1);
 
-		list_for_each_entry_rcu(user, &intf->users, link) {
+		list_for_each_entry(user, &intf->users, link) {
 			if (user->handler->ipmi_panic_handler)
 				user->handler->ipmi_panic_handler(
 					user->handler_data);
@@ -5501,8 +5457,8 @@ static int ipmi_init_msghandler(void)
 	if (rv)
 		goto out;
 
-	remove_work_wq = create_singlethread_workqueue("ipmi-msghandler-remove-wq");
-	if (!remove_work_wq) {
+	bmc_remove_work_wq = create_singlethread_workqueue("ipmi-msghandler-remove-wq");
+	if (!bmc_remove_work_wq) {
 		pr_err("unable to create ipmi-msghandler-remove-wq workqueue");
 		rv = -ENOMEM;
 		goto out_wq;
@@ -5541,7 +5497,7 @@ static void __exit cleanup_ipmi(void)
 	int count;
 
 	if (initialized) {
-		destroy_workqueue(remove_work_wq);
+		destroy_workqueue(bmc_remove_work_wq);
 
 		atomic_notifier_chain_unregister(&panic_notifier_list,
 						 &panic_block);
-- 
2.43.0

[PATCH 08/23] ipmi:msghandler: Remove srcu for the ipmi_interfaces list

[Corey Minyard]

With reworks srcu is no longer necessary, this simplifies locking a lot.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 194 +++++++++++++++-------------
 1 file changed, 102 insertions(+), 92 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 7dd264e40957..3e88ac6831d8 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -628,7 +628,6 @@ static DEFINE_MUTEX(ipmidriver_mutex);
 
 static LIST_HEAD(ipmi_interfaces);
 static DEFINE_MUTEX(ipmi_interfaces_mutex);
-static struct srcu_struct ipmi_interfaces_srcu;
 
 /*
  * List of watchers that want to know when smi's are added and deleted.
@@ -694,28 +693,20 @@ static void free_smi_msg_list(struct list_head *q)
 	}
 }
 
-static void clean_up_interface_data(struct ipmi_smi *intf)
+static void intf_free(struct kref *ref)
 {
+	struct ipmi_smi *intf = container_of(ref, struct ipmi_smi, refcount);
 	int              i;
 	struct cmd_rcvr  *rcvr, *rcvr2;
-	struct list_head list;
-
-	cancel_work_sync(&intf->smi_work);
-	/* smi_work() can no longer be in progress after this. */
 
 	free_smi_msg_list(&intf->waiting_rcv_msgs);
 	free_recv_msg_list(&intf->waiting_events);
 
 	/*
 	 * Wholesale remove all the entries from the list in the
-	 * interface.
+	 * interface.  No need for locks, this is single-threaded.
 	 */
-	mutex_lock(&intf->cmd_rcvrs_mutex);
-	INIT_LIST_HEAD(&list);
-	list_splice_init_rcu(&intf->cmd_rcvrs, &list, synchronize_rcu);
-	mutex_unlock(&intf->cmd_rcvrs_mutex);
-
-	list_for_each_entry_safe(rcvr, rcvr2, &list, link)
+	list_for_each_entry_safe(rcvr, rcvr2, &intf->cmd_rcvrs, link)
 		kfree(rcvr);
 
 	for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) {
@@ -723,20 +714,17 @@ static void clean_up_interface_data(struct ipmi_smi *intf)
 					&& (intf->seq_table[i].recv_msg))
 			ipmi_free_recv_msg(intf->seq_table[i].recv_msg);
 	}
-}
 
-static void intf_free(struct kref *ref)
-{
-	struct ipmi_smi *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)
 {
 	struct ipmi_smi *intf;
-	int index, rv;
+	unsigned int count = 0, i;
+	int *interfaces = NULL;
+	struct device **devices = NULL;
+	int rv = 0;
 
 	/*
 	 * Make sure the driver is actually initialized, this handles
@@ -750,20 +738,53 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 
 	list_add(&watcher->link, &smi_watchers);
 
-	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
-				lockdep_is_held(&smi_watchers_mutex)) {
-		int intf_num = READ_ONCE(intf->intf_num);
+	/*
+	 * Build an array of ipmi interfaces and fill it in, and
+	 * another array of the devices.  We can't call the callback
+	 * with ipmi_interfaces_mutex held.  smi_watchers_mutex will
+	 * keep things in order for the user.
+	 */
+	mutex_lock(&ipmi_interfaces_mutex);
+	list_for_each_entry(intf, &ipmi_interfaces, link)
+		count++;
+	if (count > 0) {
+		interfaces = kmalloc_array(count, sizeof(*interfaces),
+					   GFP_KERNEL);
+		if (!interfaces) {
+			rv = -ENOMEM;
+		} else {
+			devices = kmalloc_array(count, sizeof(*devices),
+						GFP_KERNEL);
+			if (!devices) {
+				kfree(interfaces);
+				interfaces = NULL;
+				rv = -ENOMEM;
+			}
+		}
+		count = 0;
+	}
+	if (interfaces) {
+		list_for_each_entry(intf, &ipmi_interfaces, link) {
+			int intf_num = READ_ONCE(intf->intf_num);
 
-		if (intf_num == -1)
-			continue;
-		watcher->new_smi(intf_num, intf->si_dev);
+			if (intf_num == -1)
+				continue;
+			devices[count] = intf->si_dev;
+			interfaces[count++] = intf_num;
+		}
+	}
+	mutex_unlock(&ipmi_interfaces_mutex);
+
+	if (interfaces) {
+		for (i = 0; i < count; i++)
+			watcher->new_smi(interfaces[i], devices[i]);
+		kfree(interfaces);
+		kfree(devices);
 	}
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
 
 	mutex_unlock(&smi_watchers_mutex);
 
-	return 0;
+	return rv;
 }
 EXPORT_SYMBOL(ipmi_smi_watcher_register);
 
@@ -781,14 +802,12 @@ call_smi_watchers(int i, struct device *dev)
 {
 	struct ipmi_smi_watcher *w;
 
-	mutex_lock(&smi_watchers_mutex);
 	list_for_each_entry(w, &smi_watchers, link) {
 		if (try_module_get(w->owner)) {
 			w->new_smi(i, dev);
 			module_put(w->owner);
 		}
 	}
-	mutex_unlock(&smi_watchers_mutex);
 }
 
 static int
@@ -1195,7 +1214,7 @@ int ipmi_create_user(unsigned int          if_num,
 {
 	unsigned long flags;
 	struct ipmi_user *new_user = NULL;
-	int           rv, index;
+	int           rv = 0;
 	struct ipmi_smi *intf;
 
 	/*
@@ -1217,8 +1236,8 @@ int ipmi_create_user(unsigned int          if_num,
 	if (rv)
 		return rv;
 
-	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	mutex_lock(&ipmi_interfaces_mutex);
+	list_for_each_entry(intf, &ipmi_interfaces, link) {
 		if (intf->intf_num == if_num)
 			goto found;
 	}
@@ -1260,45 +1279,44 @@ int ipmi_create_user(unsigned int          if_num,
 	new_user->intf = intf;
 	new_user->gets_events = false;
 
+	mutex_lock(&intf->users_mutex);
 	spin_lock_irqsave(&intf->seq_lock, flags);
 	list_add(&new_user->link, &intf->users);
 	spin_unlock_irqrestore(&intf->seq_lock, flags);
+	mutex_unlock(&intf->users_mutex);
+
 	if (handler->ipmi_watchdog_pretimeout)
 		/* User wants pretimeouts, so make sure to watch for them. */
 		smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG);
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
-	*user = new_user;
-	return 0;
 
 out_kfree:
-	atomic_dec(&intf->nr_users);
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
-	vfree(new_user);
+	if (rv) {
+		atomic_dec(&intf->nr_users);
+		vfree(new_user);
+	} else {
+		*user = new_user;
+	}
+	mutex_unlock(&ipmi_interfaces_mutex);
 	return rv;
 }
 EXPORT_SYMBOL(ipmi_create_user);
 
 int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
 {
-	int rv, index;
+	int rv = -EINVAL;
 	struct ipmi_smi *intf;
 
-	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
-		if (intf->intf_num == if_num)
-			goto found;
+	mutex_lock(&ipmi_interfaces_mutex);
+	list_for_each_entry(intf, &ipmi_interfaces, link) {
+		if (intf->intf_num == if_num) {
+			if (!intf->handlers->get_smi_info)
+				rv = -ENOTTY;
+			else
+				rv = intf->handlers->get_smi_info(intf->send_info, data);
+			break;
+		}
 	}
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
-
-	/* Not found, return an error */
-	return -EINVAL;
-
-found:
-	if (!intf->handlers->get_smi_info)
-		rv = -ENOTTY;
-	else
-		rv = intf->handlers->get_smi_info(intf->send_info, data);
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
+	mutex_unlock(&ipmi_interfaces_mutex);
 
 	return rv;
 }
@@ -1345,7 +1363,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	 * Remove the user from the command receiver's table.  First
 	 * we build a list of everything (not using the standard link,
 	 * since other things may be using it till we do
-	 * synchronize_srcu()) then free everything in that list.
+	 * synchronize_rcu()) then free everything in that list.
 	 */
 	mutex_lock(&intf->cmd_rcvrs_mutex);
 	list_for_each_entry_rcu(rcvr, &intf->cmd_rcvrs, link,
@@ -1357,7 +1375,6 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 		}
 	}
 	mutex_unlock(&intf->cmd_rcvrs_mutex);
-	synchronize_rcu();
 	while (rcvrs) {
 		rcvr = rcvrs;
 		rcvrs = rcvr->next;
@@ -2298,7 +2315,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 		}
 	}
 
-	rcu_read_lock();
+	mutex_lock(&ipmi_interfaces_mutex);
 	if (intf->in_shutdown) {
 		rv = -ENODEV;
 		goto out_err;
@@ -2344,7 +2361,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 
 		smi_send(intf, intf->handlers, smi_msg, priority);
 	}
-	rcu_read_unlock();
+	mutex_unlock(&ipmi_interfaces_mutex);
 
 out:
 	if (rv && user)
@@ -3578,12 +3595,16 @@ int ipmi_add_smi(struct module         *owner,
 	for (i = 0; i < IPMI_NUM_STATS; i++)
 		atomic_set(&intf->stats[i], 0);
 
+	/*
+	 * Grab the watchers mutex so we can deliver the new interface
+	 * without races.
+	 */
+	mutex_lock(&smi_watchers_mutex);
 	mutex_lock(&ipmi_interfaces_mutex);
 	/* Look for a hole in the numbers. */
 	i = 0;
 	link = &ipmi_interfaces;
-	list_for_each_entry_rcu(tintf, &ipmi_interfaces, link,
-				ipmi_interfaces_mutex_held()) {
+	list_for_each_entry(tintf, &ipmi_interfaces, link) {
 		if (tintf->intf_num != i) {
 			link = &tintf->link;
 			break;
@@ -3592,9 +3613,9 @@ int ipmi_add_smi(struct module         *owner,
 	}
 	/* Add the new interface in numeric order. */
 	if (i == 0)
-		list_add_rcu(&intf->link, &ipmi_interfaces);
+		list_add(&intf->link, &ipmi_interfaces);
 	else
-		list_add_tail_rcu(&intf->link, link);
+		list_add_tail(&intf->link, link);
 
 	rv = handlers->start_processing(send_info, intf);
 	if (rv)
@@ -3626,18 +3647,14 @@ int ipmi_add_smi(struct module         *owner,
 		goto out_err_bmc_reg;
 	}
 
-	/*
-	 * Keep memory order straight for RCU readers.  Make
-	 * sure everything else is committed to memory before
-	 * setting intf_num to mark the interface valid.
-	 */
-	smp_wmb();
 	intf->intf_num = i;
 	mutex_unlock(&ipmi_interfaces_mutex);
 
 	/* After this point the interface is legal to use. */
 	call_smi_watchers(i, intf->si_dev);
 
+	mutex_unlock(&smi_watchers_mutex);
+
 	return 0;
 
  out_err_bmc_reg:
@@ -3646,9 +3663,9 @@ int ipmi_add_smi(struct module         *owner,
 	if (intf->handlers->shutdown)
 		intf->handlers->shutdown(intf->send_info);
  out_err:
-	list_del_rcu(&intf->link);
+	list_del(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
-	synchronize_srcu(&ipmi_interfaces_srcu);
+	mutex_unlock(&smi_watchers_mutex);
 	kref_put(&intf->refcount, intf_free);
 
 	return rv;
@@ -3718,13 +3735,16 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
 
 	if (!intf)
 		return;
+
 	intf_num = intf->intf_num;
 	mutex_lock(&ipmi_interfaces_mutex);
+	cancel_work_sync(&intf->smi_work);
+	/* smi_work() can no longer be in progress after this. */
+
 	intf->intf_num = -1;
 	intf->in_shutdown = true;
-	list_del_rcu(&intf->link);
+	list_del(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
-	synchronize_srcu(&ipmi_interfaces_srcu);
 
 	/* At this point no users can be added to the interface. */
 
@@ -3880,7 +3900,7 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf,
 		dev_dbg(intf->si_dev, "Invalid command: %*ph\n",
 			msg->data_size, msg->data);
 
-		rcu_read_lock();
+		mutex_lock(&ipmi_interfaces_mutex);
 		if (!intf->in_shutdown) {
 			smi_send(intf, intf->handlers, msg, 0);
 			/*
@@ -3890,7 +3910,7 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf,
 			 */
 			rv = -1;
 		}
-		rcu_read_unlock();
+		mutex_unlock(&ipmi_interfaces_mutex);
 	} else {
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
@@ -3971,7 +3991,7 @@ static int handle_ipmb_direct_rcv_cmd(struct ipmi_smi *intf,
 		msg->data[4] = IPMI_INVALID_CMD_COMPLETION_CODE;
 		msg->data_size = 5;
 
-		rcu_read_lock();
+		mutex_lock(&ipmi_interfaces_mutex);
 		if (!intf->in_shutdown) {
 			smi_send(intf, intf->handlers, msg, 0);
 			/*
@@ -3981,7 +4001,7 @@ static int handle_ipmb_direct_rcv_cmd(struct ipmi_smi *intf,
 			 */
 			rv = -1;
 		}
-		rcu_read_unlock();
+		mutex_unlock(&ipmi_interfaces_mutex);
 	} else {
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
@@ -5053,13 +5073,12 @@ static void ipmi_timeout_work(struct work_struct *work)
 
 	struct ipmi_smi *intf;
 	bool need_timer = false;
-	int index;
 
 	if (atomic_read(&stop_operation))
 		return;
 
-	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	mutex_lock(&ipmi_interfaces_mutex);
+	list_for_each_entry(intf, &ipmi_interfaces, link) {
 		if (atomic_read(&intf->event_waiters)) {
 			intf->ticks_to_req_ev--;
 			if (intf->ticks_to_req_ev == 0) {
@@ -5071,7 +5090,7 @@ static void ipmi_timeout_work(struct work_struct *work)
 
 		need_timer |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME);
 	}
-	srcu_read_unlock(&ipmi_interfaces_srcu, index);
+	mutex_unlock(&ipmi_interfaces_mutex);
 
 	if (need_timer)
 		mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES);
@@ -5453,15 +5472,11 @@ static int ipmi_init_msghandler(void)
 	if (initialized)
 		goto out;
 
-	rv = init_srcu_struct(&ipmi_interfaces_srcu);
-	if (rv)
-		goto out;
-
 	bmc_remove_work_wq = create_singlethread_workqueue("ipmi-msghandler-remove-wq");
 	if (!bmc_remove_work_wq) {
 		pr_err("unable to create ipmi-msghandler-remove-wq workqueue");
 		rv = -ENOMEM;
-		goto out_wq;
+		goto out;
 	}
 
 	timer_setup(&ipmi_timer, ipmi_timeout, 0);
@@ -5471,9 +5486,6 @@ static int ipmi_init_msghandler(void)
 
 	initialized = true;
 
-out_wq:
-	if (rv)
-		cleanup_srcu_struct(&ipmi_interfaces_srcu);
 out:
 	mutex_unlock(&ipmi_interfaces_mutex);
 	return rv;
@@ -5525,8 +5537,6 @@ static void __exit cleanup_ipmi(void)
 		count = atomic_read(&recv_msg_inuse_count);
 		if (count != 0)
 			pr_warn("recv message count %d at exit\n", count);
-
-		cleanup_srcu_struct(&ipmi_interfaces_srcu);
 	}
 	if (drvregistered)
 		driver_unregister(&ipmidriver.driver);
-- 
2.43.0

[PATCH 09/23] ipmi:watchdog: Change lock to mutex

[Corey Minyard]

Now that the msghandler does all callbacks in user threads, there is
no need to have a lock any more, a mutex will work fine.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index f1875b2bebbc..01c10bd5f099 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -150,7 +150,7 @@ static char preaction[16] = "pre_none";
 static unsigned char preop_val = WDOG_PREOP_NONE;
 
 static char preop[16] = "preop_none";
-static DEFINE_SPINLOCK(ipmi_read_lock);
+static DEFINE_MUTEX(ipmi_read_mutex);
 static char data_to_read;
 static DECLARE_WAIT_QUEUE_HEAD(read_q);
 static struct fasync_struct *fasync_q;
@@ -793,7 +793,7 @@ static ssize_t ipmi_read(struct file *file,
 	 * Reading returns if the pretimeout has gone off, and it only does
 	 * it once per pretimeout.
 	 */
-	spin_lock_irq(&ipmi_read_lock);
+	mutex_lock(&ipmi_read_mutex);
 	if (!data_to_read) {
 		if (file->f_flags & O_NONBLOCK) {
 			rv = -EAGAIN;
@@ -804,9 +804,9 @@ static ssize_t ipmi_read(struct file *file,
 		add_wait_queue(&read_q, &wait);
 		while (!data_to_read && !signal_pending(current)) {
 			set_current_state(TASK_INTERRUPTIBLE);
-			spin_unlock_irq(&ipmi_read_lock);
+			mutex_unlock(&ipmi_read_mutex);
 			schedule();
-			spin_lock_irq(&ipmi_read_lock);
+			mutex_lock(&ipmi_read_mutex);
 		}
 		remove_wait_queue(&read_q, &wait);
 
@@ -818,7 +818,7 @@ static ssize_t ipmi_read(struct file *file,
 	data_to_read = 0;
 
  out:
-	spin_unlock_irq(&ipmi_read_lock);
+	mutex_unlock(&ipmi_read_mutex);
 
 	if (rv == 0) {
 		if (copy_to_user(buf, &data_to_read, 1))
@@ -856,10 +856,10 @@ static __poll_t ipmi_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &read_q, wait);
 
-	spin_lock_irq(&ipmi_read_lock);
+	mutex_lock(&ipmi_read_mutex);
 	if (data_to_read)
 		mask |= (EPOLLIN | EPOLLRDNORM);
-	spin_unlock_irq(&ipmi_read_lock);
+	mutex_unlock(&ipmi_read_mutex);
 
 	return mask;
 }
@@ -932,13 +932,11 @@ static void ipmi_wdog_pretimeout_handler(void *handler_data)
 			if (atomic_inc_and_test(&preop_panic_excl))
 				panic("Watchdog pre-timeout");
 		} else if (preop_val == WDOG_PREOP_GIVE_DATA) {
-			unsigned long flags;
-
-			spin_lock_irqsave(&ipmi_read_lock, flags);
+			mutex_lock(&ipmi_read_mutex);
 			data_to_read = 1;
 			wake_up_interruptible(&read_q);
 			kill_fasync(&fasync_q, SIGIO, POLL_IN);
-			spin_unlock_irqrestore(&ipmi_read_lock, flags);
+			mutex_unlock(&ipmi_read_mutex);
 		}
 	}
 
-- 
2.43.0

[PATCH 10/23] ipmi: Add a note about the pretimeout callback

[Corey Minyard]

You can't do IPMI calls from the callback, it's called with locks
held.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 include/linux/ipmi.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h
index 2f74dd90c271..27cd5980bb27 100644
--- a/include/linux/ipmi.h
+++ b/include/linux/ipmi.h
@@ -93,7 +93,8 @@ struct ipmi_user_hndl {
 
 	/*
 	 * Called when the interface detects a watchdog pre-timeout.  If
-	 * this is NULL, it will be ignored for the user.
+	 * this is NULL, it will be ignored for the user.  Note that you
+	 * can't do any IPMI calls from here, it's called with locks held.
 	 */
 	void (*ipmi_watchdog_pretimeout)(void *handler_data);
 
-- 
2.43.0

[PATCH 11/23] ipmi:msghandler: Remove some user level processing in panic mode

[Corey Minyard]

When run to completion is set, don't call things that will claim
mutexes or call user callbacks.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 3e88ac6831d8..e7bed764b4bb 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -4808,6 +4808,10 @@ static void smi_work(struct work_struct *t)
 
 	handle_new_recv_msgs(intf);
 
+	/* Nothing below applies during panic time. */
+	if (run_to_completion)
+		return;
+
 	/*
 	 * If the pretimout count is non-zero, decrement one from it and
 	 * deliver pretimeouts to all the users.
-- 
2.43.0

[PATCH 12/23] ipmi:msghandler: Fix locking around users and interfaces

[Corey Minyard]

Now that SRCU is gone from IPMI, it can no longer be sloppy about
locking.  Use the users mutex now when sending a message, not the big
ipmi_interfaces mutex, because it can result in a recursive lock.  The
users mutex will work because the interface destroy code claims it after
setting the interface in shutdown mode.

Also, due to the same changes, rework the refcounting on users and
interfaces.  Remove the refcount to an interface when the user is
freed, not when it is destroyed.  If the interface is destroyed
while the user still exists, the user will still point to the
interface to test that it is valid if the user tries to do anything
but delete the user.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 51 ++++++++++++++---------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index e7bed764b4bb..d05032dc1f69 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -46,6 +46,7 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf);
 static void need_waiter(struct ipmi_smi *intf);
 static int handle_one_recv_msg(struct ipmi_smi *intf,
 			       struct ipmi_smi_msg *msg);
+static void intf_free(struct kref *ref);
 
 static bool initialized;
 static bool drvregistered;
@@ -196,25 +197,6 @@ struct ipmi_user {
 	atomic_t nr_msgs;
 };
 
-static void free_ipmi_user(struct kref *ref)
-{
-	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
-
-	vfree(user);
-}
-
-static void release_ipmi_user(struct ipmi_user *user)
-{
-	kref_put(&user->refcount, free_ipmi_user);
-}
-
-static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user)
-{
-	if (!kref_get_unless_zero(&user->refcount))
-		return NULL;
-	return user;
-}
-
 struct cmd_rcvr {
 	struct list_head link;
 
@@ -611,6 +593,28 @@ static int __ipmi_bmc_register(struct ipmi_smi *intf,
 			       bool guid_set, guid_t *guid, int intf_num);
 static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id);
 
+static void free_ipmi_user(struct kref *ref)
+{
+	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
+	struct module *owner;
+
+	owner = user->intf->owner;
+	kref_put(&user->intf->refcount, intf_free);
+	module_put(owner);
+	vfree(user);
+}
+
+static void release_ipmi_user(struct ipmi_user *user)
+{
+	kref_put(&user->refcount, free_ipmi_user);
+}
+
+static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user)
+{
+	if (!kref_get_unless_zero(&user->refcount))
+		return NULL;
+	return user;
+}
 
 /*
  * The driver model view of the IPMI messaging driver.
@@ -1330,7 +1334,6 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	unsigned long    flags;
 	struct cmd_rcvr  *rcvr;
 	struct cmd_rcvr  *rcvrs = NULL;
-	struct module    *owner;
 
 	if (!refcount_dec_if_one(&user->destroyed))
 		return;
@@ -1382,10 +1385,6 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	}
 
 	release_ipmi_user(user);
-
-	owner = intf->owner;
-	kref_put(&intf->refcount, intf_free);
-	module_put(owner);
 }
 
 void ipmi_destroy_user(struct ipmi_user *user)
@@ -2315,7 +2314,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 		}
 	}
 
-	mutex_lock(&ipmi_interfaces_mutex);
+	mutex_lock(&intf->users_mutex);
 	if (intf->in_shutdown) {
 		rv = -ENODEV;
 		goto out_err;
@@ -2361,7 +2360,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 
 		smi_send(intf, intf->handlers, smi_msg, priority);
 	}
-	mutex_unlock(&ipmi_interfaces_mutex);
+	mutex_unlock(&intf->users_mutex);
 
 out:
 	if (rv && user)
-- 
2.43.0

[PATCH 13/23] ipmi:msghandler: Don't acquire a user refcount for queued messages

[Corey Minyard]

Messages already have a refcount for the user, so there's no need to
account for a new one.

As part of this, grab a refcount to the interface when processing
received messages.  The messages can be freed there, cause the user
then the interface to be freed.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 32 +++++++++++++++--------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index d05032dc1f69..edc53c22ca84 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -958,20 +958,14 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 		ipmi_free_recv_msg(msg);
 		atomic_dec(&msg->user->nr_msgs);
 	} else {
-		struct ipmi_user *user = acquire_ipmi_user(msg->user);
-
-		if (user) {
-			/* Deliver it in smi_work. */
-			mutex_lock(&intf->user_msgs_mutex);
-			list_add_tail(&msg->link, &intf->user_msgs);
-			mutex_unlock(&intf->user_msgs_mutex);
-			queue_work(system_wq, &intf->smi_work);
-			/* User release will happen in the work queue. */
-		} else {
-			/* User went away, give up. */
-			ipmi_free_recv_msg(msg);
-			rv = -EINVAL;
-		}
+		/*
+		 * Deliver it in smi_work.  The message will hold a
+		 * refcount to the user.
+		 */
+		mutex_lock(&intf->user_msgs_mutex);
+		list_add_tail(&msg->link, &intf->user_msgs);
+		mutex_unlock(&intf->user_msgs_mutex);
+		queue_work(system_wq, &intf->smi_work);
 	}
 
 	return rv;
@@ -4827,6 +4821,13 @@ static void smi_work(struct work_struct *t)
 		mutex_unlock(&intf->users_mutex);
 	}
 
+	/*
+	 * Freeing the message can cause a user to be released, which
+	 * can then cause the interface to be freed.  Make sure that
+	 * doesn't happen until we are ready.
+	 */
+	kref_get(&intf->refcount);
+
 	mutex_lock(&intf->user_msgs_mutex);
 	list_for_each_entry_safe(msg, msg2, &intf->user_msgs, link) {
 		struct ipmi_user *user = msg->user;
@@ -4834,9 +4835,10 @@ static void smi_work(struct work_struct *t)
 		list_del(&msg->link);
 		atomic_dec(&user->nr_msgs);
 		user->handler->ipmi_recv_hndl(msg, user->handler_data);
-		release_ipmi_user(user);
 	}
 	mutex_unlock(&intf->user_msgs_mutex);
+
+	kref_put(&intf->refcount, intf_free);
 }
 
 /* Handle a new message from the lower layer. */
-- 
2.43.0

[PATCH 14/23] ipmi:msghandler: Don't check for shutdown when returning responses

[Corey Minyard]

The lower level interface shouldn't attempt to unregister if it has a
callback in the pending queue.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 34 ++++++++++-------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index edc53c22ca84..74a84eeeed9c 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3893,17 +3893,12 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf,
 		dev_dbg(intf->si_dev, "Invalid command: %*ph\n",
 			msg->data_size, msg->data);
 
-		mutex_lock(&ipmi_interfaces_mutex);
-		if (!intf->in_shutdown) {
-			smi_send(intf, intf->handlers, msg, 0);
-			/*
-			 * We used the message, so return the value
-			 * that causes it to not be freed or
-			 * queued.
-			 */
-			rv = -1;
-		}
-		mutex_unlock(&ipmi_interfaces_mutex);
+		smi_send(intf, intf->handlers, msg, 0);
+		/*
+		 * We used the message, so return the value that
+		 * causes it to not be freed or queued.
+		 */
+		rv = -1;
 	} else {
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
@@ -3984,17 +3979,12 @@ static int handle_ipmb_direct_rcv_cmd(struct ipmi_smi *intf,
 		msg->data[4] = IPMI_INVALID_CMD_COMPLETION_CODE;
 		msg->data_size = 5;
 
-		mutex_lock(&ipmi_interfaces_mutex);
-		if (!intf->in_shutdown) {
-			smi_send(intf, intf->handlers, msg, 0);
-			/*
-			 * We used the message, so return the value
-			 * that causes it to not be freed or
-			 * queued.
-			 */
-			rv = -1;
-		}
-		mutex_unlock(&ipmi_interfaces_mutex);
+		smi_send(intf, intf->handlers, msg, 0);
+		/*
+		 * We used the message, so return the value that
+		 * causes it to not be freed or queued.
+		 */
+		rv = -1;
 	} else {
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
-- 
2.43.0

[PATCH 15/23] ipmi:msghandler: Remove proc_fs.h

[Corey Minyard]

It's no longer used.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 74a84eeeed9c..927556ca469d 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -27,7 +27,6 @@
 #include <linux/ipmi_smi.h>
 #include <linux/notifier.h>
 #include <linux/init.h>
-#include <linux/proc_fs.h>
 #include <linux/rcupdate.h>
 #include <linux/interrupt.h>
 #include <linux/moduleparam.h>
-- 
2.43.0

[PATCH 16/23] ipmi:msghandler: Shut down lower layer first at unregister

[Corey Minyard]

This makes sure any outstanding messages are returned to the user before
the interface is cleaned up.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 927556ca469d..56654a9c5cf7 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3738,7 +3738,13 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
 	list_del(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
 
-	/* At this point no users can be added to the interface. */
+	/*
+	 * At this point no users can be added to the interface and no
+	 * new messages can be sent.
+	 */
+
+	if (intf->handlers->shutdown)
+		intf->handlers->shutdown(intf->send_info);
 
 	device_remove_file(intf->si_dev, &intf->nr_msgs_devattr);
 	device_remove_file(intf->si_dev, &intf->nr_users_devattr);
@@ -3761,9 +3767,6 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
 	}
 	mutex_unlock(&intf->users_mutex);
 
-	if (intf->handlers->shutdown)
-		intf->handlers->shutdown(intf->send_info);
-
 	cleanup_smi_msgs(intf);
 
 	ipmi_bmc_unregister(intf);
-- 
2.43.0

[PATCH 17/23] ipmi:msghandler: Add a error return from unhandle LAN cmds

[Corey Minyard]

If we get a command from a LAN channel, return an error instead of just
throwing it away.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 56654a9c5cf7..c72ff523216c 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -4165,14 +4165,33 @@ static int handle_lan_get_msg_cmd(struct ipmi_smi *intf,
 	rcu_read_unlock();
 
 	if (user == NULL) {
-		/* We didn't find a user, just give up. */
+		/* We didn't find a user, just give up and return an error. */
 		ipmi_inc_stat(intf, unhandled_commands);
 
+		msg->data[0] = (IPMI_NETFN_APP_REQUEST << 2);
+		msg->data[1] = IPMI_SEND_MSG_CMD;
+		msg->data[2] = chan;
+		msg->data[3] = msg->rsp[4]; /* handle */
+		msg->data[4] = msg->rsp[8]; /* rsSWID */
+		msg->data[5] = ((netfn + 1) << 2) | (msg->rsp[9] & 0x3);
+		msg->data[6] = ipmb_checksum(&msg->data[3], 3);
+		msg->data[7] = msg->rsp[5]; /* rqSWID */
+		/* rqseq/lun */
+		msg->data[8] = (msg->rsp[9] & 0xfc) | (msg->rsp[6] & 0x3);
+		msg->data[9] = cmd;
+		msg->data[10] = IPMI_INVALID_CMD_COMPLETION_CODE;
+		msg->data[11] = ipmb_checksum(&msg->data[7], 4);
+		msg->data_size = 12;
+
+		dev_dbg(intf->si_dev, "Invalid command: %*ph\n",
+			msg->data_size, msg->data);
+
+		smi_send(intf, intf->handlers, msg, 0);
 		/*
-		 * Don't do anything with these messages, just allow
-		 * them to be freed.
+		 * We used the message, so return the value that
+		 * causes it to not be freed or queued.
 		 */
-		rv = 0;
+		rv = -1;
 	} else {
 		recv_msg = ipmi_alloc_recv_msg();
 		if (!recv_msg) {
-- 
2.43.0

[PATCH 18/23] ipmi:si: Rework startup of IPMI devices

[Corey Minyard]

It is possible in some situations that IPMI devices won't get started up
properly.  This change makes it so all non-duplicate devices will get
started up.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_si_intf.c | 82 ++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 20 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 062f92cace24..8837f85f2d77 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2095,9 +2095,18 @@ static int try_smi_init(struct smi_info *new_smi)
 	return rv;
 }
 
+/*
+ * Devices in the same address space at the same address are the same.
+ */
+static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
+{
+	return (e1->io.addr_space == e2->io.addr_space &&
+		e1->io.addr_data == e2->io.addr_data);
+}
+
 static int __init init_ipmi_si(void)
 {
-	struct smi_info *e;
+	struct smi_info *e, *e2;
 	enum ipmi_addr_src type = SI_INVALID;
 
 	if (initialized)
@@ -2113,37 +2122,70 @@ static int __init init_ipmi_si(void)
 
 	ipmi_si_parisc_init();
 
-	/* We prefer devices with interrupts, but in the case of a machine
-	   with multiple BMCs we assume that there will be several instances
-	   of a given type so if we succeed in registering a type then also
-	   try to register everything else of the same type */
 	mutex_lock(&smi_infos_lock);
+
+	/*
+	 * Scan through all the devices.  We prefer devices with
+	 * interrupts, so go through those first in case there are any
+	 * duplicates that don't have the interrupt set.
+	 */
 	list_for_each_entry(e, &smi_infos, link) {
-		/* Try to register a device if it has an IRQ and we either
-		   haven't successfully registered a device yet or this
-		   device has the same type as one we successfully registered */
-		if (e->io.irq && (!type || e->io.addr_source == type)) {
-			if (!try_smi_init(e)) {
-				type = e->io.addr_source;
+		bool dup = false;
+
+		/* Register ones with interrupts first. */
+		if (!e->io.irq)
+			continue;
+
+		/*
+		 * Go through the ones we have already seen to see if this
+		 * is a dup.
+		 */
+		list_for_each_entry(e2, &smi_infos, link) {
+			if (e2 == e)
+				break;
+			if (e2->io.irq && ipmi_smi_info_same(e, e2)) {
+				dup = true;
+				break;
 			}
 		}
+		if (!dup)
+			try_smi_init(e);
 	}
 
-	/* type will only have been set if we successfully registered an si */
-	if (type)
-		goto skip_fallback_noirq;
+	/*
+	 * Now try devices without interrupts.
+	 */
+	list_for_each_entry(e, &smi_infos, link) {
+		bool dup = false;
 
-	/* Fall back to the preferred device */
+		if (e->io.irq)
+			continue;
 
-	list_for_each_entry(e, &smi_infos, link) {
-		if (!e->io.irq && (!type || e->io.addr_source == type)) {
-			if (!try_smi_init(e)) {
-				type = e->io.addr_source;
+		/*
+		 * Go through the ones we have already seen to see if
+		 * this is a dup.  We have already looked at the ones
+		 * with interrupts.
+		 */
+		list_for_each_entry(e2, &smi_infos, link) {
+			if (!e2->io.irq)
+				continue;
+			if (ipmi_smi_info_same(e, e2)) {
+				dup = true;
+				break;
+			}
+		}
+		list_for_each_entry(e2, &smi_infos, link) {
+			if (e2 == e)
+				break;
+			if (ipmi_smi_info_same(e, e2)) {
+				dup = true;
+				break;
 			}
 		}
+		if (!dup)
+			try_smi_init(e);
 	}
 
-skip_fallback_noirq:
 	initialized = true;
 	mutex_unlock(&smi_infos_lock);
 
-- 
2.43.0

[PATCH 19/23] ipmi:msghandler: Don't deliver messages to deleted users

[Corey Minyard]

Check to see if they have been destroyed before trying to deliver a
message.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index c72ff523216c..938c566624d9 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -1327,6 +1327,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	unsigned long    flags;
 	struct cmd_rcvr  *rcvr;
 	struct cmd_rcvr  *rcvrs = NULL;
+	struct ipmi_recv_msg *msg, *msg2;
 
 	if (!refcount_dec_if_one(&user->destroyed))
 		return;
@@ -1377,6 +1378,15 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 		kfree(rcvr);
 	}
 
+	mutex_lock(&intf->user_msgs_mutex);
+	list_for_each_entry_safe(msg, msg2, &intf->user_msgs, link) {
+		if (msg->user != user)
+			continue;
+		list_del(&msg->link);
+		ipmi_free_recv_msg(msg);
+	}
+	mutex_unlock(&intf->user_msgs_mutex);
+
 	release_ipmi_user(user);
 }
 
@@ -4844,8 +4854,22 @@ static void smi_work(struct work_struct *t)
 		struct ipmi_user *user = msg->user;
 
 		list_del(&msg->link);
-		atomic_dec(&user->nr_msgs);
-		user->handler->ipmi_recv_hndl(msg, user->handler_data);
+
+		/*
+		 * I would like for this check (and user->destroyed)
+		 * to go away, but it's possible that an interface is
+		 * processing a message that belongs to the user while
+		 * the user is being deleted.  When that response
+		 * comes back, it could be queued after the user is
+		 * destroyed.  This is simpler than handling it in the
+		 * interface.
+		 */
+		if (refcount_read(&user->destroyed) == 0) {
+			ipmi_free_recv_msg(msg);
+		} else {
+			atomic_dec(&user->nr_msgs);
+			user->handler->ipmi_recv_hndl(msg, user->handler_data);
+		}
 	}
 	mutex_unlock(&intf->user_msgs_mutex);
 
-- 
2.43.0

[PATCH 20/23] ipmi:ssif: Fix a shutdown race

[Corey Minyard]

It was possible for the SSIF thread to stop and quit before the
kthread_stop() call because ssif->stopping was set before the
stop.  So only exit the SSIF thread is kthread_should_stop()
returns true.

There is no need to wake the thread, as the wait will be interrupted
by kthread_stop().

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_ssif.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 506d9988721e..b016fabaac68 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -481,8 +481,6 @@ static int ipmi_ssif_thread(void *data)
 		/* Wait for something to do */
 		result = wait_for_completion_interruptible(
 						&ssif_info->wake_thread);
-		if (ssif_info->stopping)
-			break;
 		if (result == -ERESTARTSYS)
 			continue;
 		init_completion(&ssif_info->wake_thread);
@@ -1270,10 +1268,8 @@ static void shutdown_ssif(void *send_info)
 	ssif_info->stopping = true;
 	del_timer_sync(&ssif_info->watch_timer);
 	del_timer_sync(&ssif_info->retry_timer);
-	if (ssif_info->thread) {
-		complete(&ssif_info->wake_thread);
+	if (ssif_info->thread)
 		kthread_stop(ssif_info->thread);
-	}
 }
 
 static void ssif_remove(struct i2c_client *client)
-- 
2.43.0

[PATCH 21/23] Documentation:ipmi: Remove comments about interrupt level

[Corey Minyard]

Callbacks no longer run at interrupt level or bh, so remove those
comments.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 Documentation/driver-api/ipmi.rst | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/ipmi.rst b/Documentation/driver-api/ipmi.rst
index dfa021eacd63..3a533cd2ef60 100644
--- a/Documentation/driver-api/ipmi.rst
+++ b/Documentation/driver-api/ipmi.rst
@@ -280,10 +280,8 @@ Creating the User
 To use the message handler, you must first create a user using
 ipmi_create_user.  The interface number specifies which SMI you want
 to connect to, and you must supply callback functions to be called
-when data comes in.  The callback function can run at interrupt level,
-so be careful using the callbacks.  This also allows to you pass in a
-piece of data, the handler_data, that will be passed back to you on
-all calls.
+when data comes in.  This also allows to you pass in a piece of data,
+the handler_data, that will be passed back to you on all calls.
 
 Once you are done, call ipmi_destroy_user() to get rid of the user.
 
@@ -303,8 +301,7 @@ use it for anything you like.
 
 Responses come back in the function pointed to by the ipmi_recv_hndl
 field of the "handler" that you passed in to ipmi_create_user().
-Remember again, these may be running at interrupt level.  Remember to
-look at the receive type, too.
+Remember to look at the receive type, too.
 
 From userland, you fill out an ipmi_req_t structure and use the
 IPMICTL_SEND_COMMAND ioctl.  For incoming stuff, you can use select()
-- 
2.43.0

[PATCH 22/23] ipmi:msghandler: Export and fix panic messaging capability

[Corey Minyard]

Don't have the other users that do things at panic time (the watchdog)
do all this themselves, provide a function to do it.

Also, with the new design where most stuff happens at thread context,
a few things needed to be fixed to avoid doing locking in a panic
context.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 50 ++++++++++++++++++-----------
 include/linux/ipmi.h                | 10 ++++++
 2 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 938c566624d9..ece6aa95fbb5 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -2284,6 +2284,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 {
 	struct ipmi_smi_msg *smi_msg;
 	struct ipmi_recv_msg *recv_msg;
+	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	int rv = 0;
 
 	if (user) {
@@ -2317,7 +2318,8 @@ static int i_ipmi_request(struct ipmi_user     *user,
 		}
 	}
 
-	mutex_lock(&intf->users_mutex);
+	if (!run_to_completion)
+		mutex_lock(&intf->users_mutex);
 	if (intf->in_shutdown) {
 		rv = -ENODEV;
 		goto out_err;
@@ -2363,7 +2365,8 @@ static int i_ipmi_request(struct ipmi_user     *user,
 
 		smi_send(intf, intf->handlers, smi_msg, priority);
 	}
-	mutex_unlock(&intf->users_mutex);
+	if (!run_to_completion)
+		mutex_unlock(&intf->users_mutex);
 
 out:
 	if (rv && user)
@@ -4559,7 +4562,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 	    && (msg->data[1] == IPMI_SEND_MSG_CMD)
 	    && (msg->user_data == NULL)) {
 
-		if (intf->in_shutdown)
+		if (intf->in_shutdown || intf->run_to_completion)
 			goto out;
 
 		/*
@@ -4631,6 +4634,9 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 		 */
 		struct ipmi_recv_msg *recv_msg;
 
+		if (intf->run_to_completion)
+			goto out;
+
 		chan = msg->data[2] & 0x0f;
 		if (chan >= IPMI_MAX_CHANNELS)
 			/* Invalid channel number */
@@ -4653,6 +4659,9 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 		   && (msg->rsp[1] == IPMI_GET_MSG_CMD)) {
 		struct ipmi_channel   *chans;
 
+		if (intf->run_to_completion)
+			goto out;
+
 		/* It's from the receive queue. */
 		chan = msg->rsp[3] & 0xf;
 		if (chan >= IPMI_MAX_CHANNELS) {
@@ -4727,6 +4736,9 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 	} else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
 		   && (msg->rsp[1] == IPMI_READ_EVENT_MSG_BUFFER_CMD)) {
 		/* It's an asynchronous event. */
+		if (intf->run_to_completion)
+			goto out;
+
 		requeue = handle_read_event_rsp(intf, msg);
 	} else {
 		/* It's a response from the local BMC. */
@@ -4855,15 +4867,6 @@ static void smi_work(struct work_struct *t)
 
 		list_del(&msg->link);
 
-		/*
-		 * I would like for this check (and user->destroyed)
-		 * to go away, but it's possible that an interface is
-		 * processing a message that belongs to the user while
-		 * the user is being deleted.  When that response
-		 * comes back, it could be queued after the user is
-		 * destroyed.  This is simpler than handling it in the
-		 * interface.
-		 */
 		if (refcount_read(&user->destroyed) == 0) {
 			ipmi_free_recv_msg(msg);
 		} else {
@@ -5222,9 +5225,9 @@ static void dummy_recv_done_handler(struct ipmi_recv_msg *msg)
 /*
  * Inside a panic, send a message and wait for a response.
  */
-static void ipmi_panic_request_and_wait(struct ipmi_smi *intf,
-					struct ipmi_addr *addr,
-					struct kernel_ipmi_msg *msg)
+static void _ipmi_panic_request_and_wait(struct ipmi_smi *intf,
+					 struct ipmi_addr *addr,
+					 struct kernel_ipmi_msg *msg)
 {
 	struct ipmi_smi_msg  smi_msg;
 	struct ipmi_recv_msg recv_msg;
@@ -5254,6 +5257,15 @@ static void ipmi_panic_request_and_wait(struct ipmi_smi *intf,
 		ipmi_poll(intf);
 }
 
+void ipmi_panic_request_and_wait(struct ipmi_user *user,
+				 struct ipmi_addr *addr,
+				 struct kernel_ipmi_msg *msg)
+{
+	user->intf->run_to_completion = 1;
+	_ipmi_panic_request_and_wait(user->intf, addr, msg);
+}
+EXPORT_SYMBOL(ipmi_panic_request_and_wait);
+
 static void event_receiver_fetcher(struct ipmi_smi *intf,
 				   struct ipmi_recv_msg *msg)
 {
@@ -5322,7 +5334,7 @@ static void send_panic_events(struct ipmi_smi *intf, char *str)
 	}
 
 	/* Send the event announcing the panic. */
-	ipmi_panic_request_and_wait(intf, &addr, &msg);
+	_ipmi_panic_request_and_wait(intf, &addr, &msg);
 
 	/*
 	 * On every interface, dump a bunch of OEM event holding the
@@ -5358,7 +5370,7 @@ static void send_panic_events(struct ipmi_smi *intf, char *str)
 	msg.data = NULL;
 	msg.data_len = 0;
 	intf->null_user_handler = device_id_fetcher;
-	ipmi_panic_request_and_wait(intf, &addr, &msg);
+	_ipmi_panic_request_and_wait(intf, &addr, &msg);
 
 	if (intf->local_event_generator) {
 		/* Request the event receiver from the local MC. */
@@ -5367,7 +5379,7 @@ static void send_panic_events(struct ipmi_smi *intf, char *str)
 		msg.data = NULL;
 		msg.data_len = 0;
 		intf->null_user_handler = event_receiver_fetcher;
-		ipmi_panic_request_and_wait(intf, &addr, &msg);
+		_ipmi_panic_request_and_wait(intf, &addr, &msg);
 	}
 	intf->null_user_handler = NULL;
 
@@ -5419,7 +5431,7 @@ static void send_panic_events(struct ipmi_smi *intf, char *str)
 		memcpy_and_pad(data+5, 11, p, size, '\0');
 		p += size;
 
-		ipmi_panic_request_and_wait(intf, &addr, &msg);
+		_ipmi_panic_request_and_wait(intf, &addr, &msg);
 	}
 }
 
diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h
index 27cd5980bb27..7da6602eab71 100644
--- a/include/linux/ipmi.h
+++ b/include/linux/ipmi.h
@@ -344,4 +344,14 @@ extern int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data);
 /* Helper function for computing the IPMB checksum of some data. */
 unsigned char ipmb_checksum(unsigned char *data, int size);
 
+/*
+ * For things that must send messages at panic time, like the IPMI watchdog
+ * driver that extends the reset time on a panic, use this to send messages
+ * from panic context.  Note that this puts the driver into a mode that
+ * only works at panic time, so only use it then.
+ */
+void ipmi_panic_request_and_wait(struct ipmi_user *user,
+				 struct ipmi_addr *addr,
+				 struct kernel_ipmi_msg *msg);
+
 #endif /* __LINUX_IPMI_H */
-- 
2.43.0

[PATCH 23/23] ipmi:watchdog: Use the new interface for panic messages

[Corey Minyard]

It's available, remove all the duplicate code.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 72 ++++++++-----------------------
 1 file changed, 17 insertions(+), 55 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 01c10bd5f099..ab759b492fdd 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -363,7 +363,7 @@ static int __ipmi_set_timeout(struct ipmi_smi_msg  *smi_msg,
 {
 	struct kernel_ipmi_msg            msg;
 	unsigned char                     data[6];
-	int                               rv;
+	int                               rv = 0;
 	struct ipmi_system_interface_addr addr;
 	int                               hbnow = 0;
 
@@ -405,14 +405,18 @@ static int __ipmi_set_timeout(struct ipmi_smi_msg  *smi_msg,
 	msg.cmd = IPMI_WDOG_SET_TIMER;
 	msg.data = data;
 	msg.data_len = sizeof(data);
-	rv = ipmi_request_supply_msgs(watchdog_user,
-				      (struct ipmi_addr *) &addr,
-				      0,
-				      &msg,
-				      NULL,
-				      smi_msg,
-				      recv_msg,
-				      1);
+	if (smi_msg)
+		rv = ipmi_request_supply_msgs(watchdog_user,
+					      (struct ipmi_addr *) &addr,
+					      0,
+					      &msg,
+					      NULL,
+					      smi_msg,
+					      recv_msg,
+					      1);
+	else
+		ipmi_panic_request_and_wait(watchdog_user,
+					    (struct ipmi_addr *) &addr, &msg);
 	if (rv)
 		pr_warn("set timeout error: %d\n", rv);
 	else if (send_heartbeat_now)
@@ -431,9 +435,7 @@ static int _ipmi_set_timeout(int do_heartbeat)
 
 	atomic_set(&msg_tofree, 2);
 
-	rv = __ipmi_set_timeout(&smi_msg,
-				&recv_msg,
-				&send_heartbeat_now);
+	rv = __ipmi_set_timeout(&smi_msg, &recv_msg, &send_heartbeat_now);
 	if (rv) {
 		atomic_set(&msg_tofree, 0);
 		return rv;
@@ -460,27 +462,10 @@ static int ipmi_set_timeout(int do_heartbeat)
 	return rv;
 }
 
-static atomic_t panic_done_count = ATOMIC_INIT(0);
-
-static void panic_smi_free(struct ipmi_smi_msg *msg)
-{
-	atomic_dec(&panic_done_count);
-}
-static void panic_recv_free(struct ipmi_recv_msg *msg)
-{
-	atomic_dec(&panic_done_count);
-}
-
-static struct ipmi_smi_msg panic_halt_heartbeat_smi_msg =
-	INIT_IPMI_SMI_MSG(panic_smi_free);
-static struct ipmi_recv_msg panic_halt_heartbeat_recv_msg =
-	INIT_IPMI_RECV_MSG(panic_recv_free);
-
 static void panic_halt_ipmi_heartbeat(void)
 {
 	struct kernel_ipmi_msg             msg;
 	struct ipmi_system_interface_addr addr;
-	int rv;
 
 	/*
 	 * Don't reset the timer if we have the timer turned off, that
@@ -497,24 +482,10 @@ static void panic_halt_ipmi_heartbeat(void)
 	msg.cmd = IPMI_WDOG_RESET_TIMER;
 	msg.data = NULL;
 	msg.data_len = 0;
-	atomic_add(2, &panic_done_count);
-	rv = ipmi_request_supply_msgs(watchdog_user,
-				      (struct ipmi_addr *) &addr,
-				      0,
-				      &msg,
-				      NULL,
-				      &panic_halt_heartbeat_smi_msg,
-				      &panic_halt_heartbeat_recv_msg,
-				      1);
-	if (rv)
-		atomic_sub(2, &panic_done_count);
+	ipmi_panic_request_and_wait(watchdog_user, (struct ipmi_addr *) &addr,
+				    &msg);
 }
 
-static struct ipmi_smi_msg panic_halt_smi_msg =
-	INIT_IPMI_SMI_MSG(panic_smi_free);
-static struct ipmi_recv_msg panic_halt_recv_msg =
-	INIT_IPMI_RECV_MSG(panic_recv_free);
-
 /*
  * Special call, doesn't claim any locks.  This is only to be called
  * at panic or halt time, in run-to-completion mode, when the caller
@@ -526,22 +497,13 @@ static void panic_halt_ipmi_set_timeout(void)
 	int send_heartbeat_now;
 	int rv;
 
-	/* Wait for the messages to be free. */
-	while (atomic_read(&panic_done_count) != 0)
-		ipmi_poll_interface(watchdog_user);
-	atomic_add(2, &panic_done_count);
-	rv = __ipmi_set_timeout(&panic_halt_smi_msg,
-				&panic_halt_recv_msg,
-				&send_heartbeat_now);
+	rv = __ipmi_set_timeout(NULL, NULL, &send_heartbeat_now);
 	if (rv) {
-		atomic_sub(2, &panic_done_count);
 		pr_warn("Unable to extend the watchdog timeout\n");
 	} else {
 		if (send_heartbeat_now)
 			panic_halt_ipmi_heartbeat();
 	}
-	while (atomic_read(&panic_done_count) != 0)
-		ipmi_poll_interface(watchdog_user);
 }
 
 static int __ipmi_heartbeat(void)
-- 
2.43.0