diff mbox

caif: Fix napi poll list corruption

Message ID 20141222093525.GA18616@gondor.apana.org.au
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Herbert Xu Dec. 22, 2014, 9:35 a.m. UTC
On Mon, Dec 22, 2014 at 04:18:33PM +0800, Jason Wang wrote:
>
> btw, looks like at least caif_virtio has the same issue.

Good catch.

-- >8 --
The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
interrupt masking in NAPI) breaks caif.

It is now required that if the entire budget is consumed when poll
returns, the napi poll_list must remain empty.  However, like some
other drivers caif tries to do a last-ditch check and if there is
more work it will call napi_schedule and then immediately process
some of this new work.  Should the entire budget be consumed while
processing such new work then we will violate the new caller
contract.

This patch fixes this by not touching any work when we reschedule
in caif.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,

Comments

Jason Wang Dec. 22, 2014, 10:02 a.m. UTC | #1
On Mon, Dec 22, 2014 at 5:35 PM, Herbert Xu 
<herbert@gondor.apana.org.au> wrote:
> On Mon, Dec 22, 2014 at 04:18:33PM +0800, Jason Wang wrote:
>> 
>>  btw, looks like at least caif_virtio has the same issue.
> 
> Good catch.
> 
> -- >8 --
> The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
> interrupt masking in NAPI) breaks caif.
> 
> It is now required that if the entire budget is consumed when poll
> returns, the napi poll_list must remain empty.  However, like some
> other drivers caif tries to do a last-ditch check and if there is
> more work it will call napi_schedule and then immediately process
> some of this new work.  Should the entire budget be consumed while
> processing such new work then we will violate the new caller
> contract.
> 
> This patch fixes this by not touching any work when we reschedule
> in caif.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks.
> 
> 
> diff --git a/drivers/net/caif/caif_virtio.c 
> b/drivers/net/caif/caif_virtio.c
> index a5fefb9..b306210 100644
> --- a/drivers/net/caif/caif_virtio.c
> +++ b/drivers/net/caif/caif_virtio.c
> @@ -257,7 +257,6 @@ static int cfv_rx_poll(struct napi_struct *napi, 
> int quota)
>  	struct vringh_kiov *riov = &cfv->ctx.riov;
>  	unsigned int skb_len;
>  
> -again:
>  	do {
>  		skb = NULL;
>  
> @@ -322,7 +321,6 @@ exit:
>  		    napi_schedule_prep(napi)) {
>  			vringh_notify_disable_kern(cfv->vr_rx);
>  			__napi_schedule(napi);
> -			goto again;
>  		}
>  		break;
> 
> Thanks,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Dec. 22, 2014, 9:35 p.m. UTC | #2
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 22 Dec 2014 20:35:25 +1100

> The commit d75b1ade567ffab085e8adbbdacf0092d10cd09c (net: less
> interrupt masking in NAPI) breaks caif.
> 
> It is now required that if the entire budget is consumed when poll
> returns, the napi poll_list must remain empty.  However, like some
> other drivers caif tries to do a last-ditch check and if there is
> more work it will call napi_schedule and then immediately process
> some of this new work.  Should the entire budget be consumed while
> processing such new work then we will violate the new caller
> contract.
> 
> This patch fixes this by not touching any work when we reschedule
> in caif.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index a5fefb9..b306210 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -257,7 +257,6 @@  static int cfv_rx_poll(struct napi_struct *napi, int quota)
 	struct vringh_kiov *riov = &cfv->ctx.riov;
 	unsigned int skb_len;
 
-again:
 	do {
 		skb = NULL;
 
@@ -322,7 +321,6 @@  exit:
 		    napi_schedule_prep(napi)) {
 			vringh_notify_disable_kern(cfv->vr_rx);
 			__napi_schedule(napi);
-			goto again;
 		}
 		break;