public inbox for ltp@lists.linux.it
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox