| Message ID | 20230613042422.5344-19-ricardo.neri-calderon@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | sched: Introduce classes of tasks for load balance | expand |
Hi, On Monday 12 Jun 2023 at 21:24:16 (-0700), Ricardo Neri wrote: > The raw classification that hardware provides for a task may not > be directly usable by the scheduler: the classification may change too > frequently or architecture-specific implementations may need to consider > additional factors. For instance, some processors with Intel Thread > Director need to consider the state of the SMT siblings of a core. > > Provide per-task helper variables that architectures can use to > postprocess the classification that hardware provides. > > Cc: Ben Segall <bsegall@google.com> > Cc: Daniel Bristot de Oliveira <bristot@redhat.com> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > Cc: Ionela Voinescu <ionela.voinescu@arm.com> > Cc: Joel Fernandes (Google) <joel@joelfernandes.org> > Cc: Len Brown <len.brown@intel.com> > Cc: Lukasz Luba <lukasz.luba@arm.com> > Cc: Mel Gorman <mgorman@suse.de> > Cc: Perry Yuan <Perry.Yuan@amd.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Tim C. Chen <tim.c.chen@intel.com> > Cc: Valentin Schneider <vschneid@redhat.com> > Cc: Zhao Liu <zhao1.liu@linux.intel.com> > Cc: x86@kernel.org > Cc: linux-pm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > --- > Changes since v3: > * None > > Changes since v2: > * None > > Changes since v1: > * Used bit-fields to fit all the IPC class data in 4 bytes. (PeterZ) > * Shortened names of the helpers. > * Renamed helpers with the ipcc_ prefix. > * Reworded commit message for clarity > --- > include/linux/sched.h | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 0e1c38ad09c2..719147460ca8 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1541,7 +1541,17 @@ struct task_struct { > * A hardware-defined classification of task that reflects but is > * not identical to the number of instructions per cycle. > */ > - unsigned short ipcc; > + unsigned int ipcc : 9; > + /* > + * A candidate classification that arch-specific implementations > + * qualify for correctness. > + */ > + unsigned int ipcc_tmp : 9; > + /* > + * Counter to filter out transient candidate classifications > + * of a task. > + */ > + unsigned int ipcc_cntr : 14; > #endif > Why does this need to be split in task_struct? Isn't this architecture specific? IMO the scheduler should never make use of class information itself. It only receives the value though the call of an arch function and passes it as an argument to an arch function to get a performance score. So the way one interprets the class value (splits it in relevant and helper bits) should be defined and considered in the architecture specific code. Thanks, Ionela. > /* > -- > 2.25.1 >
On Thu, Jun 22, 2023 at 11:20:47AM +0100, Ionela Voinescu wrote: > Hi, > > On Monday 12 Jun 2023 at 21:24:16 (-0700), Ricardo Neri wrote: > > The raw classification that hardware provides for a task may not > > be directly usable by the scheduler: the classification may change too > > frequently or architecture-specific implementations may need to consider > > additional factors. For instance, some processors with Intel Thread > > Director need to consider the state of the SMT siblings of a core. > > > > Provide per-task helper variables that architectures can use to > > postprocess the classification that hardware provides. > > > > Cc: Ben Segall <bsegall@google.com> > > Cc: Daniel Bristot de Oliveira <bristot@redhat.com> > > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > > Cc: Ionela Voinescu <ionela.voinescu@arm.com> > > Cc: Joel Fernandes (Google) <joel@joelfernandes.org> > > Cc: Len Brown <len.brown@intel.com> > > Cc: Lukasz Luba <lukasz.luba@arm.com> > > Cc: Mel Gorman <mgorman@suse.de> > > Cc: Perry Yuan <Perry.Yuan@amd.com> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Tim C. Chen <tim.c.chen@intel.com> > > Cc: Valentin Schneider <vschneid@redhat.com> > > Cc: Zhao Liu <zhao1.liu@linux.intel.com> > > Cc: x86@kernel.org > > Cc: linux-pm@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > --- > > Changes since v3: > > * None > > > > Changes since v2: > > * None > > > > Changes since v1: > > * Used bit-fields to fit all the IPC class data in 4 bytes. (PeterZ) > > * Shortened names of the helpers. > > * Renamed helpers with the ipcc_ prefix. > > * Reworded commit message for clarity > > --- > > include/linux/sched.h | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index 0e1c38ad09c2..719147460ca8 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -1541,7 +1541,17 @@ struct task_struct { > > * A hardware-defined classification of task that reflects but is > > * not identical to the number of instructions per cycle. > > */ > > - unsigned short ipcc; > > + unsigned int ipcc : 9; > > + /* > > + * A candidate classification that arch-specific implementations > > + * qualify for correctness. > > + */ > > + unsigned int ipcc_tmp : 9; > > + /* > > + * Counter to filter out transient candidate classifications > > + * of a task. > > + */ > > + unsigned int ipcc_cntr : 14; > > #endif > > > > Why does this need to be split in task_struct? Isn't this architecture > specific? IMO the scheduler should never make use of class information > itself. It only receives the value though the call of an arch function > and passes it as an argument to an arch function to get a performance > score. So the way one interprets the class value (splits it in relevant > and helper bits) should be defined and considered in the architecture > specific code. This is an excellent observation. The scheduler is unconcerned with what the classes mean. I'll remove these auxiliary members. This also removes 2 patches from the series. Thanks and BR, Ricardo
diff --git a/include/linux/sched.h b/include/linux/sched.h index 0e1c38ad09c2..719147460ca8 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1541,7 +1541,17 @@ struct task_struct { * A hardware-defined classification of task that reflects but is * not identical to the number of instructions per cycle. */ - unsigned short ipcc; + unsigned int ipcc : 9; + /* + * A candidate classification that arch-specific implementations + * qualify for correctness. + */ + unsigned int ipcc_tmp : 9; + /* + * Counter to filter out transient candidate classifications + * of a task. + */ + unsigned int ipcc_cntr : 14; #endif /*
The raw classification that hardware provides for a task may not be directly usable by the scheduler: the classification may change too frequently or architecture-specific implementations may need to consider additional factors. For instance, some processors with Intel Thread Director need to consider the state of the SMT siblings of a core. Provide per-task helper variables that architectures can use to postprocess the classification that hardware provides. Cc: Ben Segall <bsegall@google.com> Cc: Daniel Bristot de Oliveira <bristot@redhat.com> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> Cc: Ionela Voinescu <ionela.voinescu@arm.com> Cc: Joel Fernandes (Google) <joel@joelfernandes.org> Cc: Len Brown <len.brown@intel.com> Cc: Lukasz Luba <lukasz.luba@arm.com> Cc: Mel Gorman <mgorman@suse.de> Cc: Perry Yuan <Perry.Yuan@amd.com> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Tim C. Chen <tim.c.chen@intel.com> Cc: Valentin Schneider <vschneid@redhat.com> Cc: Zhao Liu <zhao1.liu@linux.intel.com> Cc: x86@kernel.org Cc: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> --- Changes since v3: * None Changes since v2: * None Changes since v1: * Used bit-fields to fit all the IPC class data in 4 bytes. (PeterZ) * Shortened names of the helpers. * Renamed helpers with the ipcc_ prefix. * Reworded commit message for clarity --- include/linux/sched.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)