scheduler: added default_irq_smp_affinity option #306

yarda · 2020-11-10T18:16:46Z

The option 'default_irq_smp_affinity' affects what will be written to the
/proc/irq/default_smp_affinity. The option supports the following values:

calc:
Content of the /proc/irq/default_smp_affinity will be calculated from
the 'isolated_cores'. Non-isolated cores are calculated as an inversion
of the 'isolated_cores'. Then the intersection of the non-isolated cores
and the previous content of the /proc/irq/default_smp_affinity is
written to the /proc/irq/default_smp_affinity. If the intersection is
empty set, then just the non-isolated cores are written to the
/proc/irq/default_smp_affinity. This behavior is the default if
option 'default_irq_smp_affinity' is not specified.

ignore:
The /proc/irq/default_smp_affinity is not touched by Tuned.

cpulist like e.g. '1,3-4':
The cpulist is unpacked and directly written to the
/proc/irq/default_smp_affinity

Examples:

[scheduler]
isolated_cores = 1, 2
default_irq_smp_affinity = calc

[scheduler]
isolated_cores = 1, 2
default_irq_smp_affinity = ignore

[scheduler]
isolated_cores = 1, 3
default_irq_smp_affinity = 2, 4

Resolves: rhbz#1896348

Signed-off-by: Jaroslav Škarvada jskarvad@redhat.com

packit-as-a-service · 2020-11-10T18:21:13Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo yum install -y dnf-plugins-core on RHEL 8
sudo dnf install -y dnf-plugins-core on Fedora
dnf copr enable packit/redhat-performance-tuned-306
And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

marcel-apf

Looks good to me, thanks!

jmencak · 2020-11-11T13:46:25Z

I know the comment above and commit message could be considered a sort of a documentation, but really, it is not. Can we start a practice of adding documentation to every new feature such as this directly in the doc/manual/... ?

yarda · 2020-11-11T13:56:51Z

I know the comment above and commit message could be considered a sort of a documentation, but really, it is not. Can we start a practice of adding documentation to every new feature such as this directly in the doc/manual/... ?

It's good idea, I will try to transform the commit message into the asciidoc text.

jmencak · 2020-11-11T15:27:30Z

I've tested the default behaviour is unchanged + calc/ignore/cpulist seem to work too. Rollback also works. After the docs are added, happy to "lgtm".

yarda · 2020-11-11T16:42:45Z

An attempt to document this change.

After more thinking about it, I think this is not the best way to go. We should probably write such documentation into the code (e.g. by using docstrings) and then automatically generate structured reference guide from it. But it will require more man power to kick this in.

yarda · 2020-11-11T16:45:42Z

We can keep the whole asciidoc documentation (and further extend it), but I think that the chapter regarding the plugins and their options/parameters should be auto generated. This will allow easy maintenance.

yarda · 2020-11-11T17:17:21Z

Maybe there are still some 'wording' problems in the documentation added by this commit, waiting for feedback.

jmencak

Thanks for the docs! Looks good, minor nits a documentation writer or a native English speaker should confirm.

jmencak · 2020-11-11T17:17:07Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+`scheduler`::
+Allows tuning of scheduling priorities, processes/threads/IRQs affinities, and CPU cores isolation.
+
+The `default_irq_smp_affinity` parameter controls what *Tuned* will write to the `/proc/irq/default_smp_affinity`.


Suggestion:
s/what Tuned will write/the values Tuned writes/
s/to the/to/
Even though the file is "unique" I believe 'the' should be dropped as it is is "name". A document person should review this.

jmencak · 2020-11-11T17:21:07Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+The following values are supported:
+
+--
+** `calc` content of the `/proc/irq/default_smp_affinity` will be calculated from the `isolated_cores` parameter.


s/of the/of/ (everywhere where you refer to the file :)

jmencak · 2020-11-11T17:22:36Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+
+--
+** `calc` content of the `/proc/irq/default_smp_affinity` will be calculated from the `isolated_cores` parameter.
+Non-isolated cores are calculated as an inversion of the `isolated_cores`. Then the intersection of the non-isolated cores


s/of the isolated_cores/of isolated_cores/ I don't see isolated_cores mentioned anywhere, therefore I'd drop it here.

It's mentioned on the previous line. I think the parameter isolated_cores should be described in its own paragraph preceding this text, but it's out of the context of this commit, so I didn't add it.

jmencak · 2020-11-11T17:24:02Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+** `calc` content of the `/proc/irq/default_smp_affinity` will be calculated from the `isolated_cores` parameter.
+Non-isolated cores are calculated as an inversion of the `isolated_cores`. Then the intersection of the non-isolated cores
+and the previous content of the `/proc/irq/default_smp_affinity` is written to the `/proc/irq/default_smp_affinity`.
+If the intersection is empty set, then just the non-isolated cores are written to the `/proc/irq/default_smp_affinity`.


s/is empty/is an empty/

jmencak · 2020-11-11T17:25:55Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+If the intersection is empty set, then just the non-isolated cores are written to the `/proc/irq/default_smp_affinity`.
+This behavior is the default if the parameter `default_irq_smp_affinity` is omitted.
+** `ignore` *Tuned* will not touch `/proc/irq/default_smp_affinity`.
+** cpulist like e.g. `1,3-4` the cpulist is unpacked and directly written to the `/proc/irq/default_smp_affinity`.


