Re: [PATCH 4/4] block: update cached timestamp post schedule/preemption

[Johannes Thumshirn <Johannes.Thumshirn@xxxxxxx>]

On 29.01.24 15:02, Jens Axboe wrote:
> On 1/29/24 1:01 AM, Johannes Thumshirn wrote:
>> On 26.01.24 22:39, Jens Axboe wrote:
>>>    static void sched_update_worker(struct task_struct *tsk)
>>>    {
>>> -	if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
>>> +	if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER | PF_BLOCK_TS)) {
>>> +		if (tsk->flags & PF_BLOCK_TS)
>>> +			blk_plug_invalidate_ts(tsk);
>>>    		if (tsk->flags & PF_WQ_WORKER)
>>>    			wq_worker_running(tsk);
>>> -		else
>>> +		else if (tsk->flags & PF_IO_WORKER)
>>>    			io_wq_worker_running(tsk);
>>>    	}
>>>    }
>>
>>
>> Why the nested if? Isn't that more readable:
> 
> It's so that we can keep it at a single branch for the fast case of none
> of them being true, which is also how it was done before this change.
> This one just adds one more flag to check. With your change, it's 3
> branches instead of one for the fast case.
> 

Although I don't really have hard feelings for it, that could be solved 
as well:

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9116bcc90346..74beb0126da6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6787,12 +6787,12 @@ static inline void sched_submit_work(struct
task_struct *tsk)

   static void sched_update_worker(struct task_struct *tsk)
   {
-       if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
-               if (tsk->flags & PF_WQ_WORKER)
-                       wq_worker_running(tsk);
-               else
-                       io_wq_worker_running(tsk);
-       }
+	if (tsk->flags & !(PF_WQ_WORKER | PF_IO_WORKER | PF_BLOCK_TS))
+		return;
+       if (tsk->flags & PF_BLOCK_TS)
+               blk_plug_invalidate_ts(tsk);
+       if (tsk->flags & PF_WQ_WORKER)
+               wq_worker_running(tsk);
+       else if (tsk->flags & PF_IO_WORKER)
+               io_wq_worker_running(tsk);
   }

But yep, that's bikeshedding I admit