diff mbox series

[next-next,v3,05/10] net: cdc_ncm: convert tasklets to use new tasklet_setup() API

Message ID 20201006061159.292340-6-allen.lkml@gmail.com
State Superseded
Headers show
Series drivers: net: convert tasklets to use new | expand

Commit Message

Allen Pais Oct. 6, 2020, 6:11 a.m. UTC
From: Allen Pais <apais@linux.microsoft.com>

In preparation for unconditionally passing the
struct tasklet_struct pointer to all tasklet
callbacks, switch to using the new tasklet_setup()
and from_tasklet() to pass the tasklet pointer explicitly.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
Signed-off-by: Allen Pais <apais@linux.microsoft.com>
---
 drivers/net/usb/cdc_ncm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Jakub Kicinski Oct. 8, 2020, 11:58 p.m. UTC | #1
On Tue,  6 Oct 2020 11:41:54 +0530 Allen Pais wrote:
> From: Allen Pais <apais@linux.microsoft.com>
> 
> In preparation for unconditionally passing the
> struct tasklet_struct pointer to all tasklet
> callbacks, switch to using the new tasklet_setup()
> and from_tasklet() to pass the tasklet pointer explicitly.

> @@ -815,7 +815,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
>  
>  	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
> -	tasklet_init(&ctx->bh, cdc_ncm_txpath_bh, (unsigned long)dev);
> +	tasklet_setup(&ctx->bh, cdc_ncm_txpath_bh);
>  	atomic_set(&ctx->stop, 0);
>  	spin_lock_init(&ctx->mtx);
>  
> @@ -1468,9 +1468,9 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
>  	return HRTIMER_NORESTART;
>  }
>  
> -static void cdc_ncm_txpath_bh(unsigned long param)
> +static void cdc_ncm_txpath_bh(struct tasklet_struct *t)
>  {
> -	struct usbnet *dev = (struct usbnet *)param;
> +	struct usbnet *dev = from_tasklet(dev, t, bh);
>  	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
>  
>  	spin_lock_bh(&ctx->mtx);

This one is wrong.

ctx is struct cdc_ncm_ctx, but you from_tasklet() struct usbdev.
They both happen to have a tasklet called bh in 'em.
Allen Pais Nov. 3, 2020, 7:32 a.m. UTC | #2
> 
>> @@ -815,7 +815,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
>>   
>>   	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>   	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
>> -	tasklet_init(&ctx->bh, cdc_ncm_txpath_bh, (unsigned long)dev);
>> +	tasklet_setup(&ctx->bh, cdc_ncm_txpath_bh);
>>   	atomic_set(&ctx->stop, 0);
>>   	spin_lock_init(&ctx->mtx);
>>   
>> @@ -1468,9 +1468,9 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
>>   	return HRTIMER_NORESTART;
>>   }
>>   
>> -static void cdc_ncm_txpath_bh(unsigned long param)
>> +static void cdc_ncm_txpath_bh(struct tasklet_struct *t)
>>   {
>> -	struct usbnet *dev = (struct usbnet *)param;
>> +	struct usbnet *dev = from_tasklet(dev, t, bh);
>>   	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
>>   
>>   	spin_lock_bh(&ctx->mtx);
> 
> This one is wrong.
> 
> ctx is struct cdc_ncm_ctx, but you from_tasklet() struct usbdev.
> They both happen to have a tasklet called bh in 'em.

  from_tasklet() is struct usbnet. So it should pick the right bh isn't it?

Thanks.
Jakub Kicinski Nov. 3, 2020, 3:55 p.m. UTC | #3
On Tue, 3 Nov 2020 13:02:26 +0530 Allen Pais wrote:
> >   
> >> @@ -815,7 +815,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> >>   
> >>   	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> >>   	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
> >> -	tasklet_init(&ctx->bh, cdc_ncm_txpath_bh, (unsigned long)dev);
> >> +	tasklet_setup(&ctx->bh, cdc_ncm_txpath_bh);
> >>   	atomic_set(&ctx->stop, 0);
> >>   	spin_lock_init(&ctx->mtx);
> >>   
> >> @@ -1468,9 +1468,9 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
> >>   	return HRTIMER_NORESTART;
> >>   }
> >>   
> >> -static void cdc_ncm_txpath_bh(unsigned long param)
> >> +static void cdc_ncm_txpath_bh(struct tasklet_struct *t)
> >>   {
> >> -	struct usbnet *dev = (struct usbnet *)param;
> >> +	struct usbnet *dev = from_tasklet(dev, t, bh);
> >>   	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
> >>   
> >>   	spin_lock_bh(&ctx->mtx);  
> > 
> > This one is wrong.
> > 
> > ctx is struct cdc_ncm_ctx, but you from_tasklet() struct usbdev.
> > They both happen to have a tasklet called bh in 'em.  
> 
>   from_tasklet() is struct usbnet. So it should pick the right bh isn't it?

Look at the tasklet_init / tasklet_setup line.

It used to pass dev as an argument. But the tasklet is in ctx.

Now there is no explicit param, do you gotta pick out of ctx.
diff mbox series

Patch

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index e04f58853..57a95ef90 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -61,7 +61,7 @@  static bool prefer_mbim;
 module_param(prefer_mbim, bool, 0644);
 MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM setting on dual NCM/MBIM functions");
 
-static void cdc_ncm_txpath_bh(unsigned long param);
+static void cdc_ncm_txpath_bh(struct tasklet_struct *t);
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
 static struct usb_driver cdc_ncm_driver;
@@ -815,7 +815,7 @@  int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 
 	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
-	tasklet_init(&ctx->bh, cdc_ncm_txpath_bh, (unsigned long)dev);
+	tasklet_setup(&ctx->bh, cdc_ncm_txpath_bh);
 	atomic_set(&ctx->stop, 0);
 	spin_lock_init(&ctx->mtx);
 
@@ -1468,9 +1468,9 @@  static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
 	return HRTIMER_NORESTART;
 }
 
-static void cdc_ncm_txpath_bh(unsigned long param)
+static void cdc_ncm_txpath_bh(struct tasklet_struct *t)
 {
-	struct usbnet *dev = (struct usbnet *)param;
+	struct usbnet *dev = from_tasklet(dev, t, bh);
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 
 	spin_lock_bh(&ctx->mtx);