[linux,dev-4.10,3/3] drivers: fsi: occ: switch to irqsave and irqrestore

Message ID 1517609507-20687-4-git-send-email-eajames@linux.vnet.ibm.com
State New
Headers show
Series
  • drivers: fsi: Fixup SBEFIFO and OCC locking
Related show

Commit Message

Eddie James Feb. 2, 2018, 10:11 p.m.
Fix spinlocking by storing the interrupt state and restoring that state
when unlocking, instead of blindly disabling and re-enabling all irqs.

Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
---
 drivers/fsi/occ.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

Comments

Christopher Bostic Feb. 5, 2018, 8:14 p.m. | #1
Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 2/2/18 4:11 PM, Eddie James wrote:
> Fix spinlocking by storing the interrupt state and restoring that state
> when unlocking, instead of blindly disabling and re-enabling all irqs.
>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
>   drivers/fsi/occ.c | 44 +++++++++++++++++++++++++-------------------
>   1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/fsi/occ.c b/drivers/fsi/occ.c
> index adc64f3..edb5e977 100644
> --- a/drivers/fsi/occ.c
> +++ b/drivers/fsi/occ.c
> @@ -118,18 +118,19 @@ struct occ_client {
>   static int occ_enqueue_xfr(struct occ_xfr *xfr)
>   {
>   	int empty;
> +	unsigned long flags;
>   	struct occ_client *client = to_client(xfr);
>   	struct occ *occ = client->occ;
>
>   	if (occ->cancel)
>   		return -ENODEV;
>
> -	spin_lock_irq(&occ->list_lock);
> +	spin_lock_irqsave(&occ->list_lock, flags);
>
>   	empty = list_empty(&occ->xfrs);
>   	list_add_tail(&xfr->link, &occ->xfrs);
>
> -	spin_unlock_irq(&occ->list_lock);
> +	spin_unlock_irqrestore(&occ->list_lock, flags);
>
>   	if (empty)
>   		queue_work(occ_wq, &occ->work);
> @@ -201,6 +202,7 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
>   	size_t bytes;
>   	struct occ_xfr *xfr;
>   	struct occ *occ;
> +	unsigned long flags;
>
>   	if (!client)
>   		return -ENODEV;
> @@ -212,7 +214,7 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
>   	xfr = &client->xfr;
>   	occ = client->occ;
>
> -	spin_lock_irq(&client->lock);
> +	spin_lock_irqsave(&client->lock, flags);
>
>   	if (!test_bit(CLIENT_XFR_PENDING, &client->flags)) {
>   		/* we just finished reading all data, return 0 */
> @@ -232,12 +234,12 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
>   			goto done;
>   		}
>
> -		spin_unlock_irq(&client->lock);
> +		spin_unlock_irqrestore(&client->lock, flags);
>
>   		rc = wait_event_interruptible(client->wait,
>   					      occ_read_ready(xfr, occ));
>
> -		spin_lock_irq(&client->lock);
> +		spin_lock_irqsave(&client->lock, flags);
>
>   		if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
>   			if (occ->cancel || test_bit(XFR_CANCELED, &xfr->flags))
> @@ -274,7 +276,7 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
>   	rc = bytes;
>
>   done:
> -	spin_unlock_irq(&client->lock);
> +	spin_unlock_irqrestore(&client->lock, flags);
>   	occ_put_client(client);
>   	return rc;
>   }
> @@ -293,6 +295,7 @@ static ssize_t occ_write_common(struct occ_client *client,
>   {
>   	int rc;
>   	unsigned int i;
> +	unsigned long flags;
>   	u16 data_length, checksum = 0;
>   	struct occ_xfr *xfr;
>
> @@ -305,7 +308,7 @@ static ssize_t occ_write_common(struct occ_client *client,
>   	occ_get_client(client);
>   	xfr = &client->xfr;
>
> -	spin_lock_irq(&client->lock);
> +	spin_lock_irqsave(&client->lock, flags);
>
>   	if (test_bit(CLIENT_XFR_PENDING, &client->flags)) {
>   		rc = -EBUSY;
> @@ -353,7 +356,7 @@ static ssize_t occ_write_common(struct occ_client *client,
>   	rc = len;
>
>   done:
> -	spin_unlock_irq(&client->lock);
> +	spin_unlock_irqrestore(&client->lock, flags);
>   	occ_put_client(client);
>   	return rc;
>   }
> @@ -370,6 +373,7 @@ static int occ_release_common(struct occ_client *client)
>   {
>   	struct occ *occ;
>   	struct occ_xfr *xfr;
> +	unsigned long flags;
>
>   	if (!client)
>   		return -ENODEV;
> @@ -377,13 +381,13 @@ static int occ_release_common(struct occ_client *client)
>   	xfr = &client->xfr;
>   	occ = client->occ;
>
> -	spin_lock_irq(&client->lock);
> +	spin_lock_irqsave(&client->lock, flags);
>
>   	set_bit(XFR_CANCELED, &xfr->flags);
>   	if (!test_bit(CLIENT_XFR_PENDING, &client->flags))
>   		goto done;
>
> -	spin_lock_irq(&occ->list_lock);
> +	spin_lock(&occ->list_lock);
>
>   	if (!test_bit(XFR_IN_PROGRESS, &xfr->flags)) {
>   		/* already deleted from list if complete */
> @@ -391,12 +395,12 @@ static int occ_release_common(struct occ_client *client)
>   			list_del(&xfr->link);
>   	}
>
> -	spin_unlock_irq(&occ->list_lock);
> +	spin_unlock(&occ->list_lock);
>
>   	wake_up_all(&client->wait);
>
>   done:
> -	spin_unlock_irq(&client->lock);
> +	spin_unlock_irqrestore(&client->lock, flags);
>
>   	occ_put_client(client);
>   	return 0;
> @@ -606,6 +610,7 @@ static void occ_worker(struct work_struct *work)
>   {
>   	int rc = 0, empty;
>   	u16 resp_data_length;
> +	unsigned long flags;
>   	unsigned long start;
>   	const unsigned long timeout = msecs_to_jiffies(OCC_TIMEOUT_MS);
>   	const long int wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
> @@ -619,11 +624,11 @@ static void occ_worker(struct work_struct *work)
>   	if (occ->cancel)
>   		return;
>
> -	spin_lock_irq(&occ->list_lock);
> +	spin_lock_irqsave(&occ->list_lock, flags);
>
>   	xfr = list_first_entry_or_null(&occ->xfrs, struct occ_xfr, link);
>   	if (!xfr) {
> -		spin_unlock_irq(&occ->list_lock);
> +		spin_unlock_irqrestore(&occ->list_lock, flags);
>   		return;
>   	}
>
> @@ -632,7 +637,7 @@ static void occ_worker(struct work_struct *work)
>   	resp = (struct occ_response *)xfr->buf;
>   	set_bit(XFR_IN_PROGRESS, &xfr->flags);
>
> -	spin_unlock_irq(&occ->list_lock);
> +	spin_unlock_irqrestore(&occ->list_lock, flags);
>   	mutex_lock(&occ->occ_lock);
>
>   	start = jiffies;
> @@ -686,13 +691,13 @@ static void occ_worker(struct work_struct *work)
>   	xfr->rc = rc;
>   	set_bit(XFR_COMPLETE, &xfr->flags);
>
> -	spin_lock_irq(&occ->list_lock);
> +	spin_lock_irqsave(&occ->list_lock, flags);
>
>   	clear_bit(XFR_IN_PROGRESS, &xfr->flags);
>   	list_del(&xfr->link);
>   	empty = list_empty(&occ->xfrs);
>
> -	spin_unlock_irq(&occ->list_lock);
> +	spin_unlock_irqrestore(&occ->list_lock, flags);
>
>   	wake_up_interruptible(&client->wait);
>   	occ_put_client(client);
> @@ -810,15 +815,16 @@ static int occ_remove(struct platform_device *pdev)
>   	struct occ *occ = platform_get_drvdata(pdev);
>   	struct occ_xfr *xfr;
>   	struct occ_client *client;
> +	unsigned long flags;
>
>   	occ->cancel = true;
>
> -	spin_lock_irq(&occ->list_lock);
> +	spin_lock_irqsave(&occ->list_lock, flags);
>   	list_for_each_entry(xfr, &occ->xfrs, link) {
>   		client = to_client(xfr);
>   		wake_up_all(&client->wait);
>   	}
> -	spin_unlock_irq(&occ->list_lock);
> +	spin_unlock_irqrestore(&occ->list_lock, flags);
>
>   	misc_deregister(&occ->mdev);
>   	device_for_each_child(&pdev->dev, NULL, occ_unregister_child);

Patch

diff --git a/drivers/fsi/occ.c b/drivers/fsi/occ.c
index adc64f3..edb5e977 100644
--- a/drivers/fsi/occ.c
+++ b/drivers/fsi/occ.c
@@ -118,18 +118,19 @@  struct occ_client {
 static int occ_enqueue_xfr(struct occ_xfr *xfr)
 {
 	int empty;
+	unsigned long flags;
 	struct occ_client *client = to_client(xfr);
 	struct occ *occ = client->occ;
 
 	if (occ->cancel)
 		return -ENODEV;
 
-	spin_lock_irq(&occ->list_lock);
+	spin_lock_irqsave(&occ->list_lock, flags);
 
 	empty = list_empty(&occ->xfrs);
 	list_add_tail(&xfr->link, &occ->xfrs);
 
-	spin_unlock_irq(&occ->list_lock);
+	spin_unlock_irqrestore(&occ->list_lock, flags);
 
 	if (empty)
 		queue_work(occ_wq, &occ->work);
@@ -201,6 +202,7 @@  static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
 	size_t bytes;
 	struct occ_xfr *xfr;
 	struct occ *occ;
+	unsigned long flags;
 
 	if (!client)
 		return -ENODEV;
@@ -212,7 +214,7 @@  static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
 	xfr = &client->xfr;
 	occ = client->occ;
 
-	spin_lock_irq(&client->lock);
+	spin_lock_irqsave(&client->lock, flags);
 
 	if (!test_bit(CLIENT_XFR_PENDING, &client->flags)) {
 		/* we just finished reading all data, return 0 */
@@ -232,12 +234,12 @@  static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
 			goto done;
 		}
 
-		spin_unlock_irq(&client->lock);
+		spin_unlock_irqrestore(&client->lock, flags);
 
 		rc = wait_event_interruptible(client->wait,
 					      occ_read_ready(xfr, occ));
 
-		spin_lock_irq(&client->lock);
+		spin_lock_irqsave(&client->lock, flags);
 
 		if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
 			if (occ->cancel || test_bit(XFR_CANCELED, &xfr->flags))
@@ -274,7 +276,7 @@  static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
 	rc = bytes;
 
 done:
-	spin_unlock_irq(&client->lock);
+	spin_unlock_irqrestore(&client->lock, flags);
 	occ_put_client(client);
 	return rc;
 }
@@ -293,6 +295,7 @@  static ssize_t occ_write_common(struct occ_client *client,
 {
 	int rc;
 	unsigned int i;
+	unsigned long flags;
 	u16 data_length, checksum = 0;
 	struct occ_xfr *xfr;
 
@@ -305,7 +308,7 @@  static ssize_t occ_write_common(struct occ_client *client,
 	occ_get_client(client);
 	xfr = &client->xfr;
 
-	spin_lock_irq(&client->lock);
+	spin_lock_irqsave(&client->lock, flags);
 
 	if (test_bit(CLIENT_XFR_PENDING, &client->flags)) {
 		rc = -EBUSY;
@@ -353,7 +356,7 @@  static ssize_t occ_write_common(struct occ_client *client,
 	rc = len;
 
 done:
-	spin_unlock_irq(&client->lock);
+	spin_unlock_irqrestore(&client->lock, flags);
 	occ_put_client(client);
 	return rc;
 }
@@ -370,6 +373,7 @@  static int occ_release_common(struct occ_client *client)
 {
 	struct occ *occ;
 	struct occ_xfr *xfr;
+	unsigned long flags;
 
 	if (!client)
 		return -ENODEV;
@@ -377,13 +381,13 @@  static int occ_release_common(struct occ_client *client)
 	xfr = &client->xfr;
 	occ = client->occ;
 
-	spin_lock_irq(&client->lock);
+	spin_lock_irqsave(&client->lock, flags);
 
 	set_bit(XFR_CANCELED, &xfr->flags);
 	if (!test_bit(CLIENT_XFR_PENDING, &client->flags))
 		goto done;
 
-	spin_lock_irq(&occ->list_lock);
+	spin_lock(&occ->list_lock);
 
 	if (!test_bit(XFR_IN_PROGRESS, &xfr->flags)) {
 		/* already deleted from list if complete */
@@ -391,12 +395,12 @@  static int occ_release_common(struct occ_client *client)
 			list_del(&xfr->link);
 	}
 
-	spin_unlock_irq(&occ->list_lock);
+	spin_unlock(&occ->list_lock);
 
 	wake_up_all(&client->wait);
 
 done:
-	spin_unlock_irq(&client->lock);
+	spin_unlock_irqrestore(&client->lock, flags);
 
 	occ_put_client(client);
 	return 0;
@@ -606,6 +610,7 @@  static void occ_worker(struct work_struct *work)
 {
 	int rc = 0, empty;
 	u16 resp_data_length;
+	unsigned long flags;
 	unsigned long start;
 	const unsigned long timeout = msecs_to_jiffies(OCC_TIMEOUT_MS);
 	const long int wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
@@ -619,11 +624,11 @@  static void occ_worker(struct work_struct *work)
 	if (occ->cancel)
 		return;
 
-	spin_lock_irq(&occ->list_lock);
+	spin_lock_irqsave(&occ->list_lock, flags);
 
 	xfr = list_first_entry_or_null(&occ->xfrs, struct occ_xfr, link);
 	if (!xfr) {
-		spin_unlock_irq(&occ->list_lock);
+		spin_unlock_irqrestore(&occ->list_lock, flags);
 		return;
 	}
 
@@ -632,7 +637,7 @@  static void occ_worker(struct work_struct *work)
 	resp = (struct occ_response *)xfr->buf;
 	set_bit(XFR_IN_PROGRESS, &xfr->flags);
 
-	spin_unlock_irq(&occ->list_lock);
+	spin_unlock_irqrestore(&occ->list_lock, flags);
 	mutex_lock(&occ->occ_lock);
 
 	start = jiffies;
@@ -686,13 +691,13 @@  static void occ_worker(struct work_struct *work)
 	xfr->rc = rc;
 	set_bit(XFR_COMPLETE, &xfr->flags);
 
-	spin_lock_irq(&occ->list_lock);
+	spin_lock_irqsave(&occ->list_lock, flags);
 
 	clear_bit(XFR_IN_PROGRESS, &xfr->flags);
 	list_del(&xfr->link);
 	empty = list_empty(&occ->xfrs);
 
-	spin_unlock_irq(&occ->list_lock);
+	spin_unlock_irqrestore(&occ->list_lock, flags);
 
 	wake_up_interruptible(&client->wait);
 	occ_put_client(client);
@@ -810,15 +815,16 @@  static int occ_remove(struct platform_device *pdev)
 	struct occ *occ = platform_get_drvdata(pdev);
 	struct occ_xfr *xfr;
 	struct occ_client *client;
+	unsigned long flags;
 
 	occ->cancel = true;
 
-	spin_lock_irq(&occ->list_lock);
+	spin_lock_irqsave(&occ->list_lock, flags);
 	list_for_each_entry(xfr, &occ->xfrs, link) {
 		client = to_client(xfr);
 		wake_up_all(&client->wait);
 	}
-	spin_unlock_irq(&occ->list_lock);
+	spin_unlock_irqrestore(&occ->list_lock, flags);
 
 	misc_deregister(&occ->mdev);
 	device_for_each_child(&pdev->dev, NULL, occ_unregister_child);