Linux USB
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Mikhail Zubenko <misha.zubenko.01@yandex.ru>
Cc: linux-usb@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v5] usb: gadget: dummy_hcd: add tick_us module parameter for timer-rate tuning
Date: Mon, 24 Aug 2026 21:42:49 -0400	[thread overview]
Message-ID: <24ec79bf-6b2c-4fc9-99a1-022ba19b8dcb@rowland.harvard.edu> (raw)
In-Reply-To: <178758686883.288478.6637939361965647417@yandex.ru>

On Mon, Aug 24, 2026 at 03:54:28PM -0000, Mikhail Zubenko wrote:
> dummy_hcd services URB completions from an hrtimer callback that
> rearms itself with a fixed 125 us delay (one high-speed microframe)
> for as long as any URBs are queued.  Against a permanently polled HID
> interrupt endpoint the queue never drains, so the timer runs at ~8 kHz
> regardless of actual traffic.
> 
> On timer-rate-sensitive platforms (observed on one consumer AM4 board
> with a current BIOS and a stock kernel) this timer rate co-existing
> with a live Bluetooth session on the machine's real xHCI controller
> resulted in a hard, below-software platform hang: printk, SysRq, NMI
> and both watchdogs died in the same instant, with no panic, no MCE,
> and empty pstore.  13 occurrences over 5 days, reliably reproducible
> with that configuration (hangs within 20 min to 2.5 h of uptime).
> The same workload with the emulated microframe lengthened to 1000 us
> (this parameter) ran ~8 hours with zero hangs, and USB-cable sessions
> (identical gadget traffic, no Bluetooth co-existence) never hung.
> 
> Note that lengthening the emulated microframe makes the emulation run
> slower than a physical device would; users of the parameter trade
> that emulation fidelity for interrupt-rate headroom.
> 
> The per-frame bandwidth budget in dummy_timer() does not scale with
> the emulated microframe length; making that accounting microframe-
> correct is planned as a follow-up patch.  Otherwise nothing in the
> driver's behaviour depends on the emulated microframe length.
> 
> Default keeps the current 125 us behaviour bit-for-bit (and defaults
> to 1000 us for full-speed emulation, where the natural unit is the
> 1-ms frame).  Values below 125 are rejected.
> 
> Signed-off-by: Mikhail Zubenko <misha.zubenko.01@yandex.ru>
> ---
> Changes in v5:
> - remove a leftover blank line where the macro was (review)
> - spell "(micro)frame" in the parameter description (review)
> 
> Changes in v2 (addressing Alan Stern's review):
> - reword the description: the timer rearms while URBs are queued, not
>   an unconditional "pendulum"; drop the word entirely
> - clarify the xHCI wording: the Bluetooth session lives on the
>   machine's real xHCI controller (platform-level co-existence)
> - move tick_us into dummy_hcd_module_parameters with a custom setter
>   (module_param_cb) that rejects values < 125
> - speed-aware default: 125 us for high/super-speed, 1000 us for
>   full-speed emulation
> - reword MODULE_PARM_DESC per review ("length in microseconds of an
>   emulated microframe")
> - pre-compute the delay into a ktime_t (timer_interval) at setup;
>   remove the DUMMY_TIMER_INT_NSECS macro; explicit (u64) cast so the
>   tick_us * NSEC_PER_USEC product does not overflow on 32-bit
> - parameter is 0444 (load-time only): a runtime write would not
>   affect live controllers
> - note the emulation-fidelity tradeoff and the per-frame budget
>   follow-up in the commit message

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

>  drivers/usb/gadget/udc/dummy_hcd.c | 48 +++++++++++++++++++++++++++---
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index c0e40fa..3edc4fc 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -51,8 +51,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";
>  
> @@ -66,6 +64,31 @@ struct dummy_hcd_module_parameters {
>  	bool is_super_speed;
>  	bool is_high_speed;
>  	unsigned int num;
> +	unsigned int tick_us;
> +};
> +
> +/*
> + * Length in microseconds of an emulated microframe.  The default (125,
> + * one high-speed microframe) preserves the historical timer rate; larger
> + * values slow the emulated controller down and reduce its interrupt-rate
> + * pressure on the host platform.  Values below 125 are rejected.
> + */
> +static int tick_us_set(const char *val,
> +		const struct kernel_param *kp)
> +{
> +	unsigned int v;
> +	int ret = kstrtouint(val, 0, &v);
> +
> +	if (ret < 0 || v < 125) {
> +		pr_err("dummy_hcd: tick_us must be >= 125\n");
> +		return -EINVAL;
> +	}
> +	return param_set_uint(val, kp);
> +}
> +
> +static const struct kernel_param_ops tick_us_ops = {
> +	.set = tick_us_set,
> +	.get = param_get_uint,
>  };
>  
>  static struct dummy_hcd_module_parameters mod_data = {
> @@ -79,6 +102,9 @@ module_param_named(is_high_speed, mod_data.is_high_speed, bool, S_IRUGO);
>  MODULE_PARM_DESC(is_high_speed, "true to simulate HighSpeed connection");
>  module_param_named(num, mod_data.num, uint, S_IRUGO);
>  MODULE_PARM_DESC(num, "number of emulated controllers");
> +module_param_cb(tick_us, &tick_us_ops, &mod_data.tick_us, 0444);
> +MODULE_PARM_DESC(tick_us,
> +		"Length in microseconds of an emulated (micro)frame (default 125 for high/super-speed emulation, 1000 for full-speed); larger values reduce the interrupt-rate pressure on the host platform");
>  /*-------------------------------------------------------------------------*/
>  
>  /* gadget side driver data structures */
> @@ -244,6 +270,7 @@ struct dummy_hcd {
>  	struct dummy			*dum;
>  	enum dummy_rh_state		rh_state;
>  	struct hrtimer			timer;
> +	ktime_t				timer_interval;	/* emulated microframe length */
>  	u32				port_status;
>  	u32				old_status;
>  	unsigned long			re_timeout;
> @@ -1329,7 +1356,7 @@ static int dummy_urb_enqueue(
>  	/* kick the scheduler, it'll do the rest */
>  	if (!dum_hcd->timer_pending) {
>  		dum_hcd->timer_pending = 1;
> -		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS),
> +		hrtimer_start(&dum_hcd->timer, dum_hcd->timer_interval,
>  				HRTIMER_MODE_REL_SOFT);
>  	}
>  
> @@ -2029,7 +2056,7 @@ return_urb:
>  			dum_hcd->rh_state == DUMMY_RH_RUNNING) {
>  		/* want a 1 msec delay here */
>  		dum_hcd->timer_pending = 1;
> -		hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS),
> +		hrtimer_start(&dum_hcd->timer, dum_hcd->timer_interval,
>  				HRTIMER_MODE_REL_SOFT);
>  	}
>  
> @@ -2509,6 +2536,7 @@ static DEVICE_ATTR_RO(urbs);
>  static int dummy_start_ss(struct dummy_hcd *dum_hcd)
>  {
>  	hrtimer_setup(&dum_hcd->timer, dummy_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
> +	dum_hcd->timer_interval = ns_to_ktime((u64)mod_data.tick_us * NSEC_PER_USEC);
>  	dum_hcd->rh_state = DUMMY_RH_RUNNING;
>  	dum_hcd->stream_en_ep = 0;
>  	INIT_LIST_HEAD(&dum_hcd->urbp_list);
> @@ -2538,6 +2566,7 @@ static int dummy_start(struct usb_hcd *hcd)
>  
>  	spin_lock_init(&dum_hcd->dum->lock);
>  	hrtimer_setup(&dum_hcd->timer, dummy_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
> +	dum_hcd->timer_interval = ns_to_ktime((u64)mod_data.tick_us * NSEC_PER_USEC);
>  	dum_hcd->rh_state = DUMMY_RH_RUNNING;
>  
>  	INIT_LIST_HEAD(&dum_hcd->urbp_list);
> @@ -2828,6 +2857,17 @@ static int __init dummy_hcd_init(void)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * The emulated microframe length defaults to one high-speed
> +	 * microframe (125 us).  When neither is_high_speed nor
> +	 * is_super_speed is set the emulation is full-speed, where the
> +	 * natural scheduling unit is the 1-ms frame, so default to
> +	 * 1000 us instead.
> +	 */
> +	if (!mod_data.tick_us)
> +		mod_data.tick_us = (mod_data.is_super_speed ||
> +				    mod_data.is_high_speed) ? 125 : 1000;
> +
>  	for (i = 0; i < mod_data.num; i++) {
>  		the_hcd_pdev[i] = platform_device_alloc(driver_name, i);
>  		if (!the_hcd_pdev[i]) {
> -- 
> 2.43.0
> 
> 

      reply	other threads:[~2026-08-25  1:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-23 17:15 [PATCH] usb: gadget: dummy_hcd: add tick_us module parameter for Mikhail Zubenko
2026-08-23 17:25 ` Greg Kroah-Hartman
2026-08-23 19:55 ` Alan Stern
2026-08-24  8:29 ` [PATCH v2] usb: gadget: dummy_hcd: add tick_us module parameter for timer-rate tuning Mikhail Zubenko
2026-08-24  8:29 ` Mikhail Zubenko
2026-08-24  8:43   ` Greg Kroah-Hartman
2026-08-24  9:12 ` [PATCH v3] " Mikhail Zubenko
2026-08-24  9:40   ` Greg Kroah-Hartman
2026-08-24 10:42 ` Mikhail Zubenko
2026-08-24 10:42 ` [PATCH v4] " Mikhail Zubenko
2026-08-24 14:27   ` Alan Stern
2026-08-24 15:54 ` [PATCH v5] " Mikhail Zubenko
2026-08-25  1:42   ` Alan Stern [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24ec79bf-6b2c-4fc9-99a1-022ba19b8dcb@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=misha.zubenko.01@yandex.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox