On Wed, 2022-10-05 at 17:25 -0700, Harrison, John C wrote: > On 9/21/2022 10:32, Alan Previn wrote: > > @@ -208,6 +210,11 @@ struct intel_context { > > * each priority bucket > > */ > > u32 prio_count[GUC_CLIENT_PRIORITY_NUM]; > > + /** > > + * @sched_disable_delay: worker to disable scheduling on this > > + * context > > + */ > > + struct delayed_work sched_disable_delay; > Nit: this confuses me every time that it looks like the delay timeout > rather than the worker object. It would be much easier to read the code > if it was 'sched_disable_delay_work', although that is quite the > mouthful. Maybe just call all three variables disable_delay_XXX? I agree with you - I've modified this patch many times and realize I prefer something self-explanatory even if its a mouthful. Will stick with your first proposal "sched_disable_delay_work" > > > - if (unlikely(!context_enabled(ce) || submission_disabled(guc) || > > - context_has_committed_requests(ce))) { > This function no longer has any callers so can be removed completely. > Will do. > Otherwise, it looks good to me. > > John. > >