| Message ID | 20201006061159.292340-6-allen.lkml@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drivers: net: convert tasklets to use new | expand |
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.
> >> @@ -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.
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 --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);