On 4/7/20 5:47 AM, Peter Zijlstra wrote:
>
> You seem to have lost Oleg and Al from the Cc list..
I'll add them for v2, I did point Oleg at it!
> On Mon, Apr 06, 2020 at 01:48:51PM -0600, Jens Axboe wrote:
>> If task_work has already been run on task exit, we don't always know
>> if it's safe to run again. Check for task_work_exited in the
>> task_work_pending() helper. This makes it less fragile in calling
>> from the exit files path, for example.
>>
>> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
>> ---
>> include/linux/task_work.h | 4 +++-
>> kernel/task_work.c | 8 ++++----
>> 2 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/task_work.h b/include/linux/task_work.h
>> index 54c911bbf754..24f977a8fc35 100644
>> --- a/include/linux/task_work.h
>> +++ b/include/linux/task_work.h
>> @@ -7,6 +7,8 @@
>>
>> typedef void (*task_work_func_t)(struct callback_head *);
>>
>> +extern struct callback_head task_work_exited;
>> +
>> static inline void
>> init_task_work(struct callback_head *twork, task_work_func_t func)
>> {
>> @@ -19,7 +21,7 @@ void __task_work_run(void);
>>
>> static inline bool task_work_pending(void)
>> {
>> - return current->task_works;
>> + return current->task_works && current->task_works != &task_work_exited;
>> }
>
> Hurmph.. not sure I like this. It inlines that second condition to
> every caller of task_work_run() even though for pretty much all of them
> this is impossible.
Oleg had the same concern, and I agree with both of you. Would you
prefer it we just leave task_work_run() as:
static inline void task_work_run(void)
{
if (current->task_works)
__task_work_run();
}
and then have the io_uring caller do:
if (current->task_works != &task_work_exited)
task_work_run();
instead?
--
Jens Axboe
