On 6/30/21 3:38 PM, Pavel Begunkov wrote: > On 6/30/21 10:22 PM, Jens Axboe wrote: >> On 6/30/21 3:19 PM, Pavel Begunkov wrote: >>> On 6/30/21 10:17 PM, Jens Axboe wrote: >>>> On 6/30/21 2:54 PM, Pavel Begunkov wrote: >>>>> Whenever possible we don't want to fallback a request. task_work_add() >>>>> will be fine if the task is exiting, so don't check for PF_EXITING, >>>>> there is anyway only a relatively small gap between setting the flag >>>>> and doing the final task_work_run(). >>>>> >>>>> Also add likely for the hot path. >>>> >>>> I'm not a huge fan of likely/unlikely, and in particular constructs like: >>>> >>>>> - if (test_bit(0, &tctx->task_state) || >>>>> + if (likely(test_bit(0, &tctx->task_state)) || >>>>> test_and_set_bit(0, &tctx->task_state)) >>>>> return 0; >>>> >>>> where the state is combined. In any case, it should be a separate >>>> change. If there's an "Also" paragraph in a patch, then that's also >>>> usually a good clue that that particular change should've been >>>> separate :-) >>> >>> Not sure what's wrong with likely above, but how about drop >>> this one then? >> >> Yep I did - we can do the exiting change separately, the commit message > > I think 1-2 is good enough for 5.14, I'll just send it for-next > >> just needs to be clarified a bit on why it's ok to do now. And that > > It should have been ok to do before those 2 patches, but > haven't tracked where it lost actuality. Right, I was thinking it was related to the swapping of the signal exit and task work run ordering. But didn't look that far yet... -- Jens Axboe
