All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] kernel/irq: Add irqbalance01
Date: Mon, 30 Aug 2021 21:26:35 +0200	[thread overview]
Message-ID: <YS0w6y6wxn6CjLur@pevik> (raw)
In-Reply-To: <20210824101042.11772-1-rpalethorpe@suse.com>

Hi Richie,

LGTM, only few nits found bellow (easily fixed before merge)

...
> +++ b/testcases/kernel/irq/.gitignore
> @@ -0,0 +1 @@
> +irqbalance01
/irqbalance01

> diff --git a/testcases/kernel/irq/Makefile b/testcases/kernel/irq/Makefile
> new file mode 100644
> index 000000000..085e06fac
> --- /dev/null
> +++ b/testcases/kernel/irq/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir		?= ../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +INSTALL_TARGETS		:= *.sh
This should be removed (probably copy paste error
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/irq/irqbalance01.c b/testcases/kernel/irq/irqbalance01.c
> new file mode 100644
> index 000000000..0a476839c
> --- /dev/null
> +++ b/testcases/kernel/irq/irqbalance01.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> */
> +/*\
> + * [Description]
> + *
> + * Check that something (e.g. irqbalance daemon) is performing IRQ
> + * load balancing.
> + *
> + * On most systems userland needs to set /proc/irq/$IRQ/smp_affinity
> + * to prevent many IRQs being delivered to the same CPU.
> + *
> + * Note some drivers and IRQ controllers will distribute IRQs
> + * evenly. Some systems will have housekeeping CPUs configured. Some
> + * IRQs can not be masked etc. So this test is not appropriate for all
> + * scenarios.
> + *
> + * Furthermore, exactly how IRQs should be distributed is a
> + * performance and/or security issue. This is only a generic smoke
> + * test. It will hopefully detect misconfigured systems and total
> + * balancing failures which are often silent errors.
> + *
> + * Heuristic: Evidence of Change
Add blank new line here to get better docparse formatting:

> + * 1. Find IRQs with a non-zero count
> + * 2. Check if they are now disallowed
LGTM. It'd be interesting to hear opinion of IRQ subsystem maintainer Thomas Gleixner.

> + *
> + * There are two sources of information we need to parse:
Also here.

> + * 1. /proc/interrupts
> + * 2. /proc/irq/$IRQ/smp_affinity
> + *
> + * We get the active IRQs and CPUs from /proc/interrupts. It also
> + * contains the per-CPU IRQ counts and info we do not care about.
> + *
> + * We get the IRQ masks from each active IRQ's smp_affinity file. This
> + * is a bitmask written out in hexidecimal format. It shows which CPUs
                                  ^ hexadecimal
> + * an IRQ may be recieved by.
                    ^ received
> + */
> +
> +#include <stdlib.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_stdio.h"
> +#include "tst_safe_file_at.h"
> +
> +enum affinity {
> +	ALLOW,
> +	DENY,
> +};
> +
> +static unsigned int *irq_stats;
> +static enum affinity *irq_affinity;
> +
> +static unsigned int nr_cpus;
> +static unsigned int nr_irqs;
> +static unsigned int *irq_ids;
> +
> +static void collect_irq_info(void)
> +{
> +	char *buf = NULL, *c, *first_row;
> +	char path[PATH_MAX];
> +	size_t size = 1024;
> +	size_t ret, row, col;
> +	long acc;
> +	unsigned int cpu_total, mask_pos;
> +	int fd = SAFE_OPEN("/proc/interrupts", O_RDONLY);
> +
> +	nr_cpus = 0;
> +	nr_irqs = 0;
> +
> +	do {
> +		size *= 2;
> +		buf = SAFE_REALLOC(buf, size);
> +		SAFE_LSEEK(fd, SEEK_SET, 0);
		SAFE_LSEEK(fd, 0, SEEK_SET);
(it works only because SEEK_SET is also 0)

> +		ret = SAFE_READ(0, fd, buf, size - 1);
> +	} while (ret >= size - 1);
...

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

  reply	other threads:[~2021-08-30 19:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 10:10 [LTP] [PATCH] kernel/irq: Add irqbalance01 Richard Palethorpe
2021-08-30 19:26 ` Petr Vorel [this message]
2021-08-31  9:39   ` Richard Palethorpe
2021-08-31 11:44     ` [LTP] [PATCH v2] " Richard Palethorpe
2021-08-31 12:01       ` Petr Vorel
2021-09-09 12:41       ` Cyril Hrubis
2021-09-09 12:41         ` Cyril Hrubis
2021-09-09 14:09       ` Cyril Hrubis
2021-09-09 14:09         ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YS0w6y6wxn6CjLur@pevik \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.