From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: "Matthew Wilcox" <willy@infradead.org>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Pedro Falcato" <pfalcato@suse.de>,
"Liam R. Howlett" <Liam.Howlett@oracle.com>,
"Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"David Hildenbrand" <david@redhat.com>,
amd-gfx@lists.freedesktop.org, linux-mm@kvack.org,
"Vlastimil Babka" <vbabka@suse.cz>,
"Jann Horn" <jannh@google.com>,
stable@vger.kernel.org
Subject: Re: [PATCH v4 1/2] mm_take_all_locks: change -EINTR to -ERESTARTSYS
Date: Thu, 8 Jan 2026 15:25:36 +0000 [thread overview]
Message-ID: <d76d80a8-64a0-43f8-ae68-8b6461a349b5@lucifer.local> (raw)
In-Reply-To: <d3d77df6-931a-b97c-d551-a69ee5ca9493@redhat.com>
On Wed, Jan 07, 2026 at 10:29:44PM +0100, Mikulas Patocka wrote:
>
>
> On Wed, 7 Jan 2026, Matthew Wilcox wrote:
>
> > On Wed, Jan 07, 2026 at 09:31:14PM +0100, Mikulas Patocka wrote:
> > > This commit changes -EINTR to -ERESTARTSYS, so that if the signal handler
> > > was installed with the SA_RESTART flag, the operation is automatically
> > > restarted.
> >
> > No, this is bonkers. If you get a signal, you return -EINTR.
>
> Why?
>
> fifo_open returns -ERESTARTSYS, so why not here?
This is fundamentally changing what this does for all callers, and we've
simply not encountered this issue elsewhere.
Given how long it might take to work it might be interrupted by another
signal on the next attempt, whose to say that it might not result in a
situation where it can never complete?
I thnk it should stay as it is.
Also the comment above literally has:
* mm_take_all_locks() and mm_drop_all_locks are expensive operations
* that may have to take thousand of locks.
*
* mm_take_all_locks() can fail if it's interrupted by signals.
*/
Which at any rate would need to be changed even if we were to change it.
I'm more and more inclined to say let's just drop this series in general,
and you should go fix how signals are handled in the driver/userland code.
This is really feeling like the wrong place to fix this.
>
> Mikulas
>
Cheers, Lorenzo
next prev parent reply other threads:[~2026-01-08 15:25 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-07 20:31 [PATCH v4 0/2] Fix failures with signals and AMD OpenCL Mikulas Patocka
2026-01-07 20:31 ` [PATCH v4 1/2] mm_take_all_locks: change -EINTR to -ERESTARTSYS Mikulas Patocka
2026-01-07 20:55 ` Matthew Wilcox
2026-01-07 21:29 ` Mikulas Patocka
2026-01-08 15:25 ` Lorenzo Stoakes [this message]
2026-01-08 15:33 ` Liam R. Howlett
2026-01-07 20:31 ` [PATCH v4 2/2] amdgpu: delete the "Failed to register MMU notifier" message Mikulas Patocka
2026-01-08 15:20 ` Lorenzo Stoakes
2026-01-08 15:30 ` Liam R. Howlett
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=d76d80a8-64a0-43f8-ae68-8b6461a349b5@lucifer.local \
--to=lorenzo.stoakes@oracle.com \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=david@redhat.com \
--cc=jannh@google.com \
--cc=linux-mm@kvack.org \
--cc=mpatocka@redhat.com \
--cc=pfalcato@suse.de \
--cc=stable@vger.kernel.org \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/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