* [PATCH -mm 0/2] aio: simplify/fix the usage of restore_saved_sigmask_unless()
@ 2019-06-07 10:31 Oleg Nesterov
2019-06-07 10:31 ` [PATCH 1/2] aio: simplify " Oleg Nesterov
2019-06-07 10:32 ` [PATCH 2/2] aio: fix " Oleg Nesterov
0 siblings, 2 replies; 7+ messages in thread
From: Oleg Nesterov @ 2019-06-07 10:31 UTC (permalink / raw)
To: Andrew Morton, Benjamin LaHaise
Cc: Arnd Bergmann, David Laight, Deepa Dinamani, Eric W. Biederman,
Eric Wong, Linus Torvalds, linux-aio, linux-kernel
Absolutely untested. Benjamin, please review.
This depends on
signal-remove-the-wrong-signal_pending-check-in-restore_user_sigmask.patch
signal-simplify-set_user_sigmask-restore_user_sigmask.patch
Oleg.
fs/aio.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] aio: simplify the usage of restore_saved_sigmask_unless()
2019-06-07 10:31 [PATCH -mm 0/2] aio: simplify/fix the usage of restore_saved_sigmask_unless() Oleg Nesterov
@ 2019-06-07 10:31 ` Oleg Nesterov
2019-06-07 17:33 ` Linus Torvalds
2019-06-07 10:32 ` [PATCH 2/2] aio: fix " Oleg Nesterov
1 sibling, 1 reply; 7+ messages in thread
From: Oleg Nesterov @ 2019-06-07 10:31 UTC (permalink / raw)
To: Andrew Morton, Benjamin LaHaise
Cc: Arnd Bergmann, David Laight, Deepa Dinamani, Eric W. Biederman,
Eric Wong, Linus Torvalds, linux-aio, linux-kernel
Move the signal_pending() check and restore_saved_sigmask_unless()
into do_io_getevents() and simplify the callers.
The only complication is that non-restartable io_getevents() and
io_getevents_time32() have to translate ERESTARTNOHAND into EINTR.
They do not need restore_saved_sigmask_unless(), but it is harmless
and WARN_ON(!TIF_SIGPENDING) will be usefule after the next patch.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/aio.c | 34 ++++++++--------------------------
1 file changed, 8 insertions(+), 26 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 8200f97..944eef7 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2034,6 +2034,7 @@ static long do_io_getevents(aio_context_t ctx_id,
{
ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
struct kioctx *ioctx = lookup_ioctx(ctx_id);
+ bool interrupted;
long ret = -EINVAL;
if (likely(ioctx)) {
@@ -2042,6 +2043,11 @@ static long do_io_getevents(aio_context_t ctx_id,
percpu_ref_put(&ioctx->users);
}
+ interrupted = signal_pending(current);
+ restore_saved_sigmask_unless(interrupted);
+ if (interrupted && !ret)
+ ret = -ERESTARTNOHAND;
+
return ret;
}
@@ -2072,7 +2078,7 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
return -EFAULT;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
- if (!ret && signal_pending(current))
+ if (ret == -ERESTARTNOHAND)
ret = -EINTR;
return ret;
}
@@ -2094,7 +2100,6 @@ SYSCALL_DEFINE6(io_pgetevents,
{
struct __aio_sigset ksig = { NULL, };
struct timespec64 ts;
- bool interrupted;
int ret;
if (timeout && unlikely(get_timespec64(&ts, timeout)))
@@ -2109,11 +2114,6 @@ SYSCALL_DEFINE6(io_pgetevents,
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
- interrupted = signal_pending(current);
- restore_saved_sigmask_unless(interrupted);
- if (interrupted && !ret)
- ret = -ERESTARTNOHAND;
-
return ret;
}
@@ -2129,7 +2129,6 @@ SYSCALL_DEFINE6(io_pgetevents_time32,
{
struct __aio_sigset ksig = { NULL, };
struct timespec64 ts;
- bool interrupted;
int ret;
if (timeout && unlikely(get_old_timespec32(&ts, timeout)))
@@ -2145,11 +2144,6 @@ SYSCALL_DEFINE6(io_pgetevents_time32,
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
- interrupted = signal_pending(current);
- restore_saved_sigmask_unless(interrupted);
- if (interrupted && !ret)
- ret = -ERESTARTNOHAND;
-
return ret;
}
@@ -2170,7 +2164,7 @@ SYSCALL_DEFINE5(io_getevents_time32, __u32, ctx_id,
return -EFAULT;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
- if (!ret && signal_pending(current))
+ if (ret == -ERESTARTNOHAND)
ret = -EINTR;
return ret;
}
@@ -2196,7 +2190,6 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents,
{
struct __compat_aio_sigset ksig = { NULL, };
struct timespec64 t;
- bool interrupted;
int ret;
if (timeout && get_old_timespec32(&t, timeout))
@@ -2211,11 +2204,6 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents,
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
- interrupted = signal_pending(current);
- restore_saved_sigmask_unless(interrupted);
- if (interrupted && !ret)
- ret = -ERESTARTNOHAND;
-
return ret;
}
@@ -2231,7 +2219,6 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents_time64,
{
struct __compat_aio_sigset ksig = { NULL, };
struct timespec64 t;
- bool interrupted;
int ret;
if (timeout && get_timespec64(&t, timeout))
@@ -2246,11 +2233,6 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents_time64,
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
- interrupted = signal_pending(current);
- restore_saved_sigmask_unless(interrupted);
- if (interrupted && !ret)
- ret = -ERESTARTNOHAND;
-
return ret;
}
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] aio: fix the usage of restore_saved_sigmask_unless()
2019-06-07 10:31 [PATCH -mm 0/2] aio: simplify/fix the usage of restore_saved_sigmask_unless() Oleg Nesterov
2019-06-07 10:31 ` [PATCH 1/2] aio: simplify " Oleg Nesterov
@ 2019-06-07 10:32 ` Oleg Nesterov
1 sibling, 0 replies; 7+ messages in thread
From: Oleg Nesterov @ 2019-06-07 10:32 UTC (permalink / raw)
To: Andrew Morton, Benjamin LaHaise
Cc: Arnd Bergmann, David Laight, Deepa Dinamani, Eric W. Biederman,
Eric Wong, Linus Torvalds, linux-aio, linux-kernel
do_io_getevents() is the only user of set/restore_user_sigmask which
can return success or error and deliver a signal which was temporary
unblocked by set_user_sigmask().
Change it to keep the modified sigmask (pass true to restore_unless)
only if the syscall returns ERESTARTNOHAND/EINTR. This matches all
other syscalls which modify current->blocked before wait-for-event.
Note that it doesn't even need the signal_pending() check, read_event()
wait_event_interruptible_hrtimeout() returns ERESTARTSYS if interrupted.
But it seems that read_events() needs some unrelated cleanups which
should be done first.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/aio.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 944eef7..5bdbef0 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2034,7 +2034,6 @@ static long do_io_getevents(aio_context_t ctx_id,
{
ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
struct kioctx *ioctx = lookup_ioctx(ctx_id);
- bool interrupted;
long ret = -EINVAL;
if (likely(ioctx)) {
@@ -2043,10 +2042,9 @@ static long do_io_getevents(aio_context_t ctx_id,
percpu_ref_put(&ioctx->users);
}
- interrupted = signal_pending(current);
- restore_saved_sigmask_unless(interrupted);
- if (interrupted && !ret)
+ if (!ret && signal_pending(current))
ret = -ERESTARTNOHAND;
+ restore_saved_sigmask_unless(ret == -ERESTARTNOHAND);
return ret;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] aio: simplify the usage of restore_saved_sigmask_unless()
2019-06-07 10:31 ` [PATCH 1/2] aio: simplify " Oleg Nesterov
@ 2019-06-07 17:33 ` Linus Torvalds
2019-06-07 17:37 ` Linus Torvalds
0 siblings, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2019-06-07 17:33 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Benjamin LaHaise, Arnd Bergmann, David Laight,
Deepa Dinamani, Eric W. Biederman, Eric Wong, linux-aio,
Linux List Kernel Mailing
On Fri, Jun 7, 2019 at 3:32 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> The only complication is that non-restartable io_getevents() and
> io_getevents_time32() have to translate ERESTARTNOHAND into EINTR.
Are they actually nonrestartable? I think the current EINTR is just a mistake.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] aio: simplify the usage of restore_saved_sigmask_unless()
2019-06-07 17:33 ` Linus Torvalds
@ 2019-06-07 17:37 ` Linus Torvalds
2019-06-07 18:02 ` Eric W. Biederman
0 siblings, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2019-06-07 17:37 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Benjamin LaHaise, Arnd Bergmann, David Laight,
Deepa Dinamani, Eric W. Biederman, Eric Wong, linux-aio,
Linux List Kernel Mailing
On Fri, Jun 7, 2019 at 10:33 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Are they actually nonrestartable? I think the current EINTR is just a mistake.
Oh, I guess they are, because of the relative timeout thing that
shouldn't reset to the original value.
And I don't think this is worth a ERESTAR_RESTARTTBLOCK.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] aio: simplify the usage of restore_saved_sigmask_unless()
2019-06-07 17:37 ` Linus Torvalds
@ 2019-06-07 18:02 ` Eric W. Biederman
2019-06-07 18:09 ` Linus Torvalds
0 siblings, 1 reply; 7+ messages in thread
From: Eric W. Biederman @ 2019-06-07 18:02 UTC (permalink / raw)
To: Linus Torvalds
Cc: Oleg Nesterov, Andrew Morton, Benjamin LaHaise, Arnd Bergmann,
David Laight, Deepa Dinamani, Eric Wong, linux-aio,
Linux List Kernel Mailing
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Jun 7, 2019 at 10:33 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Are they actually nonrestartable? I think the current EINTR is just a mistake.
>
> Oh, I guess they are, because of the relative timeout thing that
> shouldn't reset to the original value.
>
> And I don't think this is worth a ERESTAR_RESTARTTBLOCK.
Unless I am misreading things io_pgetevents isn't restartable
either and ERESTARTNOHAND is a bug in that case.
Is the bug going the other way?
Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] aio: simplify the usage of restore_saved_sigmask_unless()
2019-06-07 18:02 ` Eric W. Biederman
@ 2019-06-07 18:09 ` Linus Torvalds
0 siblings, 0 replies; 7+ messages in thread
From: Linus Torvalds @ 2019-06-07 18:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Oleg Nesterov, Andrew Morton, Benjamin LaHaise, Arnd Bergmann,
David Laight, Deepa Dinamani, Eric Wong, linux-aio,
Linux List Kernel Mailing
On Fri, Jun 7, 2019 at 11:03 AM Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> Unless I am misreading things io_pgetevents isn't restartable
> either and ERESTARTNOHAND is a bug in that case.
>
> Is the bug going the other way?
Yeah, I think you're right, and that should be EINTR too.
Or at least be conditional on whether it had a timeout or not. A NULL
timeout obviously _is_ restartable. But I don't think it's worth being
clever here.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-07 18:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 10:31 [PATCH -mm 0/2] aio: simplify/fix the usage of restore_saved_sigmask_unless() Oleg Nesterov
2019-06-07 10:31 ` [PATCH 1/2] aio: simplify " Oleg Nesterov
2019-06-07 17:33 ` Linus Torvalds
2019-06-07 17:37 ` Linus Torvalds
2019-06-07 18:02 ` Eric W. Biederman
2019-06-07 18:09 ` Linus Torvalds
2019-06-07 10:32 ` [PATCH 2/2] aio: fix " Oleg Nesterov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.