* [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
@ 2016-10-28 16:27 David Jeffery
2016-10-28 17:05 ` Laurence Oberman
2016-10-28 17:24 ` Ewan D. Milne
0 siblings, 2 replies; 5+ messages in thread
From: David Jeffery @ 2016-10-28 16:27 UTC (permalink / raw)
To: linux-scsi
The vmw_pvscsi driver reports most successful aborts as FAILED to the scsi
error handler. This is do to a misunderstanding of how completion_done() works
and its interaction with a successful wait using wait_for_completion_timeout().
The vmw_pvscsi driver is expecting completion_done() to always return true if
complete() has been called on the completion structure. But completion_done()
returns true after complete() has been called only if no function like
wait_for_completion_timeout() has seen the completion and cleared it as part of
successfully waiting for the completion.
Instead of using completion_done(), vmw_pvscsi should just use the return
value from wait_for_completion_timeout() to know if the wait timed out or not.
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
vmw_pvscsi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -793,6 +793,7 @@ static int pvscsi_abort(struct scsi_cmnd
unsigned long flags;
int result = SUCCESS;
DECLARE_COMPLETION_ONSTACK(abort_cmp);
+ int done;
scmd_printk(KERN_DEBUG, cmd, "task abort on host %u, %p\n",
adapter->host->host_no, cmd);
@@ -824,10 +825,10 @@ static int pvscsi_abort(struct scsi_cmnd
pvscsi_abort_cmd(adapter, ctx);
spin_unlock_irqrestore(&adapter->hw_lock, flags);
/* Wait for 2 secs for the completion. */
- wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
+ done = wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
spin_lock_irqsave(&adapter->hw_lock, flags);
- if (!completion_done(&abort_cmp)) {
+ if (!done) {
/*
* Failed to abort the command, unmark the fact that it
* was requested to be aborted.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
2016-10-28 16:27 [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts David Jeffery
@ 2016-10-28 17:05 ` Laurence Oberman
2016-10-28 17:24 ` Ewan D. Milne
1 sibling, 0 replies; 5+ messages in thread
From: Laurence Oberman @ 2016-10-28 17:05 UTC (permalink / raw)
To: David Jeffery; +Cc: linux-scsi
----- Original Message -----
> From: "David Jeffery" <djeffery@redhat.com>
> To: linux-scsi@vger.kernel.org
> Sent: Friday, October 28, 2016 12:27:26 PM
> Subject: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
>
>
> The vmw_pvscsi driver reports most successful aborts as FAILED to the scsi
> error handler. This is do to a misunderstanding of how completion_done()
> works
> and its interaction with a successful wait using
> wait_for_completion_timeout().
> The vmw_pvscsi driver is expecting completion_done() to always return true if
> complete() has been called on the completion structure. But
> completion_done()
> returns true after complete() has been called only if no function like
> wait_for_completion_timeout() has seen the completion and cleared it as part
> of
> successfully waiting for the completion.
>
> Instead of using completion_done(), vmw_pvscsi should just use the return
> value from wait_for_completion_timeout() to know if the wait timed out or
> not.
>
> Signed-off-by: David Jeffery <djeffery@redhat.com>
>
> ---
> vmw_pvscsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -793,6 +793,7 @@ static int pvscsi_abort(struct scsi_cmnd
> unsigned long flags;
> int result = SUCCESS;
> DECLARE_COMPLETION_ONSTACK(abort_cmp);
> + int done;
>
> scmd_printk(KERN_DEBUG, cmd, "task abort on host %u, %p\n",
> adapter->host->host_no, cmd);
> @@ -824,10 +825,10 @@ static int pvscsi_abort(struct scsi_cmnd
> pvscsi_abort_cmd(adapter, ctx);
> spin_unlock_irqrestore(&adapter->hw_lock, flags);
> /* Wait for 2 secs for the completion. */
> - wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> + done = wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> spin_lock_irqsave(&adapter->hw_lock, flags);
>
> - if (!completion_done(&abort_cmp)) {
> + if (!done) {
> /*
> * Failed to abort the command, unmark the fact that it
> * was requested to be aborted.
> --
> 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
>
Looks correct to me.
Reviewed-by: Laurence Oberman <loberman@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
2016-10-28 16:27 [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts David Jeffery
2016-10-28 17:05 ` Laurence Oberman
@ 2016-10-28 17:24 ` Ewan D. Milne
1 sibling, 0 replies; 5+ messages in thread
From: Ewan D. Milne @ 2016-10-28 17:24 UTC (permalink / raw)
To: David Jeffery; +Cc: linux-scsi
On Fri, 2016-10-28 at 12:27 -0400, David Jeffery wrote:
> The vmw_pvscsi driver reports most successful aborts as FAILED to the scsi
> error handler. This is do to a misunderstanding of how completion_done() works
> and its interaction with a successful wait using wait_for_completion_timeout().
> The vmw_pvscsi driver is expecting completion_done() to always return true if
> complete() has been called on the completion structure. But completion_done()
> returns true after complete() has been called only if no function like
> wait_for_completion_timeout() has seen the completion and cleared it as part of
> successfully waiting for the completion.
>
> Instead of using completion_done(), vmw_pvscsi should just use the return
> value from wait_for_completion_timeout() to know if the wait timed out or not.
>
> Signed-off-by: David Jeffery <djeffery@redhat.com>
>
> ---
> vmw_pvscsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -793,6 +793,7 @@ static int pvscsi_abort(struct scsi_cmnd
> unsigned long flags;
> int result = SUCCESS;
> DECLARE_COMPLETION_ONSTACK(abort_cmp);
> + int done;
>
> scmd_printk(KERN_DEBUG, cmd, "task abort on host %u, %p\n",
> adapter->host->host_no, cmd);
> @@ -824,10 +825,10 @@ static int pvscsi_abort(struct scsi_cmnd
> pvscsi_abort_cmd(adapter, ctx);
> spin_unlock_irqrestore(&adapter->hw_lock, flags);
> /* Wait for 2 secs for the completion. */
> - wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> + done = wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> spin_lock_irqsave(&adapter->hw_lock, flags);
>
> - if (!completion_done(&abort_cmp)) {
> + if (!done) {
> /*
> * Failed to abort the command, unmark the fact that it
> * was requested to be aborted.
> --
> 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
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
2016-10-31 22:04 Jim Gill
@ 2016-11-01 13:54 ` Martin K. Petersen
0 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2016-11-01 13:54 UTC (permalink / raw)
To: Jim Gill; +Cc: linux-scsi, emilne, djeffery, pv-drivers
>>>>> "Jim" == Jim Gill <jgill@vmware.com> writes:
Jim> VMware prefers to update the module's version number as the driver
Jim> is updated. Would you please update the
Jim> PVSCSI_DRIVER_VERSION_STRING in vmx_pvscsi.h from 1.0.6.0-k to
Jim> 1.0.7.0-k? Otherwise the patch looks fine to us.
Applied to 4.9/scsi-fixes with a bumped version number.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
@ 2016-10-31 22:04 Jim Gill
2016-11-01 13:54 ` Martin K. Petersen
0 siblings, 1 reply; 5+ messages in thread
From: Jim Gill @ 2016-10-31 22:04 UTC (permalink / raw)
To: linux-scsi; +Cc: emilne, djeffery, pv-drivers
On Fri, 2016-10-28 at 12:27 -0400, David Jeffery wrote:
> The vmw_pvscsi driver reports most successful aborts as FAILED to the scsi
> error handler. This is do to a misunderstanding of how completion_done() works
> and its interaction with a successful wait using wait_for_completion_timeout().
> The vmw_pvscsi driver is expecting completion_done() to always return true if
> complete() has been called on the completion structure. But completion_done()
> returns true after complete() has been called only if no function like
> wait_for_completion_timeout() has seen the completion and cleared it as part of
> successfully waiting for the completion.
>
> Instead of using completion_done(), vmw_pvscsi should just use the return
> value from wait_for_completion_timeout() to know if the wait timed out or not.
>
> Signed-off-by: David Jeffery <djeffery@redhat.com>
>
> ---
> vmw_pvscsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -793,6 +793,7 @@ static int pvscsi_abort(struct scsi_cmnd
> unsigned long flags;
> int result = SUCCESS;
> DECLARE_COMPLETION_ONSTACK(abort_cmp);
> + int done;
>
> scmd_printk(KERN_DEBUG, cmd, "task abort on host %u, %p\n",
> adapter->host->host_no, cmd);
> @@ -824,10 +825,10 @@ static int pvscsi_abort(struct scsi_cmnd
> pvscsi_abort_cmd(adapter, ctx);
> spin_unlock_irqrestore(&adapter->hw_lock, flags);
> /* Wait for 2 secs for the completion. */
> - wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> + done = wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> spin_lock_irqsave(&adapter->hw_lock, flags);
>
> - if (!completion_done(&abort_cmp)) {
> + if (!done) {
> /*
> * Failed to abort the command, unmark the fact that it
> * was requested to be aborted.
> --
David, VMware prefers to update the module's version number as the driver is updated. Would you
please update the PVSCSI_DRIVER_VERSION_STRING in vmx_pvscsi.h from 1.0.6.0-k to 1.0.7.0-k?
Otherwise the patch looks fine to us.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-01 13:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28 16:27 [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts David Jeffery
2016-10-28 17:05 ` Laurence Oberman
2016-10-28 17:24 ` Ewan D. Milne
2016-10-31 22:04 Jim Gill
2016-11-01 13:54 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.