From: Stanislav Kinsburskii <[email protected]> Sent: Monday, February 2, 2026 10:56 AM > > On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote: > > From: Stanislav Kinsburskii <[email protected]> Sent: > > Monday, February 2, 2026 9:18 AM > > > > > > On Mon, Feb 02, 2026 at 08:51:01AM -0800, [email protected] wrote: > > > > From: Michael Kelley <[email protected]> > > > > > > > > Huge page mappings in the guest physical address space depend on having > > > > matching alignment of the userspace address in the parent partition and > > > > of the guest physical address. Add a comment that captures this > > > > information. See the link to the mailing list thread. > > > > > > > > No code or functional change. > > > > > > > > Link: > > > > https://lore.kernel.org/linux-hyperv/[email protected]/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc > > > > Signed-off-by: Michael Kelley <[email protected]> > > > > --- > > > > drivers/hv/mshv_root_main.c | 14 ++++++++++++++ > > > > 1 file changed, 14 insertions(+) > > > > > > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c > > > > index 681b58154d5e..bc738ff4508e 100644 > > > > --- a/drivers/hv/mshv_root_main.c > > > > +++ b/drivers/hv/mshv_root_main.c > > > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct > > > > mshv_partition *partition, > > > > if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP)) > > > > return mshv_unmap_user_memory(partition, mem); > > > > > > > > + /* > > > > + * If the userspace_addr and the guest physical address (as > > > > derived > > > > + * from the guest_pfn) have the same alignment modulo PMD huge > > > > page > > > > + * size, the MSHV driver can map any PMD huge pages to the guest > > > > + * physical address space as PMD huge pages. If the alignments > > > > do > > > > + * not match, PMD huge pages must be mapped as single pages in > > > > the > > > > + * guest physical address space. The MSHV driver does not > > > > enforce > > > > + * that the alignments match, and it invokes the hypervisor to > > > > set > > > > + * up correct functional mappings either way. See > > > > mshv_chunk_stride(). > > > > + * The caller of the ioctl is responsible for providing > > > > userspace_addr > > > > + * and guest_pfn values with matching alignments if it wants > > > > the guest > > > > + * to get the performance benefits of PMD huge page mappings of > > > > its > > > > + * physical address space to real system memory. > > > > + */ > > > > > > Thanks. However, I'd suggest to reduce this commet a lot and put the > > > details into the commit message instead. Also, why this place? Why not a > > > part of the function description instead, for example? > > > > In general, I'm very much an advocate of putting a bit more detail into code > > comments, so that someone new reading the code has a chance of figuring > > out what's going on without having to search through the commit history > > and read commit messages. The commit history is certainly useful for the > > historical record, and especially how things have changed over time. But for > > "how non-obvious things work now", I like to see that in the code comments. > > > > This approach is not well aligned with the existing kernel coding style. > It is common to answer the "why" question in the commit message. > Code comments should focus on "what" the code does. > > https://www.kernel.org/doc/html/latest/process/coding-style.html >
Which says "Instead, put the comments at the head of the function, telling people what it does, and possibly WHY it does it." I'm good with that approach. > For more details, it is common to use `git blame` to learn the context > of a change when needed. Yep, I use that all the time for the historical record. > > > As for where to put the comment, I'm flexible. I thought about placing it > > outside the function as a "header" (which is what I think you mean by the > > "function description"), but the function handles both "map" and "unmap" > > operations, and this comment applies only to "map". Hence I put it after > > the test for whether we're doing "map" vs. "unmap". But I wouldn't object > > to it being placed as a function description, though the text would need to > > be > > enhanced to more broadly be a function description instead of just a comment > > about a specific aspect of "map" behavior. > > > > As for the location, since this documents the userspace API, I would > rather place it above the function as part of the function description. > Even though the function handles both map and unmap, unmap also deals > with huge pages. I'll do a version written as the function description. But the full function description will be more extensive to cover all the "what" that this function implements: * input parameters, and their valid values * map and unmap * when pinned vs. movable vs. mmio regions are created * what is done with huge pages in the above cases (i.e., a massaged version of what I've already written) * populating and pinning of pages for pinned regions Does that match with your expectations? Michael
