bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 #31789

oleksandr-pavlyk · 2022-03-09T19:10:04Z

In Linux kernel 5.14 one can dynamically request size of altstacksize
needed to handle signals based on hardware capabilities.
This patch sets stack size to be the maximum of current value of SIGSTKSZ * 2 and
SIGSTKSZ + getauxval(AT_MINSIGSTKSZ).

This changes allows for Python extension's request to Linux kernel
to use AMX_TILE instruction set on Sapphire Rapids Xeon processor
to succeed, unblocking use of the ISA in frameworks.

This change proposes a solution to the problem described
in https://bugs.python.org/msg414809

https://bugs.python.org/issue46968

the-knights-who-say-ni · 2022-03-09T19:10:08Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@oleksandr-pavlyk

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

oleksandr-pavlyk · 2022-03-09T19:37:10Z

The CLA has been signed. I am the first time contributor to CPython proper. Please guide me through the process of getting this PR to the required state to make the merge possible. Thank you.

AlexWaygood · 2022-03-09T19:52:40Z

The CLA has been signed. I am the first time contributor to CPython proper. Please guide through the process of getting this PR to the required state to make the merge possible. Thank you.

Thanks for signing the CLA! It may take a few days for the bot to update your PR with the "CLA signed" label, as the form needs to be manually reviewed by a human at the PSF :)

The first step is to change the title of your PR to "bpo-46968: Query altstack size dynamically on Linux kernel >=5.14". This will cause the bots to automatically link this PR to that BPO issue, which will make the relevant CI check go green.

The second thing you'll need to do is to add a NEWS entry to your PR, which you can do using https://blurb-it.herokuapp.com.

AlexWaygood · 2022-03-09T20:01:38Z

You'll probably also need to add a test to your PR to make it mergeable.

For a fuller guide to the process, the devguide is here :)

oleksandr-pavlyk · 2022-03-09T23:29:50Z

You'll probably also need to add a test to your PR to make it mergeable.

Since size of size of sigaltstack used by faulthandler extensions is not user accessible to the best of my knowledge, I am not entirely sure how once can test the change.

Suggestions are welcome.

AlexWaygood · 2022-03-09T23:32:22Z

Suggestions are welcome.

I'm afraid this is an area I know next to nothing about, so I can't really help you any further I'm afraid :/

Don't worry too much about it — if it seems like a good idea, a core developer with more expertise in this area should be able to help you with writing tests 🙂 (though there's quite a large PR backlog, so it might take a while).

gpshead

The core of the changes look good, the annoying part is in the generated configure and aclocal.m4 files - those should be generated with the same version as the existing ones.

See the 2022 comment on https://bugs.python.org/issue44035 for a pointer to a docker image that can be used for the autoconf regeneration stuff. (that image is not yet documented AFAICT)

aclocal.m4

bedevere-bot · 2022-03-10T08:45:48Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

configure.ac

oleksandr-pavlyk · 2022-03-10T12:39:41Z

I have made the requested changes; please review again

I modified configure.ac as requested by @tiran

I regenerated configure and aclocal.m4 by rerunning autoreconf using

bash-4.4$ autoreconf --version
autoreconf (GNU Autoconf) 2.69
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+/Autoconf: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>, <http://gnu.org/licenses/exceptions.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David J. MacKenzie and Akim Demaille.

Assuming this change eventually gets accepted, what is the process of requesting backports to all earlier versions of CPython?

bedevere-bot · 2022-03-10T12:39:44Z

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

tiran

You need a verify specific combination of autoconf 2.69 + runstatedir patch, autoarchive version, and pkg-config m4 macro version. Your version of autoconf aclocal macros is too old. If you cannot use the container image to regenerate the files, then apply the changes manually.

revert aclocal.m4 and configure changes.
manually add linux/auxvec.h to configure

bedevere-bot · 2022-03-10T12:49:08Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Introduced HAVE_LINUX_AUXVEC_H in configure.ac and pyconfig.h.in Used cpython_autoconf:269 docker container to generate configure

oleksandr-pavlyk · 2022-03-11T11:15:00Z

I have made the requested changes; please review again

bedevere-bot · 2022-03-11T11:15:02Z

Thanks for making the requested changes!

@tiran, @gpshead: please review the changes made to this pull request.

Misc/NEWS.d/next/Library/2022-03-10-14-51-11.bpo-46968.ym2QxL.rst

Modules/faulthandler.c

In Linux kernel 5.14 one can dynamically request size of altstacksize based on hardware capabilities. This patch sets stack size to be the maximum of current value of SIGSTKSZ * 2 and SIGSTKSZ + getauxval(AT_MINSIGSTKSZ) This changes allows for Python extension's request to Linux kernel to use AMX_TILE instruction set on Sapphire Rapids Xeon processor to succeed, unblocking use of the ISA in frameworks.

oleksandr-pavlyk · 2022-03-11T12:16:28Z

I have made the requested changes; please review again

bedevere-bot · 2022-03-11T12:16:30Z

Thanks for making the requested changes!

@tiran, @gpshead: please review the changes made to this pull request.

vstinner

LGTM.

vstinner · 2022-03-11T16:51:08Z

Modules/faulthandler.c

+       Linux kernel version >= 5.14 */
+    unsigned long at_minstack_size = getauxval(AT_MINSIGSTKSZ);
+    if (at_minstack_size != 0) {
+        stack.ss_size = SIGSTKSZ + at_minstack_size;


I'm surprised that AT_MINSIGSTKSZ doesn't return directly the value which should be passed to sigaltstack(), but it seems like your code is the correct way to use the value.

The AT_MINSIGSTKSZ is a preprocessor variable. The getauxval checks hardware capabilities (e.g. total size of CPU state to be saved at context switches) and may return different values on different platforms.

I mean that I expected to write (simplified code without error handling): stack.ss_size = getauxval(AT_MINSIGSTKSZ), rather than stack.ss_size = SIGSTKSZ + getauxval(AT_MINSIGSTKSZ). But your code looks correct according to the doc that I found on the Internet. Like the official Linux documentation:

ss.ss_size = getauxval(AT_MINSIGSTKSZ) + SIGSTKSZ;

ref: https://docs.kernel.org/x86/elf_auxvec.html#introduction

vstinner · 2022-03-11T16:59:38Z

I wrote PR #31824 to add os.sysconf_names['SC_MINSIGSTKSZ'].

miss-islington · 2022-03-11T22:19:38Z

Thanks @oleksandr-pavlyk for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington · 2022-03-11T22:19:42Z

Sorry, @oleksandr-pavlyk and @vstinner, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 3b128c054885fe881c3b57a5978de3ea89c81a9c 3.10

vstinner · 2022-03-11T22:19:49Z

Merged, thank you!

bedevere-bot · 2022-03-11T22:22:54Z

GH-31830 is a backport of this pull request to the 3.10 branch.

vstinner · 2022-03-11T22:23:31Z

I fixed the conflict on configure.ac and backported the change to 3.10: #31830 Then I will backport the fix to 3.9.

…1830) In Linux kernel 5.14 one can dynamically request size of altstacksize based on hardware capabilities with getauxval(AT_MINSIGSTKSZ). This changes allows for Python extension's request to Linux kernel to use AMX_TILE instruction set on Sapphire Rapids Xeon processor to succeed, unblocking use of the ISA in frameworks. Introduced HAVE_LINUX_AUXVEC_H in configure.ac and pyconfig.h.in Used cpython_autoconf:269 docker container to generate configure. (cherry picked from commit 3b128c0) Co-authored-by: Oleksandr Pavlyk <[email protected]>

…1831) In Linux kernel 5.14 one can dynamically request size of altstacksize based on hardware capabilities with getauxval(AT_MINSIGSTKSZ). This changes allows for Python extension's request to Linux kernel to use AMX_TILE instruction set on Sapphire Rapids Xeon processor to succeed, unblocking use of the ISA in frameworks. Introduced HAVE_LINUX_AUXVEC_H in configure.ac and pyconfig.h.in Used cpython_autoconf:269 docker container to generate configure. (cherry picked from commit 3b128c0) Co-authored-by: Oleksandr Pavlyk <[email protected]>

…pythonGH-31831) In Linux kernel 5.14 one can dynamically request size of altstacksize based on hardware capabilities with getauxval(AT_MINSIGSTKSZ). This changes allows for Python extension's request to Linux kernel to use AMX_TILE instruction set on Sapphire Rapids Xeon processor to succeed, unblocking use of the ISA in frameworks. Introduced HAVE_LINUX_AUXVEC_H in configure.ac and pyconfig.h.in Used cpython_autoconf:269 docker container to generate configure. (cherry picked from commit 3b128c0) Co-authored-by: Oleksandr Pavlyk <[email protected]>

bedevere-bot added the awaiting review label Mar 9, 2022

the-knights-who-say-ni added the CLA not signed label Mar 9, 2022

oleksandr-pavlyk changed the title ~~Query altstack size dynamically on Linux kernel >=5.14~~ bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 Mar 9, 2022

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Mar 9, 2022

gpshead requested changes Mar 10, 2022

View reviewed changes

aclocal.m4 Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting review labels Mar 10, 2022

gpshead self-assigned this Mar 10, 2022

gpshead requested a review from tiran March 10, 2022 08:46

gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.10 only security fixes labels Mar 10, 2022

tiran reviewed Mar 10, 2022

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

oleksandr-pavlyk force-pushed the altstack-control branch from f8c09d0 to 854a577 Compare March 10, 2022 12:37

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 10, 2022

bedevere-bot requested a review from gpshead March 10, 2022 12:39

tiran requested changes Mar 10, 2022

View reviewed changes

bedevere-bot removed the awaiting change review label Mar 10, 2022

bedevere-bot added the awaiting changes label Mar 10, 2022

Added check for linux/auxvec.h in configure

3a32f59

Introduced HAVE_LINUX_AUXVEC_H in configure.ac and pyconfig.h.in Used cpython_autoconf:269 docker container to generate configure

oleksandr-pavlyk force-pushed the altstack-control branch from d3e2045 to 570a176 Compare March 10, 2022 22:32

vstinner reviewed Mar 11, 2022

View reviewed changes

Misc/NEWS.d/next/Library/2022-03-10-14-51-11.bpo-46968.ym2QxL.rst Outdated Show resolved Hide resolved

Modules/faulthandler.c Outdated Show resolved Hide resolved

Modules/faulthandler.c Outdated Show resolved Hide resolved

oleksandr-pavlyk and others added 2 commits March 11, 2022 04:11

📜🤖 Added by blurb_it.

2a2cbd6

oleksandr-pavlyk force-pushed the altstack-control branch from 570a176 to 2a2cbd6 Compare March 11, 2022 12:12

vstinner approved these changes Mar 11, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 11, 2022

vstinner mentioned this pull request Mar 11, 2022

bpo-46968: Add os.sysconf_names['SC_MINSIGSTKSZ'] #31824

Merged

vstinner merged commit 3b128c0 into python:main Mar 11, 2022

bedevere-bot removed the awaiting merge label Mar 11, 2022

miss-islington assigned vstinner and unassigned gpshead Mar 11, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 11, 2022

oleksandr-pavlyk deleted the altstack-control branch March 12, 2022 00:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 #31789

bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 #31789

oleksandr-pavlyk commented Mar 9, 2022 •

edited by bedevere-bot

Loading

the-knights-who-say-ni commented Mar 9, 2022

oleksandr-pavlyk commented Mar 9, 2022 •

edited

Loading

AlexWaygood commented Mar 9, 2022 •

edited

Loading

AlexWaygood commented Mar 9, 2022

oleksandr-pavlyk commented Mar 9, 2022

AlexWaygood commented Mar 9, 2022 •

edited

Loading

gpshead left a comment

bedevere-bot commented Mar 10, 2022

oleksandr-pavlyk commented Mar 10, 2022

bedevere-bot commented Mar 10, 2022

tiran left a comment

bedevere-bot commented Mar 10, 2022

oleksandr-pavlyk commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

oleksandr-pavlyk commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

vstinner left a comment

vstinner Mar 11, 2022

oleksandr-pavlyk Mar 11, 2022

vstinner Mar 11, 2022

vstinner commented Mar 11, 2022

miss-islington commented Mar 11, 2022

miss-islington commented Mar 11, 2022

vstinner commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

vstinner commented Mar 11, 2022

bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 #31789

bpo-46968: Query altstack size dynamically on Linux kernel >=5.14 #31789

Conversation

oleksandr-pavlyk commented Mar 9, 2022 • edited by bedevere-bot Loading

the-knights-who-say-ni commented Mar 9, 2022

CLA Missing

oleksandr-pavlyk commented Mar 9, 2022 • edited Loading

AlexWaygood commented Mar 9, 2022 • edited Loading

AlexWaygood commented Mar 9, 2022

oleksandr-pavlyk commented Mar 9, 2022

AlexWaygood commented Mar 9, 2022 • edited Loading

gpshead left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 10, 2022

oleksandr-pavlyk commented Mar 10, 2022

bedevere-bot commented Mar 10, 2022

tiran left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 10, 2022

oleksandr-pavlyk commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

oleksandr-pavlyk commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

vstinner left a comment

Choose a reason for hiding this comment

vstinner Mar 11, 2022

Choose a reason for hiding this comment

oleksandr-pavlyk Mar 11, 2022

Choose a reason for hiding this comment

vstinner Mar 11, 2022

Choose a reason for hiding this comment

vstinner commented Mar 11, 2022

miss-islington commented Mar 11, 2022

miss-islington commented Mar 11, 2022

vstinner commented Mar 11, 2022

bedevere-bot commented Mar 11, 2022

vstinner commented Mar 11, 2022

oleksandr-pavlyk commented Mar 9, 2022 •

edited by bedevere-bot

Loading

oleksandr-pavlyk commented Mar 9, 2022 •

edited

Loading

AlexWaygood commented Mar 9, 2022 •

edited

Loading

AlexWaygood commented Mar 9, 2022 •

edited

Loading