Re: [PATCH 5/5] uas: add locking

[Oliver Neukum]

On Wednesday 19 September 2012 14:40:12 Gerd Hoffmann wrote:
> @@ -101,7 +103,10 @@ static void uas_do_work(struct work_struct *work)
> struct scsi_pointer *scp = (void *)cmdinfo;
> struct scsi_cmnd *cmnd = container_of(scp,
> struct scsi_cmnd, 
> SCp);
> +   struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
> +   spin_lock_irqsave(&devinfo->lock, flags);
> err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);

You no longer can use GFP_NOIO

> +   spin_unlock_irqrestore(&devinfo->lock, flags);
> if (err) {
> list_del(&cmdinfo->list);
> spin_lock_irq(&uas_work_lock);

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] uas: add locking

[Gerd Hoffmann]

Add spinlock to protect uas data structures.

Signed-off-by: Gerd Hoffmann 
---
 drivers/usb/storage/uas.c |   45 +
 1 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index df1d72e..cb5c9e3 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -50,6 +50,7 @@ struct uas_dev_info {
unsigned use_streams:1;
unsigned uas_sense_old:1;
struct scsi_cmnd *cmnd;
+   spinlock_t lock;
 };
 
 enum {
@@ -91,6 +92,7 @@ static void uas_do_work(struct work_struct *work)
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
struct list_head list;
+   unsigned long flags;
int err;
 
spin_lock_irq(&uas_work_lock);
@@ -101,7 +103,10 @@ static void uas_do_work(struct work_struct *work)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
+   struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
+   spin_lock_irqsave(&devinfo->lock, flags);
err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
+   spin_unlock_irqrestore(&devinfo->lock, flags);
if (err) {
list_del(&cmdinfo->list);
spin_lock_irq(&uas_work_lock);
@@ -182,7 +187,9 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
 {
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
+   struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
 
+   WARN_ON(!spin_is_locked(&devinfo->lock));
if (cmdinfo->state & (COMMAND_INFLIGHT |
  DATA_IN_URB_INFLIGHT |
  DATA_OUT_URB_INFLIGHT))
@@ -222,6 +229,7 @@ static void uas_stat_cmplt(struct urb *urb)
struct uas_dev_info *devinfo = (void *)shost->hostdata[0];
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
+   unsigned long flags;
u16 tag;
 
if (urb->status) {
@@ -235,6 +243,7 @@ static void uas_stat_cmplt(struct urb *urb)
return;
}
 
+   spin_lock_irqsave(&devinfo->lock, flags);
tag = be16_to_cpup(&iu->tag) - 1;
if (tag == 0)
cmnd = devinfo->cmnd;
@@ -243,6 +252,7 @@ static void uas_stat_cmplt(struct urb *urb)
if (!cmnd) {
if (iu->iu_id != IU_ID_RESPONSE) {
usb_free_urb(urb);
+   spin_unlock_irqrestore(&devinfo->lock, flags);
return;
}
} else {
@@ -262,10 +272,16 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd->result != 0) {
/* cancel data transfers on error */
-   if (cmdinfo->state & DATA_IN_URB_INFLIGHT)
+   if (cmdinfo->state & DATA_IN_URB_INFLIGHT) {
+   spin_unlock_irqrestore(&devinfo->lock, flags);
usb_unlink_urb(cmdinfo->data_in_urb);
-   if (cmdinfo->state & DATA_OUT_URB_INFLIGHT)
+   spin_lock_irqsave(&devinfo->lock, flags);
+   }
+   if (cmdinfo->state & DATA_OUT_URB_INFLIGHT) {
+   spin_unlock_irqrestore(&devinfo->lock, flags);
usb_unlink_urb(cmdinfo->data_out_urb);
+   spin_lock_irqsave(&devinfo->lock, flags);
+   }
}
cmdinfo->state &= ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
@@ -285,14 +301,18 @@ static void uas_stat_cmplt(struct urb *urb)
"Bogus IU (%d) received on status pipe\n", iu->iu_id);
}
usb_free_urb(urb);
+   spin_unlock_irqrestore(&devinfo->lock, flags);
 }
 
 static void uas_data_cmplt(struct urb *urb)
 {
struct scsi_cmnd *cmnd = urb->context;
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
+   struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
struct scsi_data_buffer *sdb = NULL;
+   unsigned long flags;
 
+   spin_lock_irqsave(&devinfo->lock, flags);
if (cmdinfo->data_in_urb == urb) {
sdb = scsi_in(cmnd);
cmdinfo->state &= ~DATA_IN_URB_INFLIGHT;
@@ -308,6 +328,7 @@ static void uas_data_cmplt(struct urb *urb)
sdb->resid = sdb->length - urb->actual_length;
}
uas_try_complete(cmnd, __func__);
+   spin_unlock_irqrestore(&devinfo->lock, flags);
 }
 
 static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp