Re: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-07-25 Thread Martin K. Petersen 
> "Christoph" == Christoph Hellwig  writes:

Christoph> This saves us an atomic operation for each I/O submission and
Christoph> completion for the usual case where the driver doesn't set a
Christoph> per-target can_queue value.  Only a few iscsi hardware
Christoph> offload drivers set the per-target can_queue value at the
Christoph> moment.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-07-18 Thread Christoph Hellwig 
This saves us an atomic operation for each I/O submission and completion
for the usual case where the driver doesn't set a per-target can_queue
value.  Only a few iscsi hardware offload drivers set the per-target
can_queue value at the moment.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Webb Scales 
Acked-by: Jens Axboe 
Tested-by: Bart Van Assche 
Tested-by: Robert Elliott 
---
 drivers/scsi/scsi_lib.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 69da4cb..a643353 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -295,7 +295,8 @@ void scsi_device_unbusy(struct scsi_device *sdev)
unsigned long flags;
 
atomic_dec(&shost->host_busy);
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
 
if (unlikely(scsi_host_in_recovery(shost) &&
 (shost->host_failed || shost->host_eh_scheduled))) {
@@ -364,11 +365,12 @@ static inline bool scsi_device_is_busy(struct scsi_device 
*sdev)
 
 static inline bool scsi_target_is_busy(struct scsi_target *starget)
 {
-   if (starget->can_queue > 0 &&
-   atomic_read(&starget->target_busy) >= starget->can_queue)
-   return true;
-   if (atomic_read(&starget->target_blocked) > 0)
-   return true;
+   if (starget->can_queue > 0) {
+   if (atomic_read(&starget->target_busy) >= starget->can_queue)
+   return true;
+   if (atomic_read(&starget->target_blocked) > 0)
+   return true;
+   }
return false;
 }
 
@@ -1309,6 +1311,9 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
spin_unlock_irq(shost->host_lock);
}
 
+   if (starget->can_queue <= 0)
+   return 1;
+
busy = atomic_inc_return(&starget->target_busy) - 1;
if (atomic_read(&starget->target_blocked) > 0) {
if (busy)
@@ -1324,7 +1329,7 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
 "unblocking target at zero depth\n"));
}
 
-   if (starget->can_queue > 0 && busy >= starget->can_queue)
+   if (busy >= starget->can_queue)
goto starved;
 
return 1;
@@ -1334,7 +1339,8 @@ starved:
list_move_tail(&sdev->starved_entry, &shost->starved_list);
spin_unlock_irq(shost->host_lock);
 out_dec:
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
return 0;
 }
 
@@ -1455,7 +1461,8 @@ static void scsi_kill_request(struct request *req, struct 
request_queue *q)
 */
atomic_inc(&sdev->device_busy);
atomic_inc(&shost->host_busy);
-   atomic_inc(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_inc(&starget->target_busy);
 
blk_complete_request(req);
 }
@@ -1624,7 +1631,8 @@ static void scsi_request_fn(struct request_queue *q)
return;
 
  host_not_ready:
-   atomic_dec(&scsi_target(sdev)->target_busy);
+   if (scsi_target(sdev)->can_queue > 0)
+   atomic_dec(&scsi_target(sdev)->target_busy);
  not_ready:
/*
 * lock q, handle tag, requeue req, and decrement device_busy. We
-- 
1.9.1

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

Re: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-07-09 Thread Christoph Hellwig 
On Wed, Jul 09, 2014 at 01:19:41PM +0200, Hannes Reinecke wrote:
>>host_not_ready:
>> -atomic_dec(&scsi_target(sdev)->target_busy);
>> +if (scsi_target(sdev)->can_queue > 0)
>> +atomic_dec(&scsi_target(sdev)->target_busy);
>>not_ready:
>>  /*
>>   * lock q, handle tag, requeue req, and decrement device_busy. We
>>
> Hmm. 'can_queue' can be changed by the LLDD. Don't we need some sort of 
> synchronization here?

While a few drivers change the host can_queue value at runtime none
do for the target.  While I don't think driver should even change the
host one even modification to the target one is perfectly fine as long
as no driver drops it to zero.

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

Re: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-07-09 Thread Hannes Reinecke 

On 06/25/2014 06:51 PM, Christoph Hellwig wrote:

This saves us an atomic operation for each I/O submission and completion
for the usual case where the driver doesn't set a per-target can_queue
value.  Only a few iscsi hardware offload drivers set the per-target
can_queue value at the moment.

Signed-off-by: Christoph Hellwig 
---
  drivers/scsi/scsi_lib.c |   17 -
  1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a39d5ba..a64b9d3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -295,7 +295,8 @@ void scsi_device_unbusy(struct scsi_device *sdev)
unsigned long flags;

atomic_dec(&shost->host_busy);
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);

if (unlikely(scsi_host_in_recovery(shost) &&
 (shost->host_failed || shost->host_eh_scheduled))) {
@@ -1335,6 +1336,9 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
spin_unlock_irq(shost->host_lock);
}

+   if (starget->can_queue <= 0)
+   return 1;
+
busy = atomic_inc_return(&starget->target_busy) - 1;
if (busy == 0 && atomic_read(&starget->target_blocked) > 0) {
if (atomic_dec_return(&starget->target_blocked) > 0)
@@ -1344,7 +1348,7 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
 "unblocking target at zero depth\n"));
}

-   if (starget->can_queue > 0 && busy >= starget->can_queue)
+   if (busy >= starget->can_queue)
goto starved;
if (atomic_read(&starget->target_blocked) > 0)
goto starved;
@@ -1356,7 +1360,8 @@ starved:
list_move_tail(&sdev->starved_entry, &shost->starved_list);
spin_unlock_irq(shost->host_lock);
  out_dec:
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
return 0;
  }

@@ -1473,7 +1478,8 @@ static void scsi_kill_request(struct request *req, struct 
request_queue *q)
 */
atomic_inc(&sdev->device_busy);
atomic_inc(&shost->host_busy);
-   atomic_inc(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_inc(&starget->target_busy);

blk_complete_request(req);
  }
@@ -1642,7 +1648,8 @@ static void scsi_request_fn(struct request_queue *q)
return;

   host_not_ready:
-   atomic_dec(&scsi_target(sdev)->target_busy);
+   if (scsi_target(sdev)->can_queue > 0)
+   atomic_dec(&scsi_target(sdev)->target_busy);
   not_ready:
/*
 * lock q, handle tag, requeue req, and decrement device_busy. We

Hmm. 'can_queue' can be changed by the LLDD. Don't we need some sort 
of synchronization here?

(Or move that to atomic_t, too?)

Cheers,

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

[PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-06-25 Thread Christoph Hellwig 
This saves us an atomic operation for each I/O submission and completion
for the usual case where the driver doesn't set a per-target can_queue
value.  Only a few iscsi hardware offload drivers set the per-target
can_queue value at the moment.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/scsi_lib.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a39d5ba..a64b9d3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -295,7 +295,8 @@ void scsi_device_unbusy(struct scsi_device *sdev)
unsigned long flags;
 
atomic_dec(&shost->host_busy);
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
 
if (unlikely(scsi_host_in_recovery(shost) &&
 (shost->host_failed || shost->host_eh_scheduled))) {
@@ -1335,6 +1336,9 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
spin_unlock_irq(shost->host_lock);
}
 
+   if (starget->can_queue <= 0)
+   return 1;
+
busy = atomic_inc_return(&starget->target_busy) - 1;
if (busy == 0 && atomic_read(&starget->target_blocked) > 0) {
if (atomic_dec_return(&starget->target_blocked) > 0)
@@ -1344,7 +1348,7 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
 "unblocking target at zero depth\n"));
}
 
-   if (starget->can_queue > 0 && busy >= starget->can_queue)
+   if (busy >= starget->can_queue)
goto starved;
if (atomic_read(&starget->target_blocked) > 0)
goto starved;
@@ -1356,7 +1360,8 @@ starved:
list_move_tail(&sdev->starved_entry, &shost->starved_list);
spin_unlock_irq(shost->host_lock);
 out_dec:
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
return 0;
 }
 
@@ -1473,7 +1478,8 @@ static void scsi_kill_request(struct request *req, struct 
request_queue *q)
 */
atomic_inc(&sdev->device_busy);
atomic_inc(&shost->host_busy);
-   atomic_inc(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_inc(&starget->target_busy);
 
blk_complete_request(req);
 }
@@ -1642,7 +1648,8 @@ static void scsi_request_fn(struct request_queue *q)
return;
 
  host_not_ready:
-   atomic_dec(&scsi_target(sdev)->target_busy);
+   if (scsi_target(sdev)->can_queue > 0)
+   atomic_dec(&scsi_target(sdev)->target_busy);
  not_ready:
/*
 * lock q, handle tag, requeue req, and decrement device_busy. We
-- 
1.7.10.4

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

Re: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-06-23 Thread Christoph Hellwig 
On Sat, Jun 21, 2014 at 10:10:14PM +, Elliott, Robert (Server Storage) 
wrote:
> >   not_ready:
> > /*
> >  * lock q, handle tag, requeue req, and decrement device_busy. We
> 
> There's an extra & in that if statement.

Indeed, this crept in during a rebase and a later patch fixes it.  I'll make
sure this one works on its own.

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

RE: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-06-21 Thread Elliott, Robert (Server Storage) 


> -Original Message-
> From: Christoph Hellwig [mailto:h...@lst.de]
> Sent: Thursday, 12 June, 2014 8:49 AM
> To: James Bottomley
> Cc: Jens Axboe; Bart Van Assche; Elliott, Robert (Server Storage); linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH 10/14] scsi: only maintain target_blocked if the driver has a
> target queue limit
> 
> This saves us an atomic operation for each I/O submission and completion
> for the usual case where the driver doesn't set a per-target can_queue
> value.  Only a few iscsi hardware offload drivers set the per-target
> can_queue value at the moment.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/scsi_lib.c |   17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 0e33dee..763b3c9 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
...

> @@ -1642,7 +1648,8 @@ static void scsi_request_fn(struct request_queue *q)
>   return;
> 
>   host_not_ready:
> - atomic_dec(&scsi_target(sdev)->target_busy);
> + if (&scsi_target(sdev)->can_queue > 0)
> + atomic_dec(&scsi_target(sdev)->target_busy);
>   not_ready:
>   /*
>* lock q, handle tag, requeue req, and decrement device_busy. We

There's an extra & in that if statement.

---
Rob ElliottHP Server Storage



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

[PATCH 10/14] scsi: only maintain target_blocked if the driver has a target queue limit

2014-06-12 Thread Christoph Hellwig 
This saves us an atomic operation for each I/O submission and completion
for the usual case where the driver doesn't set a per-target can_queue
value.  Only a few iscsi hardware offload drivers set the per-target
can_queue value at the moment.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/scsi_lib.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0e33dee..763b3c9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -295,7 +295,8 @@ void scsi_device_unbusy(struct scsi_device *sdev)
unsigned long flags;
 
atomic_dec(&shost->host_busy);
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
 
if (unlikely(scsi_host_in_recovery(shost) &&
 (shost->host_failed || shost->host_eh_scheduled))) {
@@ -1335,6 +1336,9 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
spin_unlock_irq(shost->host_lock);
}
 
+   if (starget->can_queue <= 0)
+   return 1;
+
busy = atomic_inc_return(&starget->target_busy) - 1;
if (busy == 0 && atomic_read(&starget->target_blocked) > 0) {
if (atomic_dec_return(&starget->target_blocked) > 0)
@@ -1344,7 +1348,7 @@ static inline int scsi_target_queue_ready(struct 
Scsi_Host *shost,
 "unblocking target at zero depth\n"));
}
 
-   if (starget->can_queue > 0 && busy >= starget->can_queue)
+   if (busy >= starget->can_queue)
goto starved;
if (atomic_read(&starget->target_blocked) > 0)
goto starved;
@@ -1356,7 +1360,8 @@ starved:
list_move_tail(&sdev->starved_entry, &shost->starved_list);
spin_unlock_irq(shost->host_lock);
 out_dec:
-   atomic_dec(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_dec(&starget->target_busy);
return 0;
 }
 
@@ -1473,7 +1478,8 @@ static void scsi_kill_request(struct request *req, struct 
request_queue *q)
 */
atomic_inc(&sdev->device_busy);
atomic_inc(&shost->host_busy);
-   atomic_inc(&starget->target_busy);
+   if (starget->can_queue > 0)
+   atomic_inc(&starget->target_busy);
 
blk_complete_request(req);
 }
@@ -1642,7 +1648,8 @@ static void scsi_request_fn(struct request_queue *q)
return;
 
  host_not_ready:
-   atomic_dec(&scsi_target(sdev)->target_busy);
+   if (&scsi_target(sdev)->can_queue > 0)
+   atomic_dec(&scsi_target(sdev)->target_busy);
  not_ready:
/*
 * lock q, handle tag, requeue req, and decrement device_busy. We
-- 
1.7.10.4

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