From 6aa2dd0092d2762cc12cccf5142313858f4153a4 Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard@mvista.com>
Date: Thu, 5 Apr 2018 14:40:30 -0500
Subject: [PATCH] ipmi_devintf: Small lock rework
Git-commit: 6aa2dd0092d2762cc12cccf5142313858f4153a4
Patch-mainline: v4.18-rc1
References: FATE#326156
The mutex didn't really serve any useful purpose, from what I can
tell, and it would just get in the way. So remove it.
Removing that required a mutex around the default value setting and
getting, so just use the receive mutex for that.
Also pull the fasync stuff outside of the lock for adding the data
to the queue, since it didn't need to be there.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Acked-by: Takashi Iwai <tiwai@suse.de>
---
drivers/char/ipmi/ipmi_devintf.c | 83 +++++++++++++--------------------------
1 file changed, 28 insertions(+), 55 deletions(-)
--- a/drivers/char/ipmi/ipmi_devintf.c
+++ b/drivers/char/ipmi/ipmi_devintf.c
@@ -37,7 +37,6 @@ struct ipmi_file_private
unsigned int default_retry_time_ms;
};
-static DEFINE_MUTEX(ipmi_mutex);
static void file_receive_handler(struct ipmi_recv_msg *msg,
void *handler_data)
{
@@ -46,16 +45,14 @@ static void file_receive_handler(struct
unsigned long flags;
spin_lock_irqsave(&priv->recv_msg_lock, flags);
-
was_empty = list_empty(&priv->recv_msgs);
list_add_tail(&msg->link, &priv->recv_msgs);
+ spin_unlock_irqrestore(&priv->recv_msg_lock, flags);
if (was_empty) {
wake_up_interruptible(&priv->wait);
kill_fasync(&priv->fasync_queue, SIGIO, POLL_IN);
}
-
- spin_unlock_irqrestore(&priv->recv_msg_lock, flags);
}
static unsigned int ipmi_poll(struct file *file, poll_table *wait)
@@ -79,13 +76,8 @@ static unsigned int ipmi_poll(struct fil
static int ipmi_fasync(int fd, struct file *file, int on)
{
struct ipmi_file_private *priv = file->private_data;
- int result;
-
- mutex_lock(&ipmi_mutex); /* could race against open() otherwise */
- result = fasync_helper(fd, file, on, &priv->fasync_queue);
- mutex_unlock(&ipmi_mutex);
- return (result);
+ return fasync_helper(fd, file, on, &priv->fasync_queue);
}
static const struct ipmi_user_hndl ipmi_hndlrs =
@@ -99,12 +91,10 @@ static int ipmi_open(struct inode *inode
int rv;
struct ipmi_file_private *priv;
-
priv = kmalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- mutex_lock(&ipmi_mutex);
priv->file = file;
rv = ipmi_create_user(if_num,
@@ -129,7 +119,6 @@ static int ipmi_open(struct inode *inode
priv->default_retry_time_ms = 0;
out:
- mutex_unlock(&ipmi_mutex);
return rv;
}
@@ -137,7 +126,7 @@ static int ipmi_release(struct inode *in
{
struct ipmi_file_private *priv = file->private_data;
int rv;
- struct ipmi_recv_msg *msg, *next;
+ struct ipmi_recv_msg *msg, *next;
rv = ipmi_destroy_user(priv->user);
if (rv)
@@ -303,9 +292,9 @@ static int copyout_recv(struct ipmi_recv
return copy_to_user(to, rsp, sizeof(struct ipmi_recv)) ? -EFAULT : 0;
}
-static int ipmi_ioctl(struct file *file,
- unsigned int cmd,
- unsigned long data)
+static long ipmi_ioctl(struct file *file,
+ unsigned int cmd,
+ unsigned long data)
{
int rv = -EINVAL;
struct ipmi_file_private *priv = file->private_data;
@@ -316,16 +305,20 @@ static int ipmi_ioctl(struct file *fil
case IPMICTL_SEND_COMMAND:
{
struct ipmi_req req;
+ int retries;
+ unsigned int retry_time_ms;
if (copy_from_user(&req, arg, sizeof(req))) {
rv = -EFAULT;
break;
}
- rv = handle_send_req(priv->user,
- &req,
- priv->default_retries,
- priv->default_retry_time_ms);
+ mutex_lock(&priv->recv_mutex);
+ retries = priv->default_retries;
+ retry_time_ms = priv->default_retry_time_ms;
+ mutex_unlock(&priv->recv_mutex);
+
+ rv = handle_send_req(priv->user, &req, retries, retry_time_ms);
break;
}
@@ -565,8 +558,10 @@ static int ipmi_ioctl(struct file *fil
break;
}
+ mutex_lock(&priv->recv_mutex);
priv->default_retries = parms.retries;
priv->default_retry_time_ms = parms.retry_time_ms;
+ mutex_unlock(&priv->recv_mutex);
rv = 0;
break;
}
@@ -575,8 +570,10 @@ static int ipmi_ioctl(struct file *fil
{
struct ipmi_timing_parms parms;
+ mutex_lock(&priv->recv_mutex);
parms.retries = priv->default_retries;
parms.retry_time_ms = priv->default_retry_time_ms;
+ mutex_unlock(&priv->recv_mutex);
if (copy_to_user(arg, &parms, sizeof(parms))) {
rv = -EFAULT;
@@ -616,25 +613,7 @@ static int ipmi_ioctl(struct file *fil
return rv;
}
-/*
- * Note: it doesn't make sense to take the BKL here but
- * not in compat_ipmi_ioctl. -arnd
- */
-static long ipmi_unlocked_ioctl(struct file *file,
- unsigned int cmd,
- unsigned long data)
-{
- int ret;
-
- mutex_lock(&ipmi_mutex);
- ret = ipmi_ioctl(file, cmd, data);
- mutex_unlock(&ipmi_mutex);
-
- return ret;
-}
-
#ifdef CONFIG_COMPAT
-
/*
* The following code contains code for supporting 32-bit compatible
* ioctls on 64-bit kernels. This allows running 32-bit apps on the
@@ -745,15 +724,21 @@ static long compat_ipmi_ioctl(struct fil
{
struct ipmi_req rp;
struct compat_ipmi_req r32;
+ int retries;
+ unsigned int retry_time_ms;
if (copy_from_user(&r32, compat_ptr(arg), sizeof(r32)))
return -EFAULT;
get_compat_ipmi_req(&rp, &r32);
+ mutex_lock(&priv->recv_mutex);
+ retries = priv->default_retries;
+ retry_time_ms = priv->default_retry_time_ms;
+ mutex_unlock(&priv->recv_mutex);
+
return handle_send_req(priv->user, &rp,
- priv->default_retries,
- priv->default_retry_time_ms);
+ retries, retry_time_ms);
}
case COMPAT_IPMICTL_SEND_COMMAND_SETTIME:
{
@@ -787,25 +772,13 @@ static long compat_ipmi_ioctl(struct fil
return ipmi_ioctl(filep, cmd, arg);
}
}
-
-static long unlocked_compat_ipmi_ioctl(struct file *filep, unsigned int cmd,
- unsigned long arg)
-{
- int ret;
-
- mutex_lock(&ipmi_mutex);
- ret = compat_ipmi_ioctl(filep, cmd, arg);
- mutex_unlock(&ipmi_mutex);
-
- return ret;
-}
#endif
static const struct file_operations ipmi_fops = {
.owner = THIS_MODULE,
- .unlocked_ioctl = ipmi_unlocked_ioctl,
+ .unlocked_ioctl = ipmi_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = unlocked_compat_ipmi_ioctl,
+ .compat_ioctl = compat_ipmi_ioctl,
#endif
.open = ipmi_open,
.release = ipmi_release,