| Message ID | 20240515221044.590-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tmpfs: don't interrupt fallocate with EINTR | expand |
On Thu, May 16, 2024 at 12:10:44AM +0200, Jan Kara wrote: > From: Mikulas Patocka <mpatocka@redhat.com> > > I have a program that sets up a periodic timer with 10ms interval. When > the program attempts to call fallocate(2) on tmpfs, it goes into an > infinite loop. fallocate(2) takes longer than 10ms, so it gets > interrupted by a signal and it returns EINTR. On EINTR, the fallocate > call is restarted, going into the same loop again. > > Let's change the signal_pending() check in shmem_fallocate() loop to > fatal_signal_pending(). This solves the problem of shmem_fallocate() > constantly restarting. Since most other filesystem's fallocate methods > don't react to signals, it is unlikely userspace really relies on timely > delivery of non-fatal signals while fallocate is running. Also the > comment before the signal check: > > /* > * Good, the fallocate(2) manpage permits EINTR: we may have > * been interrupted because we are using up too much memory. > */ > > indicates that the check was mainly added for OOM situations in which > case the process will be sent a fatal signal so this change preserves > the behavior in OOM situations. > > [JK: Update changelog and comment based on upstream discussion] > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
On Thu, 16 May 2024 00:10:44 +0200, Jan Kara wrote: > I have a program that sets up a periodic timer with 10ms interval. When > the program attempts to call fallocate(2) on tmpfs, it goes into an > infinite loop. fallocate(2) takes longer than 10ms, so it gets > interrupted by a signal and it returns EINTR. On EINTR, the fallocate > call is restarted, going into the same loop again. > > Let's change the signal_pending() check in shmem_fallocate() loop to > fatal_signal_pending(). This solves the problem of shmem_fallocate() > constantly restarting. Since most other filesystem's fallocate methods > don't react to signals, it is unlikely userspace really relies on timely > delivery of non-fatal signals while fallocate is running. Also the > comment before the signal check: > > [...] Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc [1/1] tmpfs: don't interrupt fallocate with EINTR https://git.kernel.org/vfs/vfs/c/f113ef08b6bd
diff --git a/mm/shmem.c b/mm/shmem.c index 1f84a41aeb85..9c148f9723f4 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3167,10 +3167,13 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset, struct folio *folio; /* - * Good, the fallocate(2) manpage permits EINTR: we may have - * been interrupted because we are using up too much memory. + * Check for fatal signal so that we abort early in OOM + * situations. We don't want to abort in case of non-fatal + * signals as large fallocate can take noticeable time and + * e.g. periodic timers may result in fallocate constantly + * restarting. */ - if (signal_pending(current)) + if (fatal_signal_pending(current)) error = -EINTR; else if (shmem_falloc.nr_unswapped > shmem_falloc.nr_falloced) error = -ENOMEM;