Thanks for review Peter! On Tue, Jan 29, 2019 at 2:44 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote: > > static void psi_update_work(struct work_struct *work) > > { > > struct delayed_work *dwork; > > struct psi_group *group; > > + bool first_pass = true; > > + u64 next_update; > > + u32 change_mask; > > + int polling; > > bool nonidle; > > + u64 now; > > > > dwork = to_delayed_work(work); > > group = container_of(dwork, struct psi_group, clock_work); > > > > + now = sched_clock(); > > + > > + mutex_lock(&group->update_lock); > > actually acquiring a mutex can take a fairly long while; would it not > make more sense to take the @now timestanp _after_ it, instead of > before? Yes, that makes sense. As long as *now* is set before the *retry* label, otherwise the retry mechanism would get even more complicated to understand with floating *now* timestamp. Will move the assignment right after the mutex_lock() > -- > You received this message because you are subscribed to the Google Groups "kernel-team" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@xxxxxxxxxxx. >