On 06/26/12 07:25, James Bottomley wrote: > On Tue, 2012-06-26 at 06:46 +0000, Bart Van Assche wrote: >> On 06/25/12 21:14, James Bottomley wrote: >> >>> On Mon, 2012-06-25 at 18:15 +0000, Bart Van Assche wrote: >>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >>>> index 6dfb978..c26ef49 100644 >>>> --- a/drivers/scsi/scsi_lib.c >>>> +++ b/drivers/scsi/scsi_lib.c >>>> @@ -406,10 +406,7 @@ static void scsi_run_queue(struct request_queue *q) >>>> LIST_HEAD(starved_list); >>>> unsigned long flags; >>>> >>>> - /* if the device is dead, sdev will be NULL, so no queue to run */ >>>> - if (!sdev) >>>> - return; >>>> - >>>> + BUG_ON(!sdev); >>> >>> Needs to be a blk_queue_dead() check as well. >> >> Callers of scsi_run_queue() don't hold the queue lock. Does it make >> sense to test whether the queue is dead without the queue lock being held ? > > I'm not entirely sure I understand the question, but I think you think > that locking is required to read the queue state? That's emphatically > not correct. In fact, you don't even need locking to write the state > provied the state variable is a natcural processor witdth (i.e. u32 on > 32 bit or u64 or u32 on 64 bit) and you don't do dependent state > variable updates or other atomic sequences. Sorry if my comment was not clear enough. What I meant is that since callers of scsi_run_queue() don't hold the queue lock that the queue can become dead even after having checked the queue state at the start of that function. Since scsi_run_queue() can already handle queue state changes while it is in progress, it is fine to leave out the queue state check at the start of that function. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
