[linux,dev-4.10] drivers: fsi: sbefifo: don't delete canceled xfrs in write

Message ID 1517613287-23508-1-git-send-email-eajames@linux.vnet.ibm.com
State Not Applicable, archived
Headers show
Series
  • [linux,dev-4.10] drivers: fsi: sbefifo: don't delete canceled xfrs in write
Related show

Commit Message

Eddie James Feb. 2, 2018, 11:14 p.m.
The write function was deleting canceled transfers before they had a
change to run and complete. This results in not acking the EOT and not
reading the rest of the data left in the FIFO for the canceled transfer.
This is the cause of the EBADMSG errors found in the OCC driver, as the
next op reads the data left over.

Fix it by just checking the first entry of the list instead of
iterating and canceling.

Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
---

Joel, I based this on top of the "Fixup SBEFIFO and OCC locking" series. Let me
know if I need to put this first and rebase that series... thanks.

 drivers/fsi/fsi-sbefifo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Christopher Bostic Feb. 5, 2018, 3:58 p.m. | #1
Reviewed-by: Christopher Bostic <clbostic@linux.vnet.ibm.com>


On 2/2/18 5:14 PM, Eddie James wrote:
> The write function was deleting canceled transfers before they had a
> change to run and complete. This results in not acking the EOT and not
Eddie,

Small nitpick 'change' should be 'chance'.   Not sure its worth a 
re-spin unless there are other changes required.

Thanks
-Chris
> reading the rest of the data left in the FIFO for the canceled transfer.
> This is the cause of the EBADMSG errors found in the OCC driver, as the
> next op reads the data left over.
>
> Fix it by just checking the first entry of the list instead of
> iterating and canceling.
>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
>
> Joel, I based this on top of the "Fixup SBEFIFO and OCC locking" series. Let me
> know if I need to put this first and rebase that series... thanks.
>
>   drivers/fsi/fsi-sbefifo.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
> index 4d024ed..7a6c4e1 100644
> --- a/drivers/fsi/fsi-sbefifo.c
> +++ b/drivers/fsi/fsi-sbefifo.c
> @@ -713,8 +713,10 @@ static ssize_t sbefifo_write_common(struct sbefifo_client *client,
>   	n = sbefifo_buf_nbwriteable(&client->wbuf);
>
>   	spin_lock_irqsave(&sbefifo->list_lock, flags);
> -	xfr = sbefifo_next_xfr(sbefifo);	/* next xfr to be executed */
>
> +	/* next xfr to be executed */
> +	xfr = list_first_entry_or_null(&sbefifo->xfrs, struct sbefifo_xfr,
> +				       xfrs);
>   	if ((client->f_flags & O_NONBLOCK) && xfr && n < len) {
>   		spin_unlock_irqrestore(&sbefifo->list_lock, flags);
>   		ret = -EAGAIN;

Patch

diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 4d024ed..7a6c4e1 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -713,8 +713,10 @@  static ssize_t sbefifo_write_common(struct sbefifo_client *client,
 	n = sbefifo_buf_nbwriteable(&client->wbuf);
 
 	spin_lock_irqsave(&sbefifo->list_lock, flags);
-	xfr = sbefifo_next_xfr(sbefifo);	/* next xfr to be executed */
 
+	/* next xfr to be executed */
+	xfr = list_first_entry_or_null(&sbefifo->xfrs, struct sbefifo_xfr,
+				       xfrs);
 	if ((client->f_flags & O_NONBLOCK) && xfr && n < len) {
 		spin_unlock_irqrestore(&sbefifo->list_lock, flags);
 		ret = -EAGAIN;