* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
@ 2011-03-13 22:27 Vasily Khoruzhick
2011-03-31 21:02 ` Vasily Khoruzhick
2011-04-01 7:26 ` Eric Miao
0 siblings, 2 replies; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-03-13 22:27 UTC (permalink / raw)
To: linux-arm-kernel
There's a race condition in stop_queue(),
if drv_data->queue is empty, but drv_data->busy is still set
(or opposite situation) stop_queue will return -EBUSY.
So fix loop condition to check that both drv_data->queue is empty
and drv_data->busy is not set.
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
drivers/spi/pxa2xx_spi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
index a429b01..3aa7820 100644
--- a/drivers/spi/pxa2xx_spi.c
+++ b/drivers/spi/pxa2xx_spi.c
@@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
* execution path (pump_messages) would be required to call wake_up or
* friends on every SPI message. Do this instead */
drv_data->run = QUEUE_STOPPED;
- while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
+ while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
spin_unlock_irqrestore(&drv_data->lock, flags);
msleep(10);
spin_lock_irqsave(&drv_data->lock, flags);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-03-13 22:27 [PATCH] pxa2xx_spi: Fix race condition in stop_queue() Vasily Khoruzhick
@ 2011-03-31 21:02 ` Vasily Khoruzhick
2011-04-01 7:26 ` Eric Miao
1 sibling, 0 replies; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-03-31 21:02 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 14 March 2011 00:27:10 Vasily Khoruzhick wrote:
> There's a race condition in stop_queue(),
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
Ping?
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> drivers/spi/pxa2xx_spi.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index a429b01..3aa7820 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
> * execution path (pump_messages) would be required to call wake_up or
> * friends on every SPI message. Do this instead */
> drv_data->run = QUEUE_STOPPED;
> - while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> spin_unlock_irqrestore(&drv_data->lock, flags);
> msleep(10);
> spin_lock_irqsave(&drv_data->lock, flags);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-03-13 22:27 [PATCH] pxa2xx_spi: Fix race condition in stop_queue() Vasily Khoruzhick
2011-03-31 21:02 ` Vasily Khoruzhick
@ 2011-04-01 7:26 ` Eric Miao
2011-04-01 10:03 ` Vasily Khoruzhick
1 sibling, 1 reply; 13+ messages in thread
From: Eric Miao @ 2011-04-01 7:26 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 14, 2011 at 6:27 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> There's a race condition in stop_queue(),
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
I think this is a good catch if the queue could be stopped only when
1) queue is empty AND 2) transfer is not in progress.
There are several other places you might need to change accordingly,
e.g. spi_bfin5xx.c
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> ?drivers/spi/pxa2xx_spi.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index a429b01..3aa7820 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
> ? ? ? ? * execution path (pump_messages) would be required to call wake_up or
> ? ? ? ? * friends on every SPI message. Do this instead */
> ? ? ? ?drv_data->run = QUEUE_STOPPED;
> - ? ? ? while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + ? ? ? while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&drv_data->lock, flags);
> ? ? ? ? ? ? ? ?msleep(10);
> ? ? ? ? ? ? ? ?spin_lock_irqsave(&drv_data->lock, flags);
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-01 7:26 ` Eric Miao
@ 2011-04-01 10:03 ` Vasily Khoruzhick
2011-04-04 17:35 ` Mike Frysinger
0 siblings, 1 reply; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-04-01 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 01 April 2011 10:26:37 Eric Miao wrote:
> On Mon, Mar 14, 2011 at 6:27 AM, Vasily Khoruzhick <anarsoul@gmail.com>
wrote:
> > There's a race condition in stop_queue(),
> > if drv_data->queue is empty, but drv_data->busy is still set
> > (or opposite situation) stop_queue will return -EBUSY.
> > So fix loop condition to check that both drv_data->queue is empty
> > and drv_data->busy is not set.
>
> I think this is a good catch if the queue could be stopped only when
> 1) queue is empty AND 2) transfer is not in progress.
It has check later (under while):
if (!list_empty(&drv_data->queue) || drv_data->busy)
status = -EBUSY;
And I'm hitting it on Z2 when I'm trying to enter suspend - it fails, because
spi driver refuses to enter suspend.
> There are several other places you might need to change accordingly,
> e.g. spi_bfin5xx.c
I can make a patch for other drivers, but I can't test it - I have no
appropriate boards, and even toolchain.
Regards
Vasily
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-01 10:03 ` Vasily Khoruzhick
@ 2011-04-04 17:35 ` Mike Frysinger
2011-04-04 17:47 ` Vasily Khoruzhick
0 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2011-04-04 17:35 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 1, 2011 at 06:03, Vasily Khoruzhick wrote:
> On Friday 01 April 2011 10:26:37 Eric Miao wrote:
>> On Mon, Mar 14, 2011 at 6:27 AM, Vasily Khoruzhick wrote:
>> > There's a race condition in stop_queue(),
>> > if drv_data->queue is empty, but drv_data->busy is still set
>> > (or opposite situation) stop_queue will return -EBUSY.
>> > So fix loop condition to check that both drv_data->queue is empty
>> > and drv_data->busy is not set.
>>
>> I think this is a good catch if the queue could be stopped only when
>> 1) queue is empty AND 2) transfer is not in progress.
>
> It has check later (under while):
>
> if (!list_empty(&drv_data->queue) || drv_data->busy)
> ? ? ? ?status = -EBUSY;
>
> And I'm hitting it on Z2 when I'm trying to enter suspend - it fails, because
> spi driver refuses to enter suspend.
>
>> There are several other places you might need to change accordingly,
>> e.g. spi_bfin5xx.c
>
> I can make a patch for other drivers, but I can't test it - I have no
> appropriate boards, and even toolchain.
that's fine, i do. was this patch actually merged ? or is there a
new version on the way ?
-mike
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-04 17:35 ` Mike Frysinger
@ 2011-04-04 17:47 ` Vasily Khoruzhick
2011-04-05 3:21 ` Eric Miao
0 siblings, 1 reply; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-04-04 17:47 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 04 April 2011 20:35:14 Mike Frysinger wrote:
> that's fine, i do. was this patch actually merged ? or is there a
> new version on the way ?
> -mike
I suspect it's not merged yet. Btw, should I make a patch which fixes all
drivers, or it's prefferable to make one patch per driver?
Regards
Vasily
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-04 17:47 ` Vasily Khoruzhick
@ 2011-04-05 3:21 ` Eric Miao
2011-04-06 14:46 ` Vasily Khoruzhick
0 siblings, 1 reply; 13+ messages in thread
From: Eric Miao @ 2011-04-05 3:21 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 5, 2011 at 1:47 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> On Monday 04 April 2011 20:35:14 Mike Frysinger wrote:
>
>> that's fine, i do. ?was this patch actually merged ? ?or is there a
>> new version on the way ?
>> -mike
>
> I suspect it's not merged yet. Btw, should I make a patch which fixes all
> drivers, or it's prefferable to make one patch per driver?
>
Please come up with one patch, Mike can help add a Tested-by then.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-05 3:21 ` Eric Miao
@ 2011-04-06 14:46 ` Vasily Khoruzhick
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
2011-04-07 4:58 ` [PATCH] pxa2xx_spi: " Mike Frysinger
0 siblings, 2 replies; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-04-06 14:46 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 05 April 2011 06:21:27 Eric Miao wrote:
> On Tue, Apr 5, 2011 at 1:47 AM, Vasily Khoruzhick <anarsoul@gmail.com>
wrote:
> > On Monday 04 April 2011 20:35:14 Mike Frysinger wrote:
> >> that's fine, i do. was this patch actually merged ? or is there a
> >> new version on the way ?
> >> -mike
> >
> > I suspect it's not merged yet. Btw, should I make a patch which fixes all
> > drivers, or it's prefferable to make one patch per driver?
>
> Please come up with one patch, Mike can help add a Tested-by then.
Sorry, but I did not get if you want splitted patch or not :) Anyway, 4 on-
line-patches does not look reasonable anyway, so I'm sending not-splitted
patch in few minutes.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] spi: Fix race condition in stop_queue()
2011-04-06 14:46 ` Vasily Khoruzhick
@ 2011-04-06 14:49 ` Vasily Khoruzhick
2011-04-07 6:01 ` Eric Miao
` (2 more replies)
2011-04-07 4:58 ` [PATCH] pxa2xx_spi: " Mike Frysinger
1 sibling, 3 replies; 13+ messages in thread
From: Vasily Khoruzhick @ 2011-04-06 14:49 UTC (permalink / raw)
To: linux-arm-kernel
There's a race condition in stop_queue() in some drivers -
if drv_data->queue is empty, but drv_data->busy is still set
(or opposite situation) stop_queue will return -EBUSY.
So fix loop condition to check that both drv_data->queue is empty
and drv_data->busy is not set.
This patch affects following drivers:
pxa2xx_spi
spi_bfin5xx
amba-pl022
dw_spi
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
drivers/spi/amba-pl022.c | 2 +-
drivers/spi/dw_spi.c | 2 +-
drivers/spi/pxa2xx_spi.c | 2 +-
drivers/spi/spi_bfin5xx.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
index 71a1219..23aaaa9 100644
--- a/drivers/spi/amba-pl022.c
+++ b/drivers/spi/amba-pl022.c
@@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
* A wait_queue on the pl022->busy could be used, but then the common
* execution path (pump_messages) would be required to call wake_up or
* friends on every SPI message. Do this instead */
- while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
+ while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
spin_unlock_irqrestore(&pl022->queue_lock, flags);
msleep(10);
spin_lock_irqsave(&pl022->queue_lock, flags);
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 22af77f..b53be4d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
spin_lock_irqsave(&dws->lock, flags);
dws->run = QUEUE_STOPPED;
- while (!list_empty(&dws->queue) && dws->busy && limit--) {
+ while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
spin_unlock_irqrestore(&dws->lock, flags);
msleep(10);
spin_lock_irqsave(&dws->lock, flags);
diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
index 9592883..39cc10f 100644
--- a/drivers/spi/pxa2xx_spi.c
+++ b/drivers/spi/pxa2xx_spi.c
@@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
* execution path (pump_messages) would be required to call wake_up or
* friends on every SPI message. Do this instead */
drv_data->run = QUEUE_STOPPED;
- while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
+ while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
spin_unlock_irqrestore(&drv_data->lock, flags);
msleep(10);
spin_lock_irqsave(&drv_data->lock, flags);
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index 3f22351..0e14f40 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
* friends on every SPI message. Do this instead
*/
drv_data->running = false;
- while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
+ while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
spin_unlock_irqrestore(&drv_data->lock, flags);
msleep(10);
spin_lock_irqsave(&drv_data->lock, flags);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] pxa2xx_spi: Fix race condition in stop_queue()
2011-04-06 14:46 ` Vasily Khoruzhick
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
@ 2011-04-07 4:58 ` Mike Frysinger
1 sibling, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2011-04-07 4:58 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 6, 2011 at 10:46, Vasily Khoruzhick wrote:
> On Tuesday 05 April 2011 06:21:27 Eric Miao wrote:
>> On Tue, Apr 5, 2011 at 1:47 AM, Vasily Khoruzhick wrote:
>> > On Monday 04 April 2011 20:35:14 Mike Frysinger wrote:
>> >> that's fine, i do. ?was this patch actually merged ? ?or is there a
>> >> new version on the way ?
>> >> -mike
>> >
>> > I suspect it's not merged yet. Btw, should I make a patch which fixes all
>> > drivers, or it's prefferable to make one patch per driver?
>>
>> Please come up with one patch, Mike can help add a Tested-by then.
>
> Sorry, but I did not get if you want splitted patch or not :) Anyway, 4 on-
> line-patches does not look reasonable anyway, so I'm sending not-splitted
> patch in few minutes.
yes, that is fine. thanks :).
-mike
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] spi: Fix race condition in stop_queue()
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
@ 2011-04-07 6:01 ` Eric Miao
2011-04-07 6:12 ` Mike Frysinger
2011-04-07 18:18 ` Grant Likely
2 siblings, 0 replies; 13+ messages in thread
From: Eric Miao @ 2011-04-07 6:01 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 6, 2011 at 10:49 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> There's a race condition in stop_queue() in some drivers -
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
>
> This patch affects following drivers:
> pxa2xx_spi
> spi_bfin5xx
> amba-pl022
> dw_spi
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
Acked-by: Eric Miao <eric.y.miao@gmail.com>
> ---
> ?drivers/spi/amba-pl022.c ?| ? ?2 +-
> ?drivers/spi/dw_spi.c ? ? ?| ? ?2 +-
> ?drivers/spi/pxa2xx_spi.c ?| ? ?2 +-
> ?drivers/spi/spi_bfin5xx.c | ? ?2 +-
> ?4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 71a1219..23aaaa9 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
> ? ? ? ? * A wait_queue on the pl022->busy could be used, but then the common
> ? ? ? ? * execution path (pump_messages) would be required to call wake_up or
> ? ? ? ? * friends on every SPI message. Do this instead */
> - ? ? ? while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
> + ? ? ? while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&pl022->queue_lock, flags);
> ? ? ? ? ? ? ? ?msleep(10);
> ? ? ? ? ? ? ? ?spin_lock_irqsave(&pl022->queue_lock, flags);
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 22af77f..b53be4d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
>
> ? ? ? ?spin_lock_irqsave(&dws->lock, flags);
> ? ? ? ?dws->run = QUEUE_STOPPED;
> - ? ? ? while (!list_empty(&dws->queue) && dws->busy && limit--) {
> + ? ? ? while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&dws->lock, flags);
> ? ? ? ? ? ? ? ?msleep(10);
> ? ? ? ? ? ? ? ?spin_lock_irqsave(&dws->lock, flags);
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index 9592883..39cc10f 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
> ? ? ? ? * execution path (pump_messages) would be required to call wake_up or
> ? ? ? ? * friends on every SPI message. Do this instead */
> ? ? ? ?drv_data->run = QUEUE_STOPPED;
> - ? ? ? while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + ? ? ? while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&drv_data->lock, flags);
> ? ? ? ? ? ? ? ?msleep(10);
> ? ? ? ? ? ? ? ?spin_lock_irqsave(&drv_data->lock, flags);
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index 3f22351..0e14f40 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
> ? ? ? ? * friends on every SPI message. Do this instead
> ? ? ? ? */
> ? ? ? ?drv_data->running = false;
> - ? ? ? while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + ? ? ? while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&drv_data->lock, flags);
> ? ? ? ? ? ? ? ?msleep(10);
> ? ? ? ? ? ? ? ?spin_lock_irqsave(&drv_data->lock, flags);
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] spi: Fix race condition in stop_queue()
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
2011-04-07 6:01 ` Eric Miao
@ 2011-04-07 6:12 ` Mike Frysinger
2011-04-07 18:18 ` Grant Likely
2 siblings, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2011-04-07 6:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 6, 2011 at 10:49, Vasily Khoruzhick wrote:
> ?drivers/spi/spi_bfin5xx.c | ? ?2 +-
seems to past my smoke test.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] spi: Fix race condition in stop_queue()
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
2011-04-07 6:01 ` Eric Miao
2011-04-07 6:12 ` Mike Frysinger
@ 2011-04-07 18:18 ` Grant Likely
2 siblings, 0 replies; 13+ messages in thread
From: Grant Likely @ 2011-04-07 18:18 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 06, 2011 at 05:49:15PM +0300, Vasily Khoruzhick wrote:
> There's a race condition in stop_queue() in some drivers -
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
>
> This patch affects following drivers:
> pxa2xx_spi
> spi_bfin5xx
> amba-pl022
> dw_spi
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
Applied, thanks.
g.
> ---
> drivers/spi/amba-pl022.c | 2 +-
> drivers/spi/dw_spi.c | 2 +-
> drivers/spi/pxa2xx_spi.c | 2 +-
> drivers/spi/spi_bfin5xx.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 71a1219..23aaaa9 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
> * A wait_queue on the pl022->busy could be used, but then the common
> * execution path (pump_messages) would be required to call wake_up or
> * friends on every SPI message. Do this instead */
> - while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
> + while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
> spin_unlock_irqrestore(&pl022->queue_lock, flags);
> msleep(10);
> spin_lock_irqsave(&pl022->queue_lock, flags);
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 22af77f..b53be4d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
>
> spin_lock_irqsave(&dws->lock, flags);
> dws->run = QUEUE_STOPPED;
> - while (!list_empty(&dws->queue) && dws->busy && limit--) {
> + while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
> spin_unlock_irqrestore(&dws->lock, flags);
> msleep(10);
> spin_lock_irqsave(&dws->lock, flags);
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index 9592883..39cc10f 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
> * execution path (pump_messages) would be required to call wake_up or
> * friends on every SPI message. Do this instead */
> drv_data->run = QUEUE_STOPPED;
> - while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> spin_unlock_irqrestore(&drv_data->lock, flags);
> msleep(10);
> spin_lock_irqsave(&drv_data->lock, flags);
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index 3f22351..0e14f40 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
> * friends on every SPI message. Do this instead
> */
> drv_data->running = false;
> - while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> + while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
> spin_unlock_irqrestore(&drv_data->lock, flags);
> msleep(10);
> spin_lock_irqsave(&drv_data->lock, flags);
> --
> 1.7.4.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-04-07 18:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-13 22:27 [PATCH] pxa2xx_spi: Fix race condition in stop_queue() Vasily Khoruzhick
2011-03-31 21:02 ` Vasily Khoruzhick
2011-04-01 7:26 ` Eric Miao
2011-04-01 10:03 ` Vasily Khoruzhick
2011-04-04 17:35 ` Mike Frysinger
2011-04-04 17:47 ` Vasily Khoruzhick
2011-04-05 3:21 ` Eric Miao
2011-04-06 14:46 ` Vasily Khoruzhick
2011-04-06 14:49 ` [PATCH] spi: " Vasily Khoruzhick
2011-04-07 6:01 ` Eric Miao
2011-04-07 6:12 ` Mike Frysinger
2011-04-07 18:18 ` Grant Likely
2011-04-07 4:58 ` [PATCH] pxa2xx_spi: " Mike Frysinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).