Re: [PATCH v3 1/3] nvme: double KA polling frequency to avoid KATO with TBKAS on

[Christoph Hellwig <hch@lst.de>]

On Thu, May 18, 2023 at 12:33:09PM -0600, Uday Shankar wrote:
> + * 
> + * When TBKAS is on, we need to run nvme_keep_alive_work at twice this
> + * frequency, as one command completion can postpone sending a keep alive
> + * command by up to twice the delay between runs.
>   */
>  static void nvme_queue_keep_alive_work(struct nvme_ctrl *ctrl)
>  {
> -	queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ / 2);
> +	unsigned long delay = (ctrl->ctratt & NVME_CTRL_ATTR_TBKAS) ?
> +		ctrl->kato * HZ / 4 : ctrl->kato * HZ / 2;
> +	queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);

Kernel coding style wants an empty line after the variable declarations.

I also find the style rather hard to read.  Why not:

	unsigned long delay = ctrl->kato * HZ / 2;

	/*
	 * When using Traffic Based Keep Alive, we need to run
	 * nvme_keep_alive_work at twice the normal frequency, as one command
	 * completion can postpone sending a keep alive command by up to
	 * twice the delay between runs.
	 */
	if (ctrl->ctratt & NVME_CTRL_ATTR_TBKAS)
		delay /= 2;

	queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);

That's not quite as dense, but relaly gets the point across to the
reader much better.