Re: [PATCH 1/6] Drivers: hv: vmbus: Add utility function for querying ring size

[Saurabh Singh Sengar]

On Sun, Feb 18, 2024 at 08:11:58AM +0100, Greg KH wrote:
> On Sat, Feb 17, 2024 at 10:03:35AM -0800, Saurabh Sengar wrote:
> > Add a function to query for the preferred ring buffer size of VMBus
> > device.
> 
> That says what you did, but not why you did it.

I thought subsequent patch will make it clear, but I can add more
info in cover letter. I will enhance this commit as well.

> 
> > 
> > Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  drivers/hv/channel_mgmt.c | 7 +++++--
> >  drivers/hv/hyperv_vmbus.h | 5 +++++
> >  include/linux/hyperv.h    | 1 +
> >  3 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> > index 2f4d09ce027a..7ea444d72f9f 100644
> > --- a/drivers/hv/channel_mgmt.c
> > +++ b/drivers/hv/channel_mgmt.c
> > @@ -120,7 +120,8 @@ const struct vmbus_device vmbus_devs[] = {
> >  	},
> >  
> >  	/* File copy */
> > -	{ .dev_type = HV_FCOPY,
> > +	{ .pref_ring_size = 0x4000,
> > +	  .dev_type = HV_FCOPY,
> >  	  HV_FCOPY_GUID,
> >  	  .perf_device = false,
> >  	  .allowed_in_isolated = false,
> > @@ -141,11 +142,13 @@ const struct vmbus_device vmbus_devs[] = {
> >  	},
> >  
> >  	/* Unknown GUID */
> > -	{ .dev_type = HV_UNKNOWN,
> > +	{ .pref_ring_size = 0x11000,
> > +	  .dev_type = HV_UNKNOWN,
> 
> Where do these magic numbers for the size come from?


This value is (16 + 1) page_size, which is found sufficient for most
of the slow devices. 16 page_size is for the ringbuffer and 1 page_size
for the headre. This is the approximation for default case, to avoid
using fall back case of 512 page_size as used by uio_hv_generic.

> 
> >  	  .perf_device = false,
> >  	  .allowed_in_isolated = false,> >  	},
> >  };
> > +EXPORT_SYMBOL_GPL(vmbus_devs);
> >  
> >  static const struct {
> >  	guid_t guid;
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index f6b1e710f805..76ac5185a01a 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -417,6 +417,11 @@ static inline bool hv_is_perf_channel(struct vmbus_channel *channel)
> >  	return vmbus_devs[channel->device_id].perf_device;
> >  }
> >  
> > +static inline size_t hv_dev_ring_size(struct vmbus_channel *channel)
> > +{
> > +	return vmbus_devs[channel->device_id].pref_ring_size;
> > +}
> > +
> >  static inline bool hv_is_allocated_cpu(unsigned int cpu)
> >  {
> >  	struct vmbus_channel *channel, *sc;
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index 2b00faf98017..5951c7bb5712 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -800,6 +800,7 @@ struct vmbus_requestor {
> >  #define VMBUS_RQST_RESET (U64_MAX - 3)
> >  
> >  struct vmbus_device {
> > +	size_t pref_ring_size;
> 
> No documentation for this?  What is the size in units of?

I can add a comment here like below:

/* 
 * Total memory in bytes allocated for the one complete ring buffer,
 * which includes the ring buffer header, of size PAGE_SIZE. This value
 * should be aligned to page_size.
 */

- Saurabh

> 
> thanks,
> 
> greg k-h