Re: [PATCH v4 18/21] drivers: hv: vmbus: replace deprecated mmap hook with mmap_prepare
"Lorenzo Stoakes (Oracle)" <[email protected]>
| Newsgroups | org.kernel.vger.target-devel,dev.linux.lists.linux-staging,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mtd,org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-hyperv,org.kernel.vger.linux-kernel,org.kernel.vger.linux-scsi,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Mar 23, 2026 at 04:16:20AM +0000, Michael Kelley wrote: > From: Lorenzo Stoakes (Oracle) <[email protected]> Sent: Friday, March 20, 2026 3:40 PM > > > > The f_op->mmap interface is deprecated, so update the vmbus driver to use > > its successor, mmap_prepare. > > > > This updates all callbacks which referenced the function pointer > > hv_mmap_ring_buffer to instead reference hv_mmap_prepare_ring_buffer, > > utilising the newly introduced compat_set_desc_from_vma() and > > __compat_vma_mmap() to be able to implement this change. > > > > The UIO HV generic driver is the only user of hv_create_ring_sysfs(), > > which is the only function which references > > vmbus_channel->mmap_prepare_ring_buffer which, in turn, is the only > > external interface to hv_mmap_prepare_ring_buffer. > > > > This patch therefore updates this caller to use mmap_prepare instead, > > which also previously used vm_iomap_memory(), so this change replaces it > > with its mmap_prepare equivalent, mmap_action_simple_ioremap(). > > > > Signed-off-by: Lorenzo Stoakes (Oracle) <[email protected]> > > --- > > drivers/hv/hyperv_vmbus.h | 4 ++-- > > drivers/hv/vmbus_drv.c | 31 +++++++++++++++++++------------ > > drivers/uio/uio_hv_generic.c | 11 ++++++----- > > include/linux/hyperv.h | 4 ++-- > > 4 files changed, 29 insertions(+), 21 deletions(-) > > > > There are two mmap() code paths in the Hyper-V UIO code. One path is > to mmap() the file descriptor for /dev/uio<n>, and the other is to mmap() > the "ring" entry under /sys/devices/vmbus/devices/<uuid>. The former is > done by uio_mmap(), and the latter by hv_uio_ring_mmap_prepare(). > > I tested both these paths using a combination of two methods in a > x86/x64 VM on Hyper-V: > > 1) Using the fcopy daemon, which maps the ring buffer for the primary > channel and sends/receives messages with the Hyper-V host. This > method tests only the 1st path because the fcopy daemon doesn't create > any subchannels that would use the "ring" entry. > > 2) Using a custom-built test program. This program doesn't communicate > with the Hyper-V host, but allows mostly verifying both code paths for the > primary channel. As a sanity check, it verifies that the two mmaps are > mapping the same memory, as expected. > > As such, > > Reviewed-by: Michael Kelley <[email protected]> > Tested-by: Michael Kelley <[email protected]> Perfect, thanks so much for this! It is tricky for me to test these, beyond fairly exhaustive logical confirmation of equivalence, so this is _hugely_ helpful. > > The most robust test would be to run DPDK networking against > UIO, as it would communicate with the Hyper-V host and use > multiple subchannels that resulting in mmap'ing the "ring" > entry under /sys. > > @Long Li -- I'll leave it to your discretion as to whether you want > to test DPDK against these mmap() changes. Thanks in advance for taking a look on this also! > > I've noted one minor issue below. > > [snip] > > --- a/include/linux/hyperv.h > +++ b/include/linux/hyperv.h > @@ -1015,8 +1015,8 @@ struct vmbus_channel { > /* The max size of a packet on this channel */ > u32 max_pkt_size; > > - /* function to mmap ring buffer memory to the channel's sysfs ring attribute */ > - int (*mmap_ring_buffer)(struct vmbus_channel *channel, struct vm_area_struct *vma); > + /* function to mmap_prepare ring buffer memory to the channel's sysfs ring attribute */ > > Changing the comment from "mmap ring buffer" to "mmap_prepare ring buffer" > produces awkward wording since "mmap" is used here as a verb. It might be better > to just leave the comment unchanged. Sure am happy with that of course, I think Sashiko moaned about this but it's obviously fine either way. Andrew - do you mind restoring the comment to its original form above? Thanks! > > Michael > > > + int (*mmap_prepare_ring_buffer)(struct vmbus_channel *channel, struct vm_area_desc *desc); > > /* boolean to control visibility of sysfs for ring buffer */ > bool ring_sysfs_visible; Cheers, Lorenzo