Re: [PATCH] tmpfs: don't interrupt fallocate with EINTR

From: Christian Brauner <brauner@kernel.org>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Hugh Dickins <hughd@google.com>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org,  linux-mm@kvack.org
Subject: Re: [PATCH] tmpfs: don't interrupt fallocate with EINTR
Date: Tue, 5 Mar 2024 11:10:47 +0100	[thread overview]
Message-ID: <20240305-zugunsten-busbahnhof-6dc705d80152@brauner> (raw)
In-Reply-To: <84acfa88-816f-50d7-50a2-92ea7a7db42@redhat.com>

On Tue, Mar 05, 2024 at 10:34:26AM +0100, Mikulas Patocka wrote:
> 
> 
> On Tue, 5 Mar 2024, Christian Brauner wrote:
> 
> > On Mon, Mar 04, 2024 at 07:43:39PM +0100, Mikulas Patocka wrote:
> > > 
> > > Index: linux-2.6/mm/shmem.c
> > > ===================================================================
> > > --- linux-2.6.orig/mm/shmem.c	2024-01-18 19:18:31.000000000 +0100
> > > +++ linux-2.6/mm/shmem.c	2024-03-04 19:05:25.000000000 +0100
> > > @@ -3143,7 +3143,7 @@ static long shmem_fallocate(struct file
> > >  		 * Good, the fallocate(2) manpage permits EINTR: we may have
> > >  		 * been interrupted because we are using up too much memory.
> > >  		 */
> > > -		if (signal_pending(current))
> > > +		if (fatal_signal_pending(current))
> > 
> > I think that's likely wrong and probably would cause regressions as
> > there may be users relying on this?
> 
> ext4 fallocate doesn't return -EINTR. So, userspace code can't rely on it.

I'm confused what does this have to do with ext4 since this is about
tmpfs. Also note, that fallocate(2) documents EINTR as a valid return
value. And fwiw, the manpage also states that "EINTR  A signal was
caught during execution; see signal(7)." not a "fatal signal".

Aside from that. If a user sends SIGUSR1 then with the code as it is now
that fallocate call will be interrupted. With your change that SIGUSR1
won't do anything anymore. Instead userspace would need to send SIGKILL.
So userspace that uses SIGUSR1 will suddenly hang.