Messages in this thread | | | From | Suren Baghdasaryan <> | Date | Thu, 23 Mar 2023 09:43:16 -0700 | Subject | Re: [PATCH v2 3/3] sched/psi: allow unprivileged polling of N*2s period |
| |
On Thu, Mar 23, 2023 at 8:09 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Thu, Mar 23, 2023 at 11:33:50AM +0100, Domenico Cerasuolo wrote: > > @@ -151,6 +151,14 @@ struct psi_trigger { > > > > /* Deferred event(s) from previous ratelimit window */ > > bool pending_event; > > + > > + /* Used to differentiate destruction action*/ > > + enum psi_aggregators aggregator; > > +}; > > + > > +struct trigger_info { > > + struct list_head triggers; > > + u32 nr_triggers[NR_PSI_STATES - 1]; > > }; > > > > struct psi_group { > > @@ -186,8 +194,7 @@ struct psi_group { > > struct mutex trigger_lock; > > > > /* Configured polling triggers */ > > - struct list_head triggers; > > - u32 nr_triggers[NR_PSI_STATES - 1]; > > + struct trigger_info trig_info[NR_PSI_AGGREGATORS]; > > u32 poll_states; > > u64 poll_min_period; > > Thanks for trying out this variant, but I think this is grouping up > unrelated things, and that makes the code more difficult to understand > and maintan. > > The *only* thing that's shared between those two is the > update_triggers() part. trig_info[PSI_AVGS] doesn't use trigger_lock. > It also doesn't use poll_task, poll_wait, poll_wakeup, poll_scheduled, > poll_min_period, polling_next_update and polling_until. All these > things are specific to the rt polling thread. > > The rename in the previous version is a bit churny, but it's justified > in order to keep unrelated things separate / make it obvious which > parts belong together, and who is reading and writing which fields. > > So my vote would be on the previous version.
Hmm. Ok, but then I would suggest keeping RT trigger naming as is and calling the new triggers based on averages as avg_triggers/avg_nr_triggers/etc. This would limit the churn and since we already have polling_total and avg_total, this naming would be appropriate IMO. If we want to be even stricter, we could rename the polling variables to poll_triggers/poll_nr_triggers/etc. Some more churn but then the names are very distinct.
| |