s/cpulist like e.g./for a cpulist such as/

It's list describing 'calc', 'ignore', and cpulist parameters and it's introduced by the sentence: 'The following values are supported:', that's why I didn't want to write 'for a cpulist' but just 'cpulist'. I added dashes.

I reformatted the text to use asciidoc description, but I am not sure whether it looks better, nor whether the syntax I used is correct :)

jmencak · 2020-11-11T17:27:55Z

@courtneypacheco , could you please go over the docs? Thank you!

yarda · 2020-11-11T17:56:10Z

I addressed most of the issues except those where I put comment.

jmencak

Just a minor nit. Looks good to me otherwise, thank you. I've asked Courtney to review this as a native speaker of English. You may want to give her a day or two before this merges. Thank you. /cc @courtneypacheco

jmencak · 2020-11-12T08:19:57Z

doc/manual/modules/performance/ref_available-tuned-plug-ins.adoc

+`ignore`::
+*Tuned* will not touch `/proc/irq/default_smp_affinity`.
+cpulist such as `1,3-4`::
+The cpulist is unpacked and directly written to `/proc/irq/default_smp_affinity`.


s/directly written/written directly/ ?

I don't know :) but I changed it.

The option 'default_irq_smp_affinity' affects what will be written to the /proc/irq/default_smp_affinity. The option supports the following values: calc: Content of the /proc/irq/default_smp_affinity will be calculated from the 'isolated_cores'. Non-isolated cores are calculated as an inversion of the 'isolated_cores'. Then the intersection of the non-isolated cores and the previous content of the /proc/irq/default_smp_affinity is written to the /proc/irq/default_smp_affinity. If the intersection is empty set, then just the non-isolated cores are written to the /proc/irq/default_smp_affinity. This behavior is the default if option 'default_irq_smp_affinity' is not specified. ignore: The /proc/irq/default_smp_affinity is not touched by Tuned. cpulist like e.g. '1,3-4': The cpulist is unpacked and directly written to the /proc/irq/default_smp_affinity Examples: [scheduler] isolated_cores = 1, 2 default_irq_smp_affinity = calc [scheduler] isolated_cores = 1, 2 default_irq_smp_affinity = ignore [scheduler] isolated_cores = 1, 3 default_irq_smp_affinity = 2, 4 Resolves: rhbz#1896348 Signed-off-by: Jaroslav Škarvada <jskarvad@redhat.com>

jmencak · 2020-11-12T11:17:09Z

The rendered asciidoc looks OK to me. Still /lgtm

jmencak · 2020-11-14T07:00:51Z

It is unlikely we are getting a native-speaker review of the docs within a reasonable timeframe. I believe this looks good and is good to go, @yarda

yarda · 2020-11-14T09:52:58Z

OK, thanks for info. I also think there is no point in blocking the merge due to it. We can update the text anytime later.

marcel-apf approved these changes Nov 11, 2020

View reviewed changes

yarda force-pushed the rhbz_1896348 branch from f82d375 to 4fbcfed Compare November 11, 2020 16:36

jmencak reviewed Nov 11, 2020

View reviewed changes

yarda force-pushed the rhbz_1896348 branch from 4fbcfed to 5ef998b Compare November 11, 2020 17:51

jmencak approved these changes Nov 12, 2020

View reviewed changes

yarda force-pushed the rhbz_1896348 branch from 5ef998b to 1b0573d Compare November 12, 2020 10:00

yarda merged commit 23dcbd7 into redhat-performance:master Nov 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scheduler: added default_irq_smp_affinity option #306

scheduler: added default_irq_smp_affinity option #306

yarda commented Nov 10, 2020

packit-as-a-service bot commented Nov 10, 2020

marcel-apf left a comment

jmencak commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak commented Nov 11, 2020 •

edited

yarda commented Nov 11, 2020

yarda commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak left a comment

jmencak Nov 11, 2020

jmencak Nov 11, 2020

jmencak Nov 11, 2020

yarda Nov 11, 2020

jmencak Nov 11, 2020

jmencak Nov 11, 2020

yarda Nov 11, 2020 •

edited

yarda Nov 11, 2020

jmencak commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak left a comment

jmencak Nov 12, 2020

yarda Nov 12, 2020

jmencak commented Nov 12, 2020

jmencak commented Nov 14, 2020 •

edited

yarda commented Nov 14, 2020

scheduler: added default_irq_smp_affinity option #306

scheduler: added default_irq_smp_affinity option #306

Conversation

yarda commented Nov 10, 2020

packit-as-a-service bot commented Nov 10, 2020

marcel-apf left a comment

Choose a reason for hiding this comment

jmencak commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak commented Nov 11, 2020 • edited

yarda commented Nov 11, 2020

yarda commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarda Nov 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmencak commented Nov 11, 2020

yarda commented Nov 11, 2020

jmencak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmencak commented Nov 12, 2020

jmencak commented Nov 14, 2020 • edited

yarda commented Nov 14, 2020

jmencak commented Nov 11, 2020 •

edited

yarda Nov 11, 2020 •

edited

jmencak commented Nov 14, 2020 •

edited