From: Nuno Das Neves <[email protected]> Sent: Friday, January 
23, 2026 4:13 PM
> 
> On 1/23/2026 2:31 PM, Stanislav Kinsburskii wrote:
> > On Fri, Jan 23, 2026 at 11:04:52AM -0800, Nuno Das Neves wrote:
> >> On 1/23/2026 9:09 AM, Michael Kelley wrote:
> >>> From: Nuno Das Neves <[email protected]> Sent: Wednesday, 
> >>> January 21, 2026 1:46 PM
> >>>>
> >>>> Introduce hv_counters.c, containing static data corresponding to
> >>>> HV_*_COUNTER enums in the hypervisor source. Defining the enum
> >>>> members as an array instead makes more sense, since it will be
> >>>> iterated over to print counter information to debugfs.
> >>>
> >>> I would have expected the filename to be mshv_counters.c, so that the 
> >>> association
> >>> with the MS hypervisor is clear. And the file is inextricably linked to 
> >>> mshv_debugfs.c,
> >>> which of course has the "mshv_" prefix. Or is there some thinking I'm not 
> >>> aware of
> >>> for using the "hv_" prefix?
> >>>
> >> Good question - I originally thought of using hv_ because the definitions 
> >> inside are
> >> part of the hypervisor ABI, and hence also have the hv_ prefix.
> >>
> >> However you have a good point, and I'm not opposed to changing it.
> >>
> >> Maybe to just be super explicit: "mshv_debugfs_counters.c" ?
> >>
> >
> > This is reudnant from my POV.
> > If these counters are only used by mshv_debugfs.c, then should rather be
> > a part of this file.
> > What was the reason to move them elsewhere?
> >
> 
> Just a matter of taste - so there isn't ~450 lines of definitions at the 
> beginning of
> mshv_debugfs.c. But I'm not fussed. If you think it's better to just prepend 
> the
> definitions to mshv_debugfs.c, then that's an easy change.
> 
> Nuno

FWIW, I preferred the separate file so that the main debugfs code
isn't burdened with 450 lines of definitions that aren't going to be
edited/revised/improved via the typical processes. The current
mshv_debugfs.c is a reasonable 700 lines of code without all the
definitions.

But it's not a big deal for me either way.

Michael

> 
> > Thanks,
> > Stanislav
> >
> >>> Also, I see in Patch 7 of this series that hv_counters.c is #included as 
> >>> a .c file
> >>> in mshv_debugfs.c. Is there a reason for doing the #include instead of 
> >>> adding
> >>> hv_counters.c to the Makefile and building it on its own? You would need 
> >>> to
> >>> add a handful of extern statements to mshv_root.h so that the tables are
> >>> referenceable from mshv_debugfs.c. But that would seem to be the more
> >>> normal way of doing things.  #including a .c file is unusual.
> >>>
> >>
> >> Yes...I thought I could avoid noise in mshv_root.h and the Makefile, since 
> >> it's
> >> only relevant for mshv_debugfs.c. However I could see this file (whether 
> >> as .c or
> >> .h) being misused and included elsewhere inadvertantly, which would 
> >> duplicate the
> >> tables, so maybe doing it the normal way is a better idea, even if 
> >> mshv_debugfs.c
> >> is likely the only user.
> >>
> >>> See one more comment on the last line of this patch ...
> >>>
> 
> <snip>

Reply via email to