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

[Uday Shankar <ushankar@purestorage.com>]

On Sat, May 20, 2023 at 06:34:22AM +0200, Christoph Hellwig wrote:
> I also can't really see a reason for the barrier here

In this version of the patch, I have this bit in nvme_complete_rq:

+	if (ctrl->kas && !ctrl->comp_seen &&
+	    nvme_req(req)->start_time >= ctrl->ka_last_check_time)

and the code that updates ka_last_check_time looks like this:

+	WRITE_ONCE(ctrl->ka_last_check_time, jiffies);
+	smp_wmb();
 	ctrl->comp_seen = false;

I used WRITE_ONCE and smp_wmb here to try to avoid reordering of writes
here, as I didn't want nvme_complete_rq to be able to observe comp_seen
as false while ka_last_check_time still has its pre-update value. I
agree if I wanted to do this properly, I need to add a smb_rmb and
READ_ONCE(ka_last_check_time) in nvme_complete_rq.

However, the read of comp_seen here is a (probably premature) attempt at
optimization, which may well be completely unnoticeable. I removed it in
v4, and as a result the READ/WRITE_ONCE and barriers are no longer
needed.