question about drivers/scsi/scsi_transport_fc.c

question about drivers/scsi/scsi_transport_fc.c

[Julia Lawall]

The function fc_bsg_goose_queue in the file drivers/scsi/scsi_transport_fc.c
contains the following code:

        flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
                  !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);

I have the impression that this is always 0.  QUEUE_FLAG_REENTER is 
defined with quite a lot of other constants, so I don't really have a 
guess as to what was intended.

thanks,
julia

Re: question about drivers/scsi/scsi_transport_fc.c

[Christof Schmitt]

On Sat, Aug 28, 2010 at 07:11:12PM +0200, Julia Lawall wrote:
> The function fc_bsg_goose_queue in the file drivers/scsi/scsi_transport_fc.c
> contains the following code:
> 
>         flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
>                   !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> 
> I have the impression that this is always 0.  QUEUE_FLAG_REENTER is 
> defined with quite a lot of other constants, so I don't really have a 
> guess as to what was intended.

I just came across the code in scsi_run_queue in
drivers/scsi/scsi_lib.c. It looks like the check of QUEUE_FLAG_REENTER
in fc_bsg_goose_queue is modeled after the check in scsi_run_queue.

The check in scsi_run_queue has been introduced with this commit,
maybe this helps understanding the code:

commit 04846f25920d4b05d6040c531cc601049260db52
Author: Andreas Herrmann <aherrman@de.ibm.com>
Date:   Wed Aug 9 17:31:16 2006 +0200

    [SCSI] limit recursion when flushing shost->starved_list

--
Christof

Re: question about drivers/scsi/scsi_transport_fc.c

[Hannes Reinecke]

Christof Schmitt wrote:
> On Sat, Aug 28, 2010 at 07:11:12PM +0200, Julia Lawall wrote:
>> The function fc_bsg_goose_queue in the file drivers/scsi/scsi_transport_fc.c
>> contains the following code:
>>
>>         flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
>>                   !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
>>
>> I have the impression that this is always 0.  QUEUE_FLAG_REENTER is 
>> defined with quite a lot of other constants, so I don't really have a 
>> guess as to what was intended.
> 
> I just came across the code in scsi_run_queue in
> drivers/scsi/scsi_lib.c. It looks like the check of QUEUE_FLAG_REENTER
> in fc_bsg_goose_queue is modeled after the check in scsi_run_queue.
> 
> The check in scsi_run_queue has been introduced with this commit,
> maybe this helps understanding the code:
> 
> commit 04846f25920d4b05d6040c531cc601049260db52
> Author: Andreas Herrmann <aherrman@de.ibm.com>
> Date:   Wed Aug 9 17:31:16 2006 +0200
> 
>     [SCSI] limit recursion when flushing shost->starved_list
> 
Then it's still wrong. The above commit has:

+               if (test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
+                   !test_and_set_bit(QUEUE_FLAG_REENTER,
+                                     &sdev->request_queue->queue_flags)) {

Which is slightly different than the above. Looks like a copy and past error.
It probably should read

         flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
                   !test_and_set_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);

This was introduced by

commit 9e4f5e29610162fd426366f3b29e3cc6e575b858
Author: James Smart <James.Smart@Emulex.Com>
Date:   Thu Mar 26 13:33:19 2009 -0400

    [SCSI] FC Pass Thru support

so we should be blaming^Wasking him.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: question about drivers/scsi/scsi_transport_fc.c

[Christof Schmitt]

On Thu, Sep 02, 2010 at 03:39:39PM +0200, Hannes Reinecke wrote:
> Christof Schmitt wrote:
> > On Sat, Aug 28, 2010 at 07:11:12PM +0200, Julia Lawall wrote:
> >> The function fc_bsg_goose_queue in the file drivers/scsi/scsi_transport_fc.c
> >> contains the following code:
> >>
> >>         flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
> >>                   !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> >>
> >> I have the impression that this is always 0.  QUEUE_FLAG_REENTER is 
> >> defined with quite a lot of other constants, so I don't really have a 
> >> guess as to what was intended.
> > 
> > I just came across the code in scsi_run_queue in
> > drivers/scsi/scsi_lib.c. It looks like the check of QUEUE_FLAG_REENTER
> > in fc_bsg_goose_queue is modeled after the check in scsi_run_queue.
> > 
> > The check in scsi_run_queue has been introduced with this commit,
> > maybe this helps understanding the code:
> > 
> > commit 04846f25920d4b05d6040c531cc601049260db52
> > Author: Andreas Herrmann <aherrman@de.ibm.com>
> > Date:   Wed Aug 9 17:31:16 2006 +0200
> > 
> >     [SCSI] limit recursion when flushing shost->starved_list
> > 
> Then it's still wrong. The above commit has:
> 
> +               if (test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
> +                   !test_and_set_bit(QUEUE_FLAG_REENTER,
> +                                     &sdev->request_queue->queue_flags)) {
> 
> Which is slightly different than the above. Looks like a copy and past error.

I missed one commit that changed the locking and the flag handling:

commit 75ad23bc0fcb4f992a5d06982bf0857ab1738e9e
Author: Nick Piggin <npiggin@suse.de>
Date:   Tue Apr 29 14:48:33 2008 +0200

    block: make queue flags non-atomic

This one introduced
+               spin_lock(sdev->request_queue->queue_lock);
+               flagset = test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
+                               !test_bit(QUEUE_FLAG_REENTER,
+				&sdev->request_queue->queue_flags);
which does not make sense to me, maybe it would be enough to test the
bit?

> It probably should read
> 
>          flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
>                    !test_and_set_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> 
> This was introduced by
> 
> commit 9e4f5e29610162fd426366f3b29e3cc6e575b858
> Author: James Smart <James.Smart@Emulex.Com>
> Date:   Thu Mar 26 13:33:19 2009 -0400
> 
>     [SCSI] FC Pass Thru support
> 
> so we should be blaming^Wasking him.

Or could the whole handling of the QUEUE_FLAG_REENTER simply be
removed from the SCSI and FC code? The flag is set before calling
__blk_run_queue, but this function already sets the flag internally to
avoid recursion.

--
Christof

[PATCH] scsi: Remove QUEUE_FLAG_REENTER from SCSI code

[Christof Schmitt]

scsi_run_queue and fc_bsg_goose_queue try to call __blk_run_queue with
the flag QUEUE_FLAG_REENTER set. Since __blk_run_queue already sets
the flag internally, this part can be removed from the SCSI and FC
transport code. With this change, the function fc_bsg_goose_queue can
simply be replaced with a conditional call to blk_run_queue.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
---
 drivers/scsi/scsi_lib.c          |   15 +--------------
 drivers/scsi/scsi_transport_fc.c |   34 ++--------------------------------
 2 files changed, 3 insertions(+), 46 deletions(-)

--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -411,8 +411,6 @@ static void scsi_run_queue(struct reques
 	list_splice_init(&shost->starved_list, &starved_list);
 
 	while (!list_empty(&starved_list)) {
-		int flagset;
-
 		/*
 		 * As long as shost is accepting commands and we have
 		 * starved queues, call blk_run_queue. scsi_request_fn
@@ -436,18 +434,7 @@ static void scsi_run_queue(struct reques
 		}
 
 		spin_unlock(shost->host_lock);
-
-		spin_lock(sdev->request_queue->queue_lock);
-		flagset = test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
-				!test_bit(QUEUE_FLAG_REENTER,
-					&sdev->request_queue->queue_flags);
-		if (flagset)
-			queue_flag_set(QUEUE_FLAG_REENTER, sdev->request_queue);
-		__blk_run_queue(sdev->request_queue);
-		if (flagset)
-			queue_flag_clear(QUEUE_FLAG_REENTER, sdev->request_queue);
-		spin_unlock(sdev->request_queue->queue_lock);
-
+		blk_run_queue(sdev->request_queue);
 		spin_lock(shost->host_lock);
 	}
 	/* put any unprocessed entries back */
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -50,7 +50,6 @@ static int fc_vport_setup(struct Scsi_Ho
 static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
 static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
 static void fc_bsg_remove(struct request_queue *);
-static void fc_bsg_goose_queue(struct fc_rport *);
 
 /*
  * Redefine so that we can have same named attributes in the
@@ -2737,7 +2736,8 @@ fc_remote_port_add(struct Scsi_Host *sho
 					spin_unlock_irqrestore(shost->host_lock,
 							flags);
 
-				fc_bsg_goose_queue(rport);
+				if (rport->rqst_q)
+					blk_run_queue(rport->rqst_q);
 
 				return rport;
 			}
@@ -3752,36 +3752,6 @@ fail_host_msg:
 	return FC_DISPATCH_UNLOCKED;
 }
 
-
-/*
- * fc_bsg_goose_queue - restart rport queue in case it was stopped
- * @rport:	rport to be restarted
- */
-static void
-fc_bsg_goose_queue(struct fc_rport *rport)
-{
-	int flagset;
-	unsigned long flags;
-
-	if (!rport->rqst_q)
-		return;
-
-	get_device(&rport->dev);
-
-	spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
-	flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
-		  !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
-	if (flagset)
-		queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
-	__blk_run_queue(rport->rqst_q);
-	if (flagset)
-		queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
-	spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
-
-	put_device(&rport->dev);
-}
-
-
 /**
  * fc_bsg_rport_dispatch - process rport bsg requests and dispatch to LLDD
  * @q:		rport request queue

Re: [PATCH] scsi: Remove QUEUE_FLAG_REENTER from SCSI code

[James Bottomley]

On Thu, 2010-09-02 at 17:01 +0200, Christof Schmitt wrote:
> scsi_run_queue and fc_bsg_goose_queue try to call __blk_run_queue with
> the flag QUEUE_FLAG_REENTER set. Since __blk_run_queue already sets
> the flag internally, this part can be removed from the SCSI and FC
> transport code. With this change, the function fc_bsg_goose_queue can
> simply be replaced with a conditional call to blk_run_queue.

OK, so Hannes made me look at this again:

Actually, there is a problem here ... and the original code doesn't pick
it up.  We could still be recursing unacceptably because the fc login
interrupt can come in while we're running the block queue, but if it's
not the rport queue (which it likely won't be) we won't see the
recursion.

The actual solution is not to run the queue directly from the interrupt
at all (and dink with the REENTER) flags, but simply to use the
kblockd_schedule_work() API to cause the queue to be run in workqueue
context.  Redo the patch like that and I'll apply it.

James


> Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
> ---
>  drivers/scsi/scsi_lib.c          |   15 +--------------
>  drivers/scsi/scsi_transport_fc.c |   34 ++--------------------------------
>  2 files changed, 3 insertions(+), 46 deletions(-)
> 
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -411,8 +411,6 @@ static void scsi_run_queue(struct reques
>  	list_splice_init(&shost->starved_list, &starved_list);
>  
>  	while (!list_empty(&starved_list)) {
> -		int flagset;
> -
>  		/*
>  		 * As long as shost is accepting commands and we have
>  		 * starved queues, call blk_run_queue. scsi_request_fn
> @@ -436,18 +434,7 @@ static void scsi_run_queue(struct reques
>  		}
>  
>  		spin_unlock(shost->host_lock);
> -
> -		spin_lock(sdev->request_queue->queue_lock);
> -		flagset = test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
> -				!test_bit(QUEUE_FLAG_REENTER,
> -					&sdev->request_queue->queue_flags);
> -		if (flagset)
> -			queue_flag_set(QUEUE_FLAG_REENTER, sdev->request_queue);
> -		__blk_run_queue(sdev->request_queue);
> -		if (flagset)
> -			queue_flag_clear(QUEUE_FLAG_REENTER, sdev->request_queue);
> -		spin_unlock(sdev->request_queue->queue_lock);
> -
> +		blk_run_queue(sdev->request_queue);
>  		spin_lock(shost->host_lock);
>  	}
>  	/* put any unprocessed entries back */
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -50,7 +50,6 @@ static int fc_vport_setup(struct Scsi_Ho
>  static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
>  static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
>  static void fc_bsg_remove(struct request_queue *);
> -static void fc_bsg_goose_queue(struct fc_rport *);
>  
>  /*
>   * Redefine so that we can have same named attributes in the
> @@ -2737,7 +2736,8 @@ fc_remote_port_add(struct Scsi_Host *sho
>  					spin_unlock_irqrestore(shost->host_lock,
>  							flags);
>  
> -				fc_bsg_goose_queue(rport);
> +				if (rport->rqst_q)
> +					blk_run_queue(rport->rqst_q);
>  
>  				return rport;
>  			}
> @@ -3752,36 +3752,6 @@ fail_host_msg:
>  	return FC_DISPATCH_UNLOCKED;
>  }
>  
> -
> -/*
> - * fc_bsg_goose_queue - restart rport queue in case it was stopped
> - * @rport:	rport to be restarted
> - */
> -static void
> -fc_bsg_goose_queue(struct fc_rport *rport)
> -{
> -	int flagset;
> -	unsigned long flags;
> -
> -	if (!rport->rqst_q)
> -		return;
> -
> -	get_device(&rport->dev);
> -
> -	spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
> -	flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
> -		  !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> -	if (flagset)
> -		queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
> -	__blk_run_queue(rport->rqst_q);
> -	if (flagset)
> -		queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
> -	spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
> -
> -	put_device(&rport->dev);
> -}
> -
> -
>  /**
>   * fc_bsg_rport_dispatch - process rport bsg requests and dispatch to LLDD
>   * @q:		rport request queue
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: Remove QUEUE_FLAG_REENTER from SCSI code

[Christof Schmitt]

On Mon, Sep 27, 2010 at 04:58:13PM +0900, James Bottomley wrote:
> On Thu, 2010-09-02 at 17:01 +0200, Christof Schmitt wrote:
> > scsi_run_queue and fc_bsg_goose_queue try to call __blk_run_queue with
> > the flag QUEUE_FLAG_REENTER set. Since __blk_run_queue already sets
> > the flag internally, this part can be removed from the SCSI and FC
> > transport code. With this change, the function fc_bsg_goose_queue can
> > simply be replaced with a conditional call to blk_run_queue.
> 
> OK, so Hannes made me look at this again:
> 
> Actually, there is a problem here ... and the original code doesn't pick
> it up.  We could still be recursing unacceptably because the fc login
> interrupt can come in while we're running the block queue, but if it's
> not the rport queue (which it likely won't be) we won't see the
> recursion.
> 
> The actual solution is not to run the queue directly from the interrupt
> at all (and dink with the REENTER) flags, but simply to use the
> kblockd_schedule_work() API to cause the queue to be run in workqueue
> context.  Redo the patch like that and I'll apply it.
> 
> James

I spent some more time to understand the requirements of the flag
setting:

As far as i see, the FC rport bsg queue does not have the recursion
problem. fc_remote_port_add is the only caller of fc_bsg_goose_queue
and fc_remote_port_add has to be called from thread context. So,
fc_bsg_goose_queue can be reduced to a simple call to blk_run_queue.

scsi_run_queue has the recursion problem: It calls blk_run_queue that
can call the request_fn that in SCSI can recurse back to
scsi_queue_insert and scsi_run_queue.  The problem in commit
04846f25920d4b05d6040c531cc601049260db52 looks like a long list of
starved devices and each starved device entry adds to the stack.

So, the solution would be to:
1) Replace fc_bsg_goose_queue with a direkt call to blk_run_queue
2) As mentioned above, in scsi_run_queue defer the call to the block
   workqueue.

Is it worth adding a block layer function to keep the workqueue usage
in blk-core? Or simply open code the required steps in scsi_run_queue,
e.g.

queue_flag_set(QUEUE_FLAG_PLUGGED, q);
kblockd_schedule_work(q, &q->unplug_work);
?

Christof

> 
> 
> > Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
> > ---
> >  drivers/scsi/scsi_lib.c          |   15 +--------------
> >  drivers/scsi/scsi_transport_fc.c |   34 ++--------------------------------
> >  2 files changed, 3 insertions(+), 46 deletions(-)
> > 
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -411,8 +411,6 @@ static void scsi_run_queue(struct reques
> >  	list_splice_init(&shost->starved_list, &starved_list);
> >  
> >  	while (!list_empty(&starved_list)) {
> > -		int flagset;
> > -
> >  		/*
> >  		 * As long as shost is accepting commands and we have
> >  		 * starved queues, call blk_run_queue. scsi_request_fn
> > @@ -436,18 +434,7 @@ static void scsi_run_queue(struct reques
> >  		}
> >  
> >  		spin_unlock(shost->host_lock);
> > -
> > -		spin_lock(sdev->request_queue->queue_lock);
> > -		flagset = test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
> > -				!test_bit(QUEUE_FLAG_REENTER,
> > -					&sdev->request_queue->queue_flags);
> > -		if (flagset)
> > -			queue_flag_set(QUEUE_FLAG_REENTER, sdev->request_queue);
> > -		__blk_run_queue(sdev->request_queue);
> > -		if (flagset)
> > -			queue_flag_clear(QUEUE_FLAG_REENTER, sdev->request_queue);
> > -		spin_unlock(sdev->request_queue->queue_lock);
> > -
> > +		blk_run_queue(sdev->request_queue);
> >  		spin_lock(shost->host_lock);
> >  	}
> >  	/* put any unprocessed entries back */
> > --- a/drivers/scsi/scsi_transport_fc.c
> > +++ b/drivers/scsi/scsi_transport_fc.c
> > @@ -50,7 +50,6 @@ static int fc_vport_setup(struct Scsi_Ho
> >  static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
> >  static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
> >  static void fc_bsg_remove(struct request_queue *);
> > -static void fc_bsg_goose_queue(struct fc_rport *);
> >  
> >  /*
> >   * Redefine so that we can have same named attributes in the
> > @@ -2737,7 +2736,8 @@ fc_remote_port_add(struct Scsi_Host *sho
> >  					spin_unlock_irqrestore(shost->host_lock,
> >  							flags);
> >  
> > -				fc_bsg_goose_queue(rport);
> > +				if (rport->rqst_q)
> > +					blk_run_queue(rport->rqst_q);
> >  
> >  				return rport;
> >  			}
> > @@ -3752,36 +3752,6 @@ fail_host_msg:
> >  	return FC_DISPATCH_UNLOCKED;
> >  }
> >  
> > -
> > -/*
> > - * fc_bsg_goose_queue - restart rport queue in case it was stopped
> > - * @rport:	rport to be restarted
> > - */
> > -static void
> > -fc_bsg_goose_queue(struct fc_rport *rport)
> > -{
> > -	int flagset;
> > -	unsigned long flags;
> > -
> > -	if (!rport->rqst_q)
> > -		return;
> > -
> > -	get_device(&rport->dev);
> > -
> > -	spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
> > -	flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
> > -		  !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> > -	if (flagset)
> > -		queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
> > -	__blk_run_queue(rport->rqst_q);
> > -	if (flagset)
> > -		queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
> > -	spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
> > -
> > -	put_device(&rport->dev);
> > -}
> > -
> > -
> >  /**
> >   * fc_bsg_rport_dispatch - process rport bsg requests and dispatch to LLDD
> >   * @q:		rport request queue
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] scsi: Remove QUEUE_FLAG_REENTER from SCSI code

[Christof Schmitt]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

fc_bsg_goose_queue is only called from fc_remote_port_add which
already requires to be run from thread context. For this case simply
replace fc_bsg_goose_queue with a call to blk_run_queue.

The code in scsi_run_queue needs a mechanism to avoid recursing
through
scsi_run_queue->blk_run_queue->scsi_request_fn->scsi_dispatch_cmd
->scsi_queue_insert->scsi_run_queue for each entry in the
starved_list. Instead of using QUEUE_FLAG_PLUGGED, always defer
running the queue for starved devices to the block layer workqueue.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
---
 drivers/scsi/scsi_lib.c          |   17 +++--------------
 drivers/scsi/scsi_transport_fc.c |   34 ++--------------------------------
 2 files changed, 5 insertions(+), 46 deletions(-)

--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -411,8 +411,6 @@ static void scsi_run_queue(struct reques
 	list_splice_init(&shost->starved_list, &starved_list);
 
 	while (!list_empty(&starved_list)) {
-		int flagset;
-
 		/*
 		 * As long as shost is accepting commands and we have
 		 * starved queues, call blk_run_queue. scsi_request_fn
@@ -435,20 +433,11 @@ static void scsi_run_queue(struct reques
 			continue;
 		}
 
-		spin_unlock(shost->host_lock);
-
 		spin_lock(sdev->request_queue->queue_lock);
-		flagset = test_bit(QUEUE_FLAG_REENTER, &q->queue_flags) &&
-				!test_bit(QUEUE_FLAG_REENTER,
-					&sdev->request_queue->queue_flags);
-		if (flagset)
-			queue_flag_set(QUEUE_FLAG_REENTER, sdev->request_queue);
-		__blk_run_queue(sdev->request_queue);
-		if (flagset)
-			queue_flag_clear(QUEUE_FLAG_REENTER, sdev->request_queue);
+		queue_flag_set(QUEUE_FLAG_PLUGGED, sdev->request_queue);
+		kblockd_schedule_work(sdev->request_queue,
+				      &sdev->request_queue->unplug_work);
 		spin_unlock(sdev->request_queue->queue_lock);
-
-		spin_lock(shost->host_lock);
 	}
 	/* put any unprocessed entries back */
 	list_splice(&starved_list, &shost->starved_list);
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -50,7 +50,6 @@ static int fc_vport_setup(struct Scsi_Ho
 static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
 static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
 static void fc_bsg_remove(struct request_queue *);
-static void fc_bsg_goose_queue(struct fc_rport *);
 
 /*
  * Redefine so that we can have same named attributes in the
@@ -2737,7 +2736,8 @@ fc_remote_port_add(struct Scsi_Host *sho
 					spin_unlock_irqrestore(shost->host_lock,
 							flags);
 
-				fc_bsg_goose_queue(rport);
+				if (rport->rqst_q)
+					blk_run_queue(rport->rqst_q);
 
 				return rport;
 			}
@@ -3752,36 +3752,6 @@ fail_host_msg:
 	return FC_DISPATCH_UNLOCKED;
 }
 
-
-/*
- * fc_bsg_goose_queue - restart rport queue in case it was stopped
- * @rport:	rport to be restarted
- */
-static void
-fc_bsg_goose_queue(struct fc_rport *rport)
-{
-	int flagset;
-	unsigned long flags;
-
-	if (!rport->rqst_q)
-		return;
-
-	get_device(&rport->dev);
-
-	spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
-	flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
-		  !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
-	if (flagset)
-		queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
-	__blk_run_queue(rport->rqst_q);
-	if (flagset)
-		queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
-	spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
-
-	put_device(&rport->dev);
-}
-
-
 /**
  * fc_bsg_rport_dispatch - process rport bsg requests and dispatch to LLDD
  * @q:		rport request queue