linux-kernel - Re: [PATCH 2/4] drm/etnaviv: add loadavg accounting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH9NwWex+9LvaBhzPkYDaYOHnvxFeK4sAMgFZi2i5b+TOSVPmA@mail.gmail.com>
Date:   Fri, 10 Jul 2020 10:50:59 +0200
From:   Christian Gmeiner <christian.gmeiner@...il.com>
To:     Lucas Stach <l.stach@...gutronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Chris Healy <cphealy@...il.com>,
        Russell King <linux+etnaviv@...linux.org.uk>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        The etnaviv authors <etnaviv@...ts.freedesktop.org>,
        DRI mailing list <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 2/4] drm/etnaviv: add loadavg accounting

Hoi Lucas,

Am Fr., 10. Juli 2020 um 10:19 Uhr schrieb Lucas Stach <l.stach@...gutronix.de>:
>
> Hi Christian,
>
> Am Freitag, den 10.07.2020, 09:41 +0200 schrieb Christian Gmeiner:
> > The GPU has an idle state register where each bit represents the idle
> > state of a sub-GPU component like FE or TX. Sample this register
> > every 10ms and calculate a simple moving average over the sub-GPU
> > component idle states with a total observation time frame of 1s.
> >
> > This provides us with a percentage based load of each sub-GPU
> > component.
> >
> > Signed-off-by: Christian Gmeiner <christian.gmeiner@...il.com>
> > ---
> >  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 ++++++++++++
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 32 +++++++++++++++++++++++++++
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 29 ++++++++++++++++++++++++
> >  3 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index f9afe11c50f0..b31920241c86 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
> >       }
> >  }
> >
> > +static void unload_gpu(struct drm_device *dev)
> > +{
> > +     struct etnaviv_drm_private *priv = dev->dev_private;
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < ETNA_MAX_PIPES; i++) {
> > +             struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > +             if (g)
> > +                     etnaviv_gpu_shutdown(g);
> > +     }
> > +}
> > +
> >  static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
> >  {
> >       struct etnaviv_drm_private *priv = dev->dev_private;
> > @@ -581,6 +594,7 @@ static void etnaviv_unbind(struct device *dev)
> >       struct drm_device *drm = dev_get_drvdata(dev);
> >       struct etnaviv_drm_private *priv = drm->dev_private;
> >
> > +     unload_gpu(drm);
> >       drm_dev_unregister(drm);
> >
> >       component_unbind_all(dev, drm);
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index a31eeff2b297..1f0eb7e00657 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -714,6 +714,28 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> >       gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> >  }
> >
> > +static void etnaviv_loadavg_function(struct timer_list *t)
> > +{
> > +     struct etnaviv_gpu *gpu = from_timer(gpu, t, loadavg_timer);
> > +     const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
>
> This isn't guaranteed to work on a clock/power gated GPU. Also we
> surely don't want to wake a idle system every 10ms just to sample a "no
> load" value, so this needs some integration with runtime PM, to disable
> the sampling when the GPU is powered down and enable when powered up.
> The loadavg must be able to adapt to jumps in the sampling interval
> while idle.
>

Oh yea.. runtime PM.. I thought I was missing something. Will tackle this in the
next version.

>
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > +             if ((idle & etna_idle_module_names[i].bit))
> > +                     sma_loadavg_add(&gpu->loadavg_value[i], 0);
> > +             else
> > +                     sma_loadavg_add(&gpu->loadavg_value[i], 100);
> > +
> > +     spin_lock_bh(&gpu->loadavg_spinlock);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > +             gpu->loadavg_percentage[i] = sma_loadavg_read(&gpu->loadavg_value[i]);
> > +
> > +     spin_unlock_bh(&gpu->loadavg_spinlock);
> > +
> > +     mod_timer(t, jiffies + msecs_to_jiffies(10));
>
> A jiffies based timer is much too coarse for a regular 10ms sampling.
> On a typical 100Hz system 10ms is a single jiffy, so your timer will
> fire anywhere in the range of ~0ms...~20ms. This won't get us a usable
> measurement.
>

Makes sense.. will switch to hrtimers.

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy