This is a note to let you know that I've just added the patch titled
ipmi: Make the smi watcher be disabled immediately when not needed
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ipmi-make-the-smi-watcher-be-disabled-immediately-when-not-needed.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From e1891cffd4c4896a899337a243273f0e23c028df Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard@xxxxxxxxxx>
Date: Wed, 24 Oct 2018 15:17:04 -0500
Subject: ipmi: Make the smi watcher be disabled immediately when not needed
From: Corey Minyard <cminyard@xxxxxxxxxx>
commit e1891cffd4c4896a899337a243273f0e23c028df upstream.
The code to tell the lower layer to enable or disable watching for
certain things was lazy in disabling, it waited until a timer tick
to see if a disable was necessary. Not a really big deal, but it
could be improved.
Modify the code to enable and disable watching immediately and don't
do it from the background timer any more.
Signed-off-by: Corey Minyard <cminyard@xxxxxxxxxx>
Tested-by: Kamlakant Patel <kamlakant.patel@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/char/ipmi/ipmi_msghandler.c | 164 +++++++++++++++++++-----------------
drivers/char/ipmi/ipmi_si_intf.c | 2
drivers/char/ipmi/ipmi_ssif.c | 2
include/linux/ipmi_smi.h | 17 ---
4 files changed, 96 insertions(+), 89 deletions(-)
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -541,15 +541,20 @@ struct ipmi_smi {
atomic_t event_waiters;
unsigned int ticks_to_req_ev;
+ spinlock_t watch_lock; /* For dealing with watch stuff below. */
+
/* How many users are waiting for commands? */
- atomic_t command_waiters;
+ unsigned int command_waiters;
/* How many users are waiting for watchdogs? */
- atomic_t watchdog_waiters;
+ unsigned int watchdog_waiters;
+
+ /* How many users are waiting for message responses? */
+ unsigned int response_waiters;
/*
* Tells what the lower layer has last been asked to watch for,
- * messages and/or watchdogs. Protected by xmit_msgs_lock.
+ * messages and/or watchdogs. Protected by watch_lock.
*/
unsigned int last_watch_mask;
@@ -945,6 +950,64 @@ static void deliver_err_response(struct
deliver_local_response(intf, msg);
}
+static void smi_add_watch(struct ipmi_smi *intf, unsigned int flags)
+{
+ unsigned long iflags;
+
+ if (!intf->handlers->set_need_watch)
+ return;
+
+ spin_lock_irqsave(&intf->watch_lock, iflags);
+ if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES)
+ intf->response_waiters++;
+
+ if (flags & IPMI_WATCH_MASK_CHECK_WATCHDOG)
+ intf->watchdog_waiters++;
+
+ if (flags & IPMI_WATCH_MASK_CHECK_COMMANDS)
+ intf->command_waiters++;
+
+ if ((intf->last_watch_mask & flags) != flags) {
+ intf->last_watch_mask |= flags;
+ intf->handlers->set_need_watch(intf->send_info,
+ intf->last_watch_mask);
+ }
+ spin_unlock_irqrestore(&intf->watch_lock, iflags);
+}
+
+static void smi_remove_watch(struct ipmi_smi *intf, unsigned int flags)
+{
+ unsigned long iflags;
+
+ if (!intf->handlers->set_need_watch)
+ return;
+
+ spin_lock_irqsave(&intf->watch_lock, iflags);
+ if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES)
+ intf->response_waiters--;
+
+ if (flags & IPMI_WATCH_MASK_CHECK_WATCHDOG)
+ intf->watchdog_waiters--;
+
+ if (flags & IPMI_WATCH_MASK_CHECK_COMMANDS)
+ intf->command_waiters--;
+
+ flags = 0;
+ if (intf->response_waiters)
+ flags |= IPMI_WATCH_MASK_CHECK_MESSAGES;
+ if (intf->watchdog_waiters)
+ flags |= IPMI_WATCH_MASK_CHECK_WATCHDOG;
+ if (intf->command_waiters)
+ flags |= IPMI_WATCH_MASK_CHECK_COMMANDS;
+
+ if (intf->last_watch_mask != flags) {
+ intf->last_watch_mask = flags;
+ intf->handlers->set_need_watch(intf->send_info,
+ intf->last_watch_mask);
+ }
+ spin_unlock_irqrestore(&intf->watch_lock, iflags);
+}
+
/*
* Find the next sequence number not being used and add the given
* message with the given timeout to the sequence table. This must be
@@ -988,6 +1051,7 @@ static int intf_next_seq(struct ipmi_smi
*seq = i;
*seqid = intf->seq_table[i].seqid;
intf->curr_seq = (i+1)%IPMI_IPMB_NUM_SEQ;
+ smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES);
need_waiter(intf);
} else {
rv = -EAGAIN;
@@ -1026,6 +1090,7 @@ static int intf_find_seq(struct ipmi_smi
&& (ipmi_addr_equal(addr, &msg->addr))) {
*recv_msg = msg;
intf->seq_table[seq].inuse = 0;
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES);
rv = 0;
}
}
@@ -1087,6 +1152,7 @@ static int intf_err_seq(struct ipmi_smi
struct seq_table *ent = &intf->seq_table[seq];
ent->inuse = 0;
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES);
msg = ent->recv_msg;
rv = 0;
}
@@ -1098,30 +1164,6 @@ static int intf_err_seq(struct ipmi_smi
return rv;
}
-/* Must be called with xmit_msgs_lock held. */
-static void smi_tell_to_watch(struct ipmi_smi *intf,
- unsigned int flags,
- struct ipmi_smi_msg *smi_msg)
-{
- if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES) {
- if (!smi_msg)
- return;
-
- if (!smi_msg->needs_response)
- return;
- }
-
- if (!intf->handlers->set_need_watch)
- return;
-
- if ((intf->last_watch_mask & flags) == flags)
- return;
-
- intf->last_watch_mask |= flags;
- intf->handlers->set_need_watch(intf->send_info,
- intf->last_watch_mask);
-}
-
static void free_user_work(struct work_struct *work)
{
struct ipmi_user *user = container_of(work, struct ipmi_user,
@@ -1198,12 +1240,9 @@ int ipmi_create_user(unsigned int
spin_lock_irqsave(&intf->seq_lock, flags);
list_add_rcu(&new_user->link, &intf->users);
spin_unlock_irqrestore(&intf->seq_lock, flags);
- if (handler->ipmi_watchdog_pretimeout) {
+ if (handler->ipmi_watchdog_pretimeout)
/* User wants pretimeouts, so make sure to watch for them. */
- if (atomic_inc_return(&intf->watchdog_waiters) == 1)
- smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG,
- NULL);
- }
+ smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG);
srcu_read_unlock(&ipmi_interfaces_srcu, index);
*user = new_user;
return 0;
@@ -1276,7 +1315,7 @@ static void _ipmi_destroy_user(struct ip
user->handler->shutdown(user->handler_data);
if (user->handler->ipmi_watchdog_pretimeout)
- atomic_dec(&intf->watchdog_waiters);
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG);
if (user->gets_events)
atomic_dec(&intf->event_waiters);
@@ -1289,6 +1328,7 @@ static void _ipmi_destroy_user(struct ip
if (intf->seq_table[i].inuse
&& (intf->seq_table[i].recv_msg->user == user)) {
intf->seq_table[i].inuse = 0;
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES);
ipmi_free_recv_msg(intf->seq_table[i].recv_msg);
}
}
@@ -1634,8 +1674,7 @@ int ipmi_register_for_cmd(struct ipmi_us
goto out_unlock;
}
- if (atomic_inc_return(&intf->command_waiters) == 1)
- smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS, NULL);
+ smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS);
list_add_rcu(&rcvr->link, &intf->cmd_rcvrs);
@@ -1685,7 +1724,7 @@ int ipmi_unregister_for_cmd(struct ipmi_
synchronize_rcu();
release_ipmi_user(user, index);
while (rcvrs) {
- atomic_dec(&intf->command_waiters);
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS);
rcvr = rcvrs;
rcvrs = rcvr->next;
kfree(rcvr);
@@ -1813,8 +1852,6 @@ static void smi_send(struct ipmi_smi *in
spin_lock_irqsave(&intf->xmit_msgs_lock, flags);
smi_msg = smi_add_send_msg(intf, smi_msg, priority);
- smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, smi_msg);
-
if (!run_to_completion)
spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
@@ -2014,9 +2051,6 @@ static int i_ipmi_req_ipmb(struct ipmi_s
ipmb_seq, broadcast,
source_address, source_lun);
- /* We will be getting a response in the BMC message queue. */
- smi_msg->needs_response = true;
-
/*
* Copy the message into the recv message data, so we
* can retransmit it later if necessary.
@@ -2204,7 +2238,6 @@ static int i_ipmi_request(struct ipmi_us
goto out;
}
}
- smi_msg->needs_response = false;
rcu_read_lock();
if (intf->in_shutdown) {
@@ -3425,9 +3458,8 @@ int ipmi_add_smi(struct module *
INIT_LIST_HEAD(&intf->xmit_msgs);
INIT_LIST_HEAD(&intf->hp_xmit_msgs);
spin_lock_init(&intf->events_lock);
+ spin_lock_init(&intf->watch_lock);
atomic_set(&intf->event_waiters, 0);
- atomic_set(&intf->watchdog_waiters, 0);
- atomic_set(&intf->command_waiters, 0);
intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME;
INIT_LIST_HEAD(&intf->waiting_events);
intf->waiting_events_count = 0;
@@ -4447,8 +4479,6 @@ static void smi_recv_tasklet(unsigned lo
}
}
- smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, newmsg);
-
if (!run_to_completion)
spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
if (newmsg)
@@ -4576,7 +4606,7 @@ static void check_msg_timeout(struct ipm
struct list_head *timeouts,
unsigned long timeout_period,
int slot, unsigned long *flags,
- unsigned int *watch_mask)
+ bool *need_timer)
{
struct ipmi_recv_msg *msg;
@@ -4588,13 +4618,14 @@ static void check_msg_timeout(struct ipm
if (timeout_period < ent->timeout) {
ent->timeout -= timeout_period;
- *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES;
+ *need_timer = true;
return;
}
if (ent->retries_left == 0) {
/* The message has used all its retries. */
ent->inuse = 0;
+ smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES);
msg = ent->recv_msg;
list_add_tail(&msg->link, timeouts);
if (ent->broadcast)
@@ -4607,7 +4638,7 @@ static void check_msg_timeout(struct ipm
struct ipmi_smi_msg *smi_msg;
/* More retries, send again. */
- *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES;
+ *need_timer = true;
/*
* Start with the max timer, set to normal timer after
@@ -4652,20 +4683,20 @@ static void check_msg_timeout(struct ipm
}
}
-static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf,
- unsigned long timeout_period)
+static bool ipmi_timeout_handler(struct ipmi_smi *intf,
+ unsigned long timeout_period)
{
struct list_head timeouts;
struct ipmi_recv_msg *msg, *msg2;
unsigned long flags;
int i;
- unsigned int watch_mask = 0;
+ bool need_timer = false;
if (!intf->bmc_registered) {
kref_get(&intf->refcount);
if (!schedule_work(&intf->bmc_reg_work)) {
kref_put(&intf->refcount, intf_free);
- watch_mask |= IPMI_WATCH_MASK_INTERNAL;
+ need_timer = true;
}
}
@@ -4685,7 +4716,7 @@ static unsigned int ipmi_timeout_handler
for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++)
check_msg_timeout(intf, &intf->seq_table[i],
&timeouts, timeout_period, i,
- &flags, &watch_mask);
+ &flags, &need_timer);
spin_unlock_irqrestore(&intf->seq_lock, flags);
list_for_each_entry_safe(msg, msg2, &timeouts, link)
@@ -4716,7 +4747,7 @@ static unsigned int ipmi_timeout_handler
tasklet_schedule(&intf->recv_tasklet);
- return watch_mask;
+ return need_timer;
}
static void ipmi_request_event(struct ipmi_smi *intf)
@@ -4736,9 +4767,8 @@ static atomic_t stop_operation;
static void ipmi_timeout(struct timer_list *unused)
{
struct ipmi_smi *intf;
- unsigned int watch_mask = 0;
+ bool need_timer = false;
int index;
- unsigned long flags;
if (atomic_read(&stop_operation))
return;
@@ -4751,28 +4781,14 @@ static void ipmi_timeout(struct timer_li
ipmi_request_event(intf);
intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME;
}
- watch_mask |= IPMI_WATCH_MASK_INTERNAL;
+ need_timer = true;
}
- if (atomic_read(&intf->watchdog_waiters))
- watch_mask |= IPMI_WATCH_MASK_CHECK_WATCHDOG;
-
- if (atomic_read(&intf->command_waiters))
- watch_mask |= IPMI_WATCH_MASK_CHECK_COMMANDS;
-
- watch_mask |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME);
-
- spin_lock_irqsave(&intf->xmit_msgs_lock, flags);
- if (watch_mask != intf->last_watch_mask &&
- intf->handlers->set_need_watch)
- intf->handlers->set_need_watch(intf->send_info,
- watch_mask);
- intf->last_watch_mask = watch_mask;
- spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
+ need_timer |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME);
}
srcu_read_unlock(&ipmi_interfaces_srcu, index);
- if (watch_mask)
+ if (need_timer)
mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES);
}
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1079,7 +1079,7 @@ static void set_need_watch(void *send_in
unsigned long flags;
int enable;
- enable = !!(watch_mask & ~IPMI_WATCH_MASK_INTERNAL);
+ enable = !!watch_mask;
atomic_set(&smi_info->need_watch, enable);
spin_lock_irqsave(&smi_info->si_lock, flags);
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1159,7 +1159,7 @@ static void ssif_set_need_watch(void *se
if (watch_mask & IPMI_WATCH_MASK_CHECK_MESSAGES)
timeout = SSIF_WATCH_MSG_TIMEOUT;
- else if (watch_mask & ~IPMI_WATCH_MASK_INTERNAL)
+ else if (watch_mask)
timeout = SSIF_WATCH_WATCHDOG_TIMEOUT;
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
--- a/include/linux/ipmi_smi.h
+++ b/include/linux/ipmi_smi.h
@@ -32,14 +32,11 @@ typedef struct ipmi_smi *ipmi_smi_t;
/*
* Flags for set_check_watch() below. Tells if the SMI should be
- * waiting for watchdog timeouts, commands and/or messages. There is
- * also an internal flag for the message handler, SMIs should ignore
- * it.
+ * waiting for watchdog timeouts, commands and/or messages.
*/
-#define IPMI_WATCH_MASK_INTERNAL (1 << 0)
-#define IPMI_WATCH_MASK_CHECK_MESSAGES (1 << 1)
-#define IPMI_WATCH_MASK_CHECK_WATCHDOG (1 << 2)
-#define IPMI_WATCH_MASK_CHECK_COMMANDS (1 << 3)
+#define IPMI_WATCH_MASK_CHECK_MESSAGES (1 << 0)
+#define IPMI_WATCH_MASK_CHECK_WATCHDOG (1 << 1)
+#define IPMI_WATCH_MASK_CHECK_COMMANDS (1 << 2)
/*
* Messages to/from the lower layer. The smi interface will take one
@@ -67,12 +64,6 @@ struct ipmi_smi_msg {
unsigned char rsp[IPMI_MAX_MSG_LENGTH];
/*
- * There should be a response message coming back in the BMC
- * message queue.
- */
- bool needs_response;
-
- /*
* Will be called when the system is done with the message
* (presumably to free it).
*/
Patches currently in stable-queue which might be from cminyard@xxxxxxxxxx are
queue-4.19/ipmi-move-message-error-checking-to-avoid-deadlock.patch
queue-4.19/ipmi-make-the-smi-watcher-be-disabled-immediately-when-not-needed.patch
