[PATCH v1 0/1] usb: gadget: dummy_hcd: Fix regression due to hrtimer migration

[PATCH v1 0/1] usb: gadget: dummy_hcd: Fix regression due to hrtimer migration

[Marcello Sylvester Bauer]

The kernel CI bots such as syzbot and intel kernel bot reported a
regression due to the migration of the transfare scheduler from timer
list to hrtimer. The current assumption is that this is because timer
list uses soft interrupt context. I have not been able to reproduce the
regression consistently. So I'm submitting this patch in the hope that
it solves the issue.

Do not apply the patch if any bot still reports the problem.

Marcello Sylvester Bauer (1):
  usb: gadget: dummy_hcd: make hrtimer expire in soft irq context

 drivers/usb/gadget/udc/dummy_hcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.45.2

[PATCH v1 1/1] usb: gadget: dummy_hcd: make hrtimer expire in soft irq context

[Marcello Sylvester Bauer]

Since migrating to hrtime, some kernel test bots are reporting
regressions like rcu stalls and inconsistent_lock_state. the listed
cause is around usb_hcd_giveback_urb() and it's locking mechanism, which
was not changed by the patch.

The main difference between the timer list timer and the current hrtimer
integration is that the timer list timer callback runs in soft interrupt
context.

This patch changes the hrtimer in the transfer scheduler to also use soft
interrupt context.

Reported-by: syzbot+c793a7eca38803212c61@syzkaller.appspotmail.com
Reported-by: syzbot+1e6e0b916b211bee1bd6@syzkaller.appspotmail.com
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202406141323.413a90d2-lkp@intel.com
Fixes: a7f3813e589f ("usb: gadget: dummy_hcd: Switch to hrtimer transfer scheduler")
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index f37b0d8386c1..cd25538e999e 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1304,7 +1304,7 @@ static int dummy_urb_enqueue(
 
 	/* kick the scheduler, it'll do the rest */
 	if (!hrtimer_active(&dum_hcd->timer))
-		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL);
+		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL_SOFT);
 
  done:
 	spin_unlock_irqrestore(&dum_hcd->dum->lock, flags);
@@ -1325,7 +1325,7 @@ static int dummy_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 	rc = usb_hcd_check_unlink_urb(hcd, urb, status);
 	if (!rc && dum_hcd->rh_state != DUMMY_RH_RUNNING &&
 			!list_empty(&dum_hcd->urbp_list))
-		hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL);
+		hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL_SOFT);
 
 	spin_unlock_irqrestore(&dum_hcd->dum->lock, flags);
 	return rc;
@@ -1995,7 +1995,7 @@ static enum hrtimer_restart dummy_timer(struct hrtimer *t)
 		dum_hcd->udev = NULL;
 	} else if (dum_hcd->rh_state == DUMMY_RH_RUNNING) {
 		/* want a 1 msec delay here */
-		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL);
+		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL_SOFT);
 	}
 
 	spin_unlock_irqrestore(&dum->lock, flags);
@@ -2389,7 +2389,7 @@ static int dummy_bus_resume(struct usb_hcd *hcd)
 		dum_hcd->rh_state = DUMMY_RH_RUNNING;
 		set_link_state(dum_hcd);
 		if (!list_empty(&dum_hcd->urbp_list))
-			hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL);
+			hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL_SOFT);
 		hcd->state = HC_STATE_RUNNING;
 	}
 	spin_unlock_irq(&dum_hcd->dum->lock);
@@ -2467,7 +2467,7 @@ static DEVICE_ATTR_RO(urbs);
 
 static int dummy_start_ss(struct dummy_hcd *dum_hcd)
 {
-	hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	dum_hcd->timer.function = dummy_timer;
 	dum_hcd->rh_state = DUMMY_RH_RUNNING;
 	dum_hcd->stream_en_ep = 0;
@@ -2497,7 +2497,7 @@ static int dummy_start(struct usb_hcd *hcd)
 		return dummy_start_ss(dum_hcd);
 
 	spin_lock_init(&dum_hcd->dum->lock);
-	hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	dum_hcd->timer.function = dummy_timer;
 	dum_hcd->rh_state = DUMMY_RH_RUNNING;
 
-- 
2.45.2

Re: [PATCH v1 0/1] usb: gadget: dummy_hcd: Fix regression due to hrtimer migration

[Alan Stern]

On Wed, Jun 26, 2024 at 03:47:19PM +0200, Marcello Sylvester Bauer wrote:
> The kernel CI bots such as syzbot and intel kernel bot reported a
> regression due to the migration of the transfare scheduler from timer
> list to hrtimer. The current assumption is that this is because timer
> list uses soft interrupt context. I have not been able to reproduce the
> regression consistently. So I'm submitting this patch in the hope that
> it solves the issue.
> 
> Do not apply the patch if any bot still reports the problem.

You should send the patch to syzbot and have it run its test.  Then 
you'll know whether the patch works.

Alan Stern

> Marcello Sylvester Bauer (1):
>   usb: gadget: dummy_hcd: make hrtimer expire in soft irq context
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> -- 
> 2.45.2
> 
>