Messages in this thread | | | Date | Wed, 13 Apr 2011 22:12:31 +0200 | From | Jens Axboe <> | Subject | Re: Strange block/scsi/workqueue issue |
| |
On 2011-04-13 19:01, James Bottomley wrote: > While you still have the problematic system, can you try this patch? It > avoids changing anything in block (other than to add a missing state > guard for the elv_next_request). If it works, we can defer the sync vs > async discussion and use it for a -stable fix. > > Thanks, > > James > > --- > > diff --git a/block/blk.h b/block/blk.h > index c8db371..11d0d25 100644 > --- a/block/blk.h > +++ b/block/blk.h > @@ -62,7 +62,8 @@ static inline struct request *__elv_next_request(struct request_queue *q) > return rq; > } > > - if (!q->elevator->ops->elevator_dispatch_fn(q, 0)) > + if (test_bit(QUEUE_FLAG_DEAD, &q->queue_flags) || > + !q->elevator->ops->elevator_dispatch_fn(q, 0)) > return NULL; > } > } > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index e44ff64..5aa4246 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -322,14 +322,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) > kfree(evt); > } > > - if (sdev->request_queue) { > - sdev->request_queue->queuedata = NULL; > - /* user context needed to free queue */ > - scsi_free_queue(sdev->request_queue); > - /* temporary expedient, try to catch use of queue lock > - * after free of sdev */ > - sdev->request_queue = NULL; > - } > + /* temporary expedient, try to catch use of queue lock after > + * free of sdev */ > + sdev->request_queue = NULL; > > scsi_target_reap(scsi_target(sdev)); > > @@ -937,6 +932,11 @@ void __scsi_remove_device(struct scsi_device *sdev) > if (sdev->host->hostt->slave_destroy) > sdev->host->hostt->slave_destroy(sdev); > transport_destroy_device(dev); > + /* Setting this to NULL causes the request function to reject > + * any I/O requests */ > + sdev->request_queue->queuedata = NULL; > + /* Freeing the queue signals to block that we're done */ > + scsi_free_queue(sdev->request_queue); > put_device(dev); > }
This patch looks pretty clean. Shouldn't you serialize that ->queuedata = NULL assignment with the queue lock, though?
-- Jens Axboe
| |