Re: [PATCH v8 2/2] misc: Add a mechanism to detect stalls on guest vCPUs

From: Sebastian Ene
Date: Tue Jun 28 2022 - 10:11:50 EST


On Tue, Jun 28, 2022 at 07:00:05AM -0700, Guenter Roeck wrote:
> On 6/28/22 02:54, Sebastian Ene wrote:
> [ ... ]
> > > > +static struct platform_device *virt_dev;
> > > > +

Hi,

> > >
> > > virt_dev is only used to call platform_set_drvdata() and platform_get_drvdata()
> > > on it. Why not just have a static variable named vm_stall_detect ?
> > >
> >
> > I think this should also work. I wanted to make use of the provided APIs
> > like platform_set/platform_get.
> >
>
> That doesn't mean such APIs should be used just to get used, though.
>

I will remove these calls and keep it static.

> > > > +
> > > > + vm_stall_detect = (struct vm_stall_detect_s __percpu *)
> > > > + platform_get_drvdata(virt_dev);
> > >
> > > platform_get_drvdata() returns void *; typecast to it is unnecessary.
> > >
> > >
> >
> > I needed this typecast because the variable is per-cpu and some
> > compilers(eg. gcc for ARCH=h8300) complain if we don't specify this
> > hint.
> >
> Hmm, interesting. I didn't know that. We live and learn.
> Though h8300 is gone now :-)

I had some Intel robot complaining about this in my previous series(v5) and
I fixed the warnings by adding these compiler hints.

>
> Did you reply in private on purpose ?
>

I misused my CC list but I will fix this in my reply.

> Thanks,
> Guenter

Thanks for the response,
Seb