Re: [PATCH 1/3] Drivers: hv: vmbus: Expose per-channel interrupts and events counters

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Oct 05, 2017 at 05:35:04PM -0700, kys@exchange.microsoft.com wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> When investigating performance, it is useful to be able to look at
> the number of host and guest events per-channel. This is equivalent
> to per-device interrupt statistics.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  Documentation/ABI/stable/sysfs-bus-vmbus | 14 ++++++++++++++
>  drivers/hv/connection.c                  |  2 ++
>  drivers/hv/vmbus_drv.c                   | 18 ++++++++++++++++++
>  include/linux/hyperv.h                   |  4 ++++
>  4 files changed, 38 insertions(+)
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
> index 0ebd8a1537a0..d4eca1717adb 100644
> --- a/Documentation/ABI/stable/sysfs-bus-vmbus
> +++ b/Documentation/ABI/stable/sysfs-bus-vmbus
> @@ -97,3 +97,17 @@ KernelVersion:	4.14
>  Contact:	Stephen Hemminger <sthemmin@microsoft.com>
>  Description:	Bytes availabble to write
>  Users:		Debuggig tools
> +
> +What:		/sys/bus/vmbus/devices/vmbus_*/channels/relid/events
> +Date:		September. 2017
> +KernelVersion:	4.14
> +Contact:	Stephen Hemminger <sthemmin@microsoft.com>
> +Description:	Number of times we have signaled the host
> +Users:		Debuggig tools

Odd spelling :)

> +
> +What:		/sys/bus/vmbus/devices/vmbus_*/channels/relid/interrupts
> +Date:		September. 2017
> +KernelVersion:	4.14
> +Contact:	Stephen Hemminger <sthemmin@microsoft.com>
> +Description:	Number of times we have taken an interrupt (incoming)
> +Users:		Debuggig tools

Same odd spelling :(

> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index f41901f80b64..b06a6b796819 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -409,6 +409,8 @@ void vmbus_set_event(struct vmbus_channel *channel)
>  	if (!channel->is_dedicated_interrupt)
>  		vmbus_send_interrupt(child_relid);
>  
> +	++channel->sig_events;
> +
>  	hv_do_fast_hypercall8(HVCALL_SIGNAL_EVENT, channel->sig_event);
>  }
>  EXPORT_SYMBOL_GPL(vmbus_set_event);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index a209527b09f5..e8cd19095212 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -945,6 +945,8 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
>  			if (channel->rescind)
>  				continue;
>  
> +			++channel->interrupts;
> +
>  			switch (channel->callback_mode) {
>  			case HV_CALL_ISR:
>  				vmbus_channel_isr(channel);
> @@ -1238,6 +1240,20 @@ static ssize_t channel_latency_show(const struct vmbus_channel *channel,
>  }
>  VMBUS_CHAN_ATTR(latency, S_IRUGO, channel_latency_show, NULL);
>  
> +static ssize_t channel_interrupts_show(const struct vmbus_channel *channel, char *buf)
> +{
> +	return sprintf(buf, "%u\n", channel->interrupts);
> +}
> +VMBUS_CHAN_ATTR(interrupts, S_IRUGO, channel_interrupts_show, NULL);
> +
> +static ssize_t channel_events_show(const struct vmbus_channel *channel, char *buf)
> +{
> +	return sprintf(buf, "%u\n", channel->sig_events);
> +}
> +VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
> +
> +
> +

Why so many blank lines?

>  static struct attribute *vmbus_chan_attrs[] = {
>  	&chan_attr_out_mask.attr,
>  	&chan_attr_in_mask.attr,
> @@ -1246,6 +1262,8 @@ static struct attribute *vmbus_chan_attrs[] = {
>  	&chan_attr_cpu.attr,
>  	&chan_attr_pending.attr,
>  	&chan_attr_latency.attr,
> +	&chan_attr_interrupts.attr,
> +	&chan_attr_events.attr,
>  	NULL
>  };
>  
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index ef16ee039850..1b7e15d22f58 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -719,6 +719,10 @@ struct vmbus_channel {
>  
>  	struct vmbus_close_msg close_msg;
>  
> +	/* Statistics */
> +	unsigned int	interrupts;	/* Host to Guest interrupts */
> +	unsigned int	sig_events;	/* Guest to Host events */

u32 or u64 here?  How is userspace going to handle these overflowing?

thanks,

greg k-h