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

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

Hi Damien,

Thanks for looking. I don't think RCU is needed here, because the reader
can only be the owner of the io_context:

 - The only path that drops the task's reference is exit_io_context(),
   which the task runs on itself in do_exit() (and on the copy_process()
   error path, where the task never ran). It clears task->io_context
   under task_lock() and only then puts the ioc.
 - So when __get_task_ioprio() runs lockless, p == current, and current
   cannot be inside exit_io_context() at the same time. Once the pointer
   is non-NULL it stays valid for as long as current is executing; the
   ioc cannot be freed under it.
 - For p != current the function already requires p->alloc_lock
   (lockdep_assert_held()), and that is what serializes against
   exit_io_context().

The only thing another task can change under the lockless reader is the
NULL -> ioc transition and ->ioprio, both done by set_task_ioprio() under
task_lock(), and that is exactly what the KCSAN report shows
("value changed: 0x0 -> 0xffff..."). The patch only needs to make that
publication and the ->ioprio accesses well defined, not to extend the
lifetime of the ioc.

I can add a sentence to the commit message and a short comment in
__get_task_ioprio() stating this lifetime rule in a v2 if you think it
helps.

Thanks,
Nguyen Ngoc Thang