Re: [PATCH rtw-next 1/2] wifi: rtw88: usb: bound what the driver feeds the after-DTIM queue

[Mehmet Fide]

Hi Ping-Ke,

thanks for the review.

> nit: a blank line

Will add both.

> The pattern ' HZ / RTW_USB_HIQ_RATE' repeat many times. Can you just define
> the it as a part of macro? (The macro name should change as well.)

Yes. v2 will have

	#define RTW_USB_HIQ_REFILL_INTERVAL	(HZ / 10)	/* jiffies per unit of budget */
	#define RTW_USB_HIQ_BUDGET_MAX		16

and use RTW_USB_HIQ_REFILL_INTERVAL everywhere.

> Will 'quota' or 'budget' be clearer than 'token'?

Budget it is: hiq_budget, hiq_budget_refill, rtw_usb_hiq_take_budget().

> It looks like you shift refill jiffies according to the tokens you are adding.
> How can I understand '+= add * (HZ / RTW_USB_HIQ_RATE)'?

hiq_refill is the point in time up to which budget has already been granted.
When 'add' whole intervals have passed I move it forward by exactly those
intervals, not to 'jiffies', so the part of the current interval that has
not completed yet keeps counting toward the next unit instead of being
thrown away (the usual token bucket bookkeeping). When more than a full
burst has elapsed the exact position no longer matters, which is why that
branch simply resets it to jiffies.

I agree it reads badly. v2 will say the same thing as

	elapsed = jiffies - rtwusb->hiq_refill;
	add = elapsed / RTW_USB_HIQ_REFILL_INTERVAL;
	if (add) {
		rtwusb->hiq_budget = min_t(u32, rtwusb->hiq_budget + add,
					   RTW_USB_HIQ_BUDGET_MAX);
		rtwusb->hiq_refill = jiffies - elapsed % RTW_USB_HIQ_REFILL_INTERVAL;
	}

with a comment on the remainder, so the intent is visible without the
multiplication.

I will send v2 early next week together with the changes to 2/2.

Mehmet