On 6/30/21 10:56 PM, Jens Axboe wrote: > On 6/30/21 3:45 PM, Jens Axboe wrote: >> 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... > > BTW, in usual testing, even just the one hunk removing the exit check > seems to result in quite a lot of memory leaks running > test/poll-mshot-update. So something is funky with the patch. Oh, have no idea why yet. Apparently, I commented out the test as it was always failing, and forgot to return it back... Will take a note -- Pavel Begunkov
