| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 Jan 2019 06:45:14 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: kimbrownkd <kimbrownkd@...il.com>,
Michael Kelley <mikelley@...rosoft.com>,
Long Li <longli@...rosoft.com>,
Sasha Levin <Alexander.Levin@...rosoft.com>
CC: KY Srinivasan <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3] Drivers: hv: vmbus: Expose counters for interrupts and
full conditions
> From: Kimberly Brown <kimbrownkd@...il.com>
> Sent: Wednesday, January 16, 2019 8:38 PM
> To: Michael Kelley <mikelley@...rosoft.com>; Long Li
> <longli@...rosoft.com>; Sasha Levin <Alexander.Levin@...rosoft.com>;
> Dexuan Cui <decui@...rosoft.com>
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Stephen Hemminger
> <sthemmin@...rosoft.com>; devel@...uxdriverproject.org;
> linux-kernel@...r.kernel.org
> Subject: [PATCH v3] Drivers: hv: vmbus: Expose counters for interrupts and full
> conditions
>
> Counter values for per-channel interrupts and ring buffer full
> conditions are useful for investigating performance.
>
> Expose counters in sysfs for 2 types of guest to host interrupts:
> 1) Interrupts caused by the channel's outbound ring buffer transitioning
> from empty to not empty
> 2) Interrupts caused by the channel's inbound ring buffer transitioning
> from full to not full while a packet is waiting for enough buffer space to
> become available
>
> Expose 2 counters in sysfs for the number of times that write operations
> encountered a full outbound ring buffer:
> 1) The total number of write operations that encountered a full
> condition
> 2) The number of write operations that were the first to encounter a
> full condition
>
> I tested this patch by confirming that the sysfs files were created and
> observing the counter values. The values seemed to increase by a
> reasonable amount when the Hyper-v related drivers were in use.
>
> Signed-off-by: Kimberly Brown <kimbrownkd@...il.com>
> ---
> Changes in v3:
> - Used the outbound ring buffer spinlock to protect the the full
> condition counters in set_channel_pending_send_size()
> - Corrected the KernelVersion values for the new entries in
> Documentation/ABI/stable/sysfs-bus-vmbus
>
> Changes in v2:
> - Added mailing lists to the cc list
> - Removed the host to guest interrupt counters proposed in v1 because
> they were not accurate
> - Added full condition counters for the channel's outbound ring buffer
>
> Documentation/ABI/stable/sysfs-bus-vmbus | 33 ++++++++++++++++++++
> drivers/hv/ring_buffer.c | 14 ++++++++-
> drivers/hv/vmbus_drv.c | 32 ++++++++++++++++++++
> include/linux/hyperv.h | 38
> ++++++++++++++++++++++++
> 4 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus
> b/Documentation/ABI/stable/sysfs-bus-vmbus
> index 3fed8fdb873d..a0304c563467 100644
> --- a/Documentation/ABI/stable/sysfs-bus-vmbus
> +++ b/Documentation/ABI/stable/sysfs-bus-vmbus
> @@ -146,3 +146,36 @@ KernelVersion: 4.16
> Contact: Stephen Hemminger <sthemmin@...rosoft.com>
> Description: Binary file created by uio_hv_generic for ring buffer
> Users: Userspace drivers
> +
> +What:
> /sys/bus/vmbus/devices/<UUID>/channels/<N>/intr_in_full
> +Date: January 2019
> +KernelVersion: 5.0
> +Contact: Michael Kelley <mikelley@...rosoft.com>
> +Description: Number of guest to host interrupts caused by the inbound
> ring
> + buffer transitioning from full to not full while a packet is
> + waiting for buffer space to become available
> +Users: Debugging tools
> +
> +What:
> /sys/bus/vmbus/devices/<UUID>/channels/<N>/intr_out_empty
> +Date: January 2019
> +KernelVersion: 5.0
> +Contact: Michael Kelley <mikelley@...rosoft.com>
> +Description: Number of guest to host interrupts caused by the outbound
> ring
> + buffer transitioning from empty to not empty
> +Users: Debugging tools
> +
> +What:
> /sys/bus/vmbus/devices/<UUID>/channels/<N>/out_full_first
> +Date: January 2019
> +KernelVersion: 5.0
> +Contact: Michael Kelley <mikelley@...rosoft.com>
> +Description: Number of write operations that were the first to encounter
> an
> + outbound ring buffer full condition
> +Users: Debugging tools
> +
> +What:
> /sys/bus/vmbus/devices/<UUID>/channels/<N>/out_full_total
> +Date: January 2019
> +KernelVersion: 5.0
> +Contact: Michael Kelley <mikelley@...rosoft.com>
> +Description: Total number of write operations that encountered an
> outbound
> + ring buffer full condition
> +Users: Debugging tools
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 1f1a55e07733..9e8b31ccc142 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -74,8 +74,10 @@ static void hv_signal_on_write(u32 old_write, struct
> vmbus_channel *channel)
> * This is the only case we need to signal when the
> * ring transitions from being empty to non-empty.
> */
> - if (old_write == READ_ONCE(rbi->ring_buffer->read_index))
> + if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) {
> + ++channel->intr_out_empty;
> vmbus_setevent(channel);
> + }
> }
>
> /* Get the next write location for the specified ring buffer. */
> @@ -272,10 +274,19 @@ int hv_ringbuffer_write(struct vmbus_channel
> *channel,
> * is empty since the read index == write index.
> */
> if (bytes_avail_towrite <= totalbytes_towrite) {
> + ++channel->out_full_total;
> +
> + if (!channel->out_full_flag) {
> + ++channel->out_full_first;
> + channel->out_full_flag = true;
> + }
> +
> spin_unlock_irqrestore(&outring_info->ring_lock, flags);
> return -EAGAIN;
> }
>
> + channel->out_full_flag = false;
> +
> /* Write to the ring buffer */
> next_write_location = hv_get_next_write_location(outring_info);
>
> @@ -530,6 +541,7 @@ void hv_pkt_iter_close(struct vmbus_channel
> *channel)
> if (curr_write_sz <= pending_sz)
> return;
>
> + ++channel->intr_in_full;
> vmbus_setevent(channel);
> }
> EXPORT_SYMBOL_GPL(hv_pkt_iter_close);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 403fee01572c..e291a7d3180c 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1496,6 +1496,34 @@ static ssize_t channel_events_show(const struct
> vmbus_channel *channel, char *bu
> }
> static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
>
> +static ssize_t channel_intr_in_full_show(const struct vmbus_channel
> *channel,
> + char *buf)
> +{
> + return sprintf(buf, "%llu\n", channel->intr_in_full);
> +}
> +static VMBUS_CHAN_ATTR(intr_in_full, 0444, channel_intr_in_full_show,
> NULL);
> +
> +static ssize_t channel_intr_out_empty_show(const struct vmbus_channel
> *channel,
> + char *buf)
> +{
> + return sprintf(buf, "%llu\n", channel->intr_out_empty);
> +}
> +static VMBUS_CHAN_ATTR(intr_out_empty, 0444,
> channel_intr_out_empty_show, NULL);
> +
> +static ssize_t channel_out_full_first_show(const struct vmbus_channel
> *channel,
> + char *buf)
> +{
> + return sprintf(buf, "%llu\n", channel->out_full_first);
> +}
> +static VMBUS_CHAN_ATTR(out_full_first, 0444, channel_out_full_first_show,
> NULL);
> +
> +static ssize_t channel_out_full_total_show(const struct vmbus_channel
> *channel,
> + char *buf)
> +{
> + return sprintf(buf, "%llu\n", channel->out_full_total);
> +}
> +static VMBUS_CHAN_ATTR(out_full_total, 0444, channel_out_full_total_show,
> NULL);
> +
> static ssize_t subchannel_monitor_id_show(const struct vmbus_channel
> *channel,
> char *buf)
> {
> @@ -1521,6 +1549,10 @@ static struct attribute *vmbus_chan_attrs[] = {
> &chan_attr_latency.attr,
> &chan_attr_interrupts.attr,
> &chan_attr_events.attr,
> + &chan_attr_intr_in_full.attr,
> + &chan_attr_intr_out_empty.attr,
> + &chan_attr_out_full_first.attr,
> + &chan_attr_out_full_total.attr,
> &chan_attr_monitor_id.attr,
> &chan_attr_subchannel_id.attr,
> NULL
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index dcb6977afce9..7e5239123276 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -751,6 +751,27 @@ struct vmbus_channel {
> u64 interrupts; /* Host to Guest interrupts */
> u64 sig_events; /* Guest to Host events */
>
> + /* Interrupt counts for 2 types of Guest to Host interrupts */
> + u64 intr_in_full; /* in ring buffer, full to not full */
> + u64 intr_out_empty; /* out ring buffer, empty to not empty */
> +
> + /*
> + * The total number of write operations that encountered a full
> + * outbound ring buffer.
> + */
> + u64 out_full_total;
> + /*
> + * The number of write operations that were the first to encounter a
> + * full outbound ring buffer.
> + */
> + u64 out_full_first;
> + /*
> + * Indicates that a full outbound ring buffer was encountered. The flag
> + * is set to true when a full outbound ring buffer is encountered and
> + * set to false when a write to the outbound ring buffer is completed.
> + */
> + bool out_full_flag;
> +
> /* Channel callback's invoked in softirq context */
> struct tasklet_struct callback_event;
> void (*onchannel_callback)(void *context);
> @@ -936,6 +957,23 @@ static inline void *get_per_channel_state(struct
> vmbus_channel *c)
> static inline void set_channel_pending_send_size(struct vmbus_channel *c,
> u32 size)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&c->outbound.ring_lock, flags);
> +
> + if (size) {
> + ++c->out_full_total;
> +
> + if (!c->out_full_flag) {
> + ++c->out_full_first;
> + c->out_full_flag = true;
> + }
> + } else {
> + c->out_full_flag = false;
> + }
> +
> + spin_unlock_irqrestore(&c->outbound.ring_lock, flags);
> +
> c->outbound.ring_buffer->pending_send_sz = size;
> }
>
> --
> 2.17.1
Looks good to me.
Reviewed-by: Dexuan Cui <decui@...rosoft.com>
-- Dexuan
Powered by blists - more mailing lists