Re: [PATCH v3] block: annotate lockless task ioprio accesses

[Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>]

Hi Damien,

Title: agreed, I'll change it (v4 below).

On the pairing: a release store pairs with any load that is ordered after
the pointer load, and here the ordering comes from the address dependency,
not from an acquire.  The only thing the reader does with the loaded
pointer is dereference it (ioc->ioprio), and the kernel memory model orders
a dependent load after the READ_ONCE() that produced its address.  This is
the same pairing as rcu_assign_pointer()/rcu_dereference() (see "ADDRESS
DEPENDENCY" in Documentation/memory-barriers.txt); READ_ONCE() also carries
the extra barrier Alpha needs (arch/alpha/include/asm/rwonce.h).
smp_load_acquire() would only be needed if the reader looked at data not
dependent on the pointer, and would add a barrier on arm64/powerpc for
nothing.

Why not plain WRITE_ONCE(): it would silence KCSAN, but it does not order
the initialization of the new io_context (alloc_io_context() runs before
task_lock(), and spin_lock() is only an acquire, so those stores may be
reordered after the pointer store on weakly ordered CPUs). A reader could
then see the pointer without seeing the initialized ioc. The release is
what makes the publication safe.

If you'd rather keep this minimal, I can drop the release and use
WRITE_ONCE(), but I think the release is the correct thing to have.

v4 follows with the new title and a comment/commit-message paragraph
explaining the above.

Thanks,
Nguyen Ngoc Thang