Re: [syzbot] [usb?] INFO: task hung in usb_port_suspend

Alan Stern
2024-10-11T14:55:26Z

On Fri, Oct 11, 2024 at 07:35:02AM -0700, syzbot wrote:
> Hello,
>
> syzbot tried to test the proposed patch but the build/boot failed:

...

> Tested on:
>
> commit: 920e7522 usb: gadget: function: Remove usage of the de..

All right, let's try again with an explicit patch to undo the timer
changes in dummy_hcd.c.

Alan Stern

#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing

Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
@@ -30,7 +30,7 @@
#include <linux/slab.h>
#include <linux/errno.h>
#include <linux/init.h>
-#include <linux/hrtimer.h>
+#include <linux/timer.h>
#include <linux/list.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
@@ -50,8 +50,6 @@
#define POWER_BUDGET 500 /* in mA; use 8 for low-power port testing */
#define POWER_BUDGET_3 900 /* in mA */

-#define DUMMY_TIMER_INT_NSECS 125000 /* 1 microframe */
-
static const char driver_name[] = "dummy_hcd";
static const char driver_desc[] = "USB Host+Gadget Emulator";

@@ -242,7 +240,7 @@ enum dummy_rh_state {
struct dummy_hcd {
struct dummy *dum;
enum dummy_rh_state rh_state;
- struct hrtimer timer;
+ struct timer_list timer;
u32 port_status;
u32 old_status;
unsigned long re_timeout;
@@ -1303,8 +1301,8 @@ static int dummy_urb_enqueue(
urb->error_count = 1; /* mark as a new urb */

/* 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);
+ if (!timer_pending(&dum_hcd->timer))
+ mod_timer(&dum_hcd->timer, jiffies + 1);

done:
spin_unlock_irqrestore(&dum_hcd->dum->lock, flags);
@@ -1325,7 +1323,7 @@ static int dummy_urb_dequeue(struct usb_
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);
+ mod_timer(&dum_hcd->timer, jiffies);

spin_unlock_irqrestore(&dum_hcd->dum->lock, flags);
return rc;
@@ -1779,7 +1777,7 @@ static int handle_control_request(struct
* drivers except that the callbacks are invoked from soft interrupt
* context.
*/
-static enum hrtimer_restart dummy_timer(struct hrtimer *t)
+static void dummy_timer(struct timer_list *t)
{
struct dummy_hcd *dum_hcd = from_timer(dum_hcd, t, timer);
struct dummy *dum = dum_hcd->dum;
@@ -1810,6 +1808,8 @@ static enum hrtimer_restart dummy_timer(
break;
}

+ /* FIXME if HZ != 1000 this will probably misbehave ... */
+
/* look at each urb queued by the host side driver */
spin_lock_irqsave(&dum->lock, flags);

@@ -1817,7 +1817,7 @@ static enum hrtimer_restart dummy_timer(
dev_err(dummy_dev(dum_hcd),
"timer fired with no URBs pending?\n");
spin_unlock_irqrestore(&dum->lock, flags);
- return HRTIMER_NORESTART;
+ return;
}
dum_hcd->next_frame_urbp = NULL;

@@ -1995,12 +1995,10 @@ return_urb:
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);
+ mod_timer(&dum_hcd->timer, jiffies + msecs_to_jiffies(1));
}

spin_unlock_irqrestore(&dum->lock, flags);
-
- return HRTIMER_NORESTART;
}

/*-------------------------------------------------------------------------*/
@@ -2389,7 +2387,7 @@ static int dummy_bus_resume(struct usb_h
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);
+ mod_timer(&dum_hcd->timer, jiffies);
hcd->state = HC_STATE_RUNNING;
}
spin_unlock_irq(&dum_hcd->dum->lock);
@@ -2467,8 +2465,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);
- dum_hcd->timer.function = dummy_timer;
+ timer_setup(&dum_hcd->timer, dummy_timer, 0);
dum_hcd->rh_state = DUMMY_RH_RUNNING;
dum_hcd->stream_en_ep = 0;
INIT_LIST_HEAD(&dum_hcd->urbp_list);
@@ -2497,8 +2494,7 @@ static int dummy_start(struct usb_hcd *h
return dummy_start_ss(dum_hcd);

spin_lock_init(&dum_hcd->dum->lock);
- hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- dum_hcd->timer.function = dummy_timer;
+ timer_setup(&dum_hcd->timer, dummy_timer, 0);
dum_hcd->rh_state = DUMMY_RH_RUNNING;

INIT_LIST_HEAD(&dum_hcd->urbp_list);
@@ -2517,11 +2513,8 @@ static int dummy_start(struct usb_hcd *h

static void dummy_stop(struct usb_hcd *hcd)
{
- struct dummy_hcd *dum_hcd = hcd_to_dummy_hcd(hcd);
-
- hrtimer_cancel(&dum_hcd->timer);
- device_remove_file(dummy_dev(dum_hcd), &dev_attr_urbs);
- dev_info(dummy_dev(dum_hcd), "stopped\n");
+ device_remove_file(dummy_dev(hcd_to_dummy_hcd(hcd)), &dev_attr_urbs);
+ dev_info(dummy_dev(hcd_to_dummy_hcd(hcd)), "stopped\n");
}

/*-------------------------------------------------------------------------*/