Re: [PATCH v6 17/23] virt/steal_monitor: Provide default method to get systemwide steal time

[Yury Norov <ynorov@nvidia.com>]

On Mon, Jul 06, 2026 at 02:17:15PM +0530, Shrikanth Hegde wrote:
> 
> 
> On 7/4/26 12:46 AM, Yury Norov wrote:
> > On Wed, Jul 01, 2026 at 07:46:48PM +0530, Shrikanth Hegde wrote:
> > > steal monitor takes global view of steal time instead of individual
> > > vCPU. For this collect overall steal values across all the vCPUs or
> > > vCPUs of interest.
> > > 
> > > Default implementation chooses steal time across all active CPUs.
> > > 
> > > Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> > > ---
> > > v5->v6:
> > > - Add cpus_read_lock() for hotplug safety
> > > 
> > >   drivers/virt/steal_monitor/Makefile   |  2 +-
> > >   drivers/virt/steal_monitor/defaults.c | 28 +++++++++++++++++++++++++++
> > >   drivers/virt/steal_monitor/sm_core.h  |  3 +++
> > >   3 files changed, 32 insertions(+), 1 deletion(-)
> > >   create mode 100644 drivers/virt/steal_monitor/defaults.c
> > > 
> > > diff --git a/drivers/virt/steal_monitor/Makefile b/drivers/virt/steal_monitor/Makefile
> > > index 24cee55342ce..7c16f8cf9583 100644
> > > --- a/drivers/virt/steal_monitor/Makefile
> > > +++ b/drivers/virt/steal_monitor/Makefile
> > > @@ -11,4 +11,4 @@
> > >   #
> > >   obj-$(subst y,m,$(CONFIG_PREFERRED_CPU)) += steal_monitor.o
> > > -steal_monitor-y := sm_core.o
> > > +steal_monitor-y := sm_core.o defaults.o
> > > diff --git a/drivers/virt/steal_monitor/defaults.c b/drivers/virt/steal_monitor/defaults.c
> > > new file mode 100644
> > > index 000000000000..6681f9938f6a
> > > --- /dev/null
> > > +++ b/drivers/virt/steal_monitor/defaults.c
> > > @@ -0,0 +1,28 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Base file contains the default implementations.
> > > + * These are defined as __weak so that arch may define
> > > + * strong symbols to override.
> > 
> > But there's no arch doing that, right? It looks like overcomplication
> > with no benefit.
> > 
> > There's just ~50 drivers defining weak symbols, so this is not a
> > common practice. I think it would be much simpler for the arch people
> > to simply write their own driver controlling cpu_preferred_mask,
> > rather than tweaking your code.
> > 
> 
> s390 folks asked for explicit hooks since they have some of the info
> coming from HW w.r.t to vertical high/vertical low etc. So i have designed it that way.
> (I know code is not there yet)
> 
> If each arch writes its own driver, wouldn't that lead to
> - code duplication as each arch more or less may have to do same.
> - each arch has to catch up any improvements that happens to default monitor.
> - each arch has to honor design construct and not violate it.
> - kconfig puzzles, to choose generic or arch specific ones etc.
 
That sounds reasonable and logically correct. But wait...

Why only 50 drivers play nasty __weak games? How all others handle
that? Maybe callbacks? Maybe hooks or function tables? Something else?

You started this as a small demonstration, now it more reminds
a framework enterprise solution. Which it isn't
 
> If it makes sense,
> - I can make it as strong symbols for now.
> - when the arch specific implementations arrive, can make it as
>   __weak to accommodate that.

Please make the symbols all strong unless you actually have arch
implementation. If you think that weak functions is the best approach
for s390, feel free to provide such a patch as the very last in the
series, mentioning it's for those s390 people reference, not for
immediate imclusion.

Thanks,
Yury