Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[MUKESH RATHOR <mukeshrathor@microsoft.com>]

On 2/21/25 10:10, Nuno Das Neves wrote:
 > On 2/20/2025 2:59 PM, MUKESH RATHOR wrote:
 >>
 >>
 >> On 2/20/25 14:56, Easwar Hariharan wrote:
 >>   > On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
 >>   >>
 >>   >>
 >>   >> On 2/20/25 10:33, Nuno Das Neves wrote:
 >>   >>   > Introduce hv_current_partition_type to store the partition type
 >>   >>   > as an enum.
 >>   >>   >
 >>   >>   > option to gate the compilation of root partition code. In
 >> particular,

<snip>

 >>   > <snip>
 >>   >
 >>   >>   > @@ -34,8 +34,11 @@
 >>   >>   >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
 >>   >>   >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
 >>   >>   >
 >>   >>   > +enum hv_partition_type hv_current_partition_type;
 >>   >>   > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
 >>   >>   > +
 >>   >>
 >>   >> nit: if possible and not too late, can we please use more Unix
 >>   >> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
 >>   >> long windows style names that causes unnecessary line wraps/splits.
 >>   >>
 >>   >> Thanks,
 >>   >> -Mukesh
 >>   >>
 >>   >
 >>   > Per
 >>
https://docs.kernel.org/process/coding-style.html#naming
 >>   >
 >>   > GLOBAL variables (to be used only if you really need them) need to
 >> have descriptive names,
 >>   > as do global functions. If you have a function that counts the
number
 >> of active users,
 >>   > you should call that count_active_users() or similar, you should not
 >> call it cntusr().
 >>
 >> Thant's hardly a fair comparison. Suggestion was NOT hvptid.
 >>
 > I'm in favor of shortening the names when the abbreviation is common and
 > therefore still perfectly clear to anyone reading it - e.g. "curr" is
 > a perfectly acceptable abbreviation of "current", in my view.
 >
 > I think abbreviating "partition" to "pt" is probably not a good fit for
 > global variables. Anyone seeing a variable with the word "partition"
 > (and hv_ prefix) can go look up what a Hyper-V partition is if they don't
 > know, but "pt" would be completely impenetrable without reading through a
 > fair amount of the code that uses it to figure out what it refers to.
 >
 > I think even slightly longer abbreviations like "part", "ptn", "prt", or
 > "prtn" are not good enough unfortunately... the word "partition" just
 > doesn't lend itself to abbreviation in an obvious way.

That is fine, IMO. We look at the code often enough, and some names like
that get well established. For example, pfn or current for current task.
Moreover, the prefix hv_* narrows it down to hyperv related. Other
suggestions: hv_curr_partn_type and hv_curr_partn_id if you don't
like part, ptn etc. I'll let you pick whatever, hv_curr_partition_id is
def better.  In the end, concise yet unique enough that anyone can
easily find all references via grep/cscope/etc without RSI :).

 > So, for this patch I'm fine with changing it to "hv_curr_partition_type"
 > which saves a few characters.
 >
 > Feel free to post a followup for "hv_curr_partition_id" if you like.

 > Note - For the driver code which isn't as exposed to the rest of the
 > kernel, I think we can continue to use "pt" or similar to keep the names
 > shorter.

That's a good idea.

Thanks,
-Mukesh