Re: [PATCH v3 2/3] nvme: check IO start time when deciding to defer KA

[Christoph Hellwig <hch@lst.de>]

On Thu, May 18, 2023 at 12:33:10PM -0600, Uday Shankar wrote:
> +	WRITE_ONCE(ctrl->ka_last_check_time, jiffies);
> +	smp_wmb();
>  	ctrl->comp_seen = false;
>  	spin_lock_irqsave(&ctrl->lock, flags);
>  	if (ctrl->state == NVME_CTRL_LIVE ||
> @@ -1206,6 +1215,9 @@ static void nvme_keep_alive_work(struct work_struct *work)
>  	bool comp_seen = ctrl->comp_seen;
>  	struct request *rq;
>  
> +	WRITE_ONCE(ctrl->ka_last_check_time, jiffies);
> +	smp_wmb();

WRITE_ONCE only works when paired with READ_ONCE on the read size,
which I don't ee here.

smp_wmb always pairs with smp_rmb, which I don't see here either.  We
also require documentation explaining the pairing because it tends to
be non-obvious (unlike READ_ONCE/WRITE_ONCE that are paired to a
variable).  I also can't really see a reason for the barrier here,
but the comment would be the thing to convince the reviewer.