Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-07 Thread Jason Yan 


On 2017/12/7 17:27, John Garry wrote:

On 07/12/2017 01:41, Jason Yan wrote:

Can anybody review this patch? Our test of SG_IO all failed because of
this bug.

On 2017/12/5 17:39, Jason Yan wrote:

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
  drivers/scsi/libsas/sas_expander.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,

  ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
  job->reply_payload.sg_list);
-if (ret > 0) {
+if (ret >= 0) {
  /* positive number is the untransferred residual */
-reslen = ret;
+reslen = job->reply_payload.payload_len - ret;


Hi Jason,

If we really want the length of the payload received, then should you
change the reslen variable name? The name implies "residual length",
which is not really what it is holding according to your change.

Thanks,
John



Thanks a lot. I will correct it.

Jason


  ret = 0;
  }





.





.

Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-07 Thread John Garry 

On 07/12/2017 01:41, Jason Yan wrote:

Can anybody review this patch? Our test of SG_IO all failed because of
this bug.

On 2017/12/5 17:39, Jason Yan wrote:

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
  drivers/scsi/libsas/sas_expander.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,

  ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
  job->reply_payload.sg_list);
-if (ret > 0) {
+if (ret >= 0) {
  /* positive number is the untransferred residual */
-reslen = ret;
+reslen = job->reply_payload.payload_len - ret;


Hi Jason,

If we really want the length of the payload received, then should you 
change the reslen variable name? The name implies "residual length", 
which is not really what it is holding according to your change.


Thanks,
John


  ret = 0;
  }





.

Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-06 Thread Jason Yan 

Can anybody review this patch? Our test of SG_IO all failed because of
this bug.

On 2017/12/5 17:39, Jason Yan wrote:

The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
  drivers/scsi/libsas/sas_expander.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct 
Scsi_Host *shost,

ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
job->reply_payload.sg_list);
-   if (ret > 0) {
+   if (ret >= 0) {
/* positive number is the untransferred residual */
-   reslen = ret;
+   reslen = job->reply_payload.payload_len - ret;
ret = 0;
}

[PATCH] scsi: libsas: fix length error in sas_smp_handler()

2017-12-05 Thread Jason Yan 
The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.

Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP")
Reported-and-tested-by: chenqilin 
Signed-off-by: Jason Yan 
CC: Christoph Hellwig 
---
 drivers/scsi/libsas/sas_expander.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct 
Scsi_Host *shost,
 
ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
job->reply_payload.sg_list);
-   if (ret > 0) {
+   if (ret >= 0) {
/* positive number is the untransferred residual */
-   reslen = ret;
+   reslen = job->reply_payload.payload_len - ret;
ret = 0;
}
 
-- 
2.9.5