Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1648062 > unrolled thread
| Started by | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| First post | 2017-05-23 15:40 +0200 |
| Last post | 2017-05-25 00:10 +0200 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-05-23 15:40 +0200
Re: [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS Martin Fuzzey <mfuzzey@parkeon.com> - 2017-05-23 16:40 +0200
Re: [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-23 22:00 +0200
Re: [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-24 23:00 +0200
[PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-24 23:50 +0200
Re: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback Andy Lutomirski <luto@kernel.org> - 2017-05-25 00:10 +0200
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-05-23 15:40 +0200 |
| Subject | Re: [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS |
| Message-ID | <tKicz-3OV-43@gated-at.bofh.it> |
On Tue, May 23, 2017 at 03:16:07PM +0200, Martin Fuzzey wrote: > When -ERESTARTSYS is returned by wait_* due to a signal this should > be returned from request_firmware() so that the syscall may be > restarted if necessary. > > Without this it is not possible to distinguish the case of > request_firmware() failing due to a timeout with that due to a signal. > > In my particular case request_firmware() was being called from > a sysfs .store() callback and the writing process was the > Android init process which received a SIGCHLD causing > request_firmware() to fail. > > Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> > --- > drivers/base/firmware_class.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) Nice find, should this go to the stable kernels as well? thanks, greg k-h
[toc] | [next] | [standalone]
| From | Martin Fuzzey <mfuzzey@parkeon.com> |
|---|---|
| Date | 2017-05-23 16:40 +0200 |
| Message-ID | <tKj8C-4rO-11@gated-at.bofh.it> |
| In reply to | #1648062 |
On 23/05/17 15:31, Greg Kroah-Hartman wrote: > On Tue, May 23, 2017 at 03:16:07PM +0200, Martin Fuzzey wrote: >> When -ERESTARTSYS is returned by wait_* due to a signal this should >> be returned from request_firmware() so that the syscall may be >> restarted if necessary. >> >> >> Nice find, should this go to the stable kernels as well? Yes I think it should. I have already applied a similar patch to my 4.4 tree. The exact same patch won't apply since the code has changed a bit since. So I was planning on sending for -stable-4.4 once it's in mainline. Regards, Martin
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2017-05-23 22:00 +0200 |
| Message-ID | <tKo8i-7OL-11@gated-at.bofh.it> |
| In reply to | #1648142 |
On Tue, May 23, 2017 at 04:32:49PM +0200, Martin Fuzzey wrote:
> On 23/05/17 15:31, Greg Kroah-Hartman wrote:
> > On Tue, May 23, 2017 at 03:16:07PM +0200, Martin Fuzzey wrote:
> > > When -ERESTARTSYS is returned by wait_* due to a signal this should
> > > be returned from request_firmware() so that the syscall may be
> > > restarted if necessary.
> > >
> > >
> > > Nice find, should this go to the stable kernels as well?
>
> Yes I think it should.
Thanks for the patch !
Just a bit of more nose diving validating this.
We actually used to send -ENOMEM for a long time always, and now you are
special-casing to allow -ERESTARTSYS -- we we must ask ourselves -- why
not other errors ?
Let us consider what is upstream only please and focus on stable later.
We use:
static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
{
long ret;
ret = swait_event_interruptible_timeout(fw_st->wq,
__fw_state_is_done(READ_ONCE(fw_st->status)),
timeout);
if (ret != 0 && fw_st->status == FW_STATUS_ABORTED)
return -ENOENT;
if (!ret)
return -ETIMEDOUT;
return ret < 0 ? ret : 0;
}
What can swait_event_interruptible_timeout() return ? It can return the value
of a timeout or whatever __swait_event_interruptible_timeout() returns.
__swait_event_interruptible_timeout() in turn uses ___swait_event() as
follows:
#define __swait_event_interruptible_timeout(wq, condition, timeout) \
___swait_event(wq, ___wait_cond_timeout(condition), \
TASK_INTERRUPTIBLE, timeout, \
__ret = schedule_timeout(__ret))
So this ultimately use ___swait_event():
/* as per ___wait_event() but for swait, therefore "exclusive == 0" */
#define ___swait_event(wq, condition, state, ret, cmd) \
({ \
struct swait_queue __wait; \
long __ret = ret; \
\
INIT_LIST_HEAD(&__wait.task_list); \
for (;;) { \
long __int = prepare_to_swait_event(&wq, &__wait, state);\
\
if (condition) \
break; \
\
if (___wait_is_interruptible(state) && __int) { \
__ret = __int; \
break; \
} \
\
cmd; \
} \
finish_swait(&wq, &__wait); \
__ret; \
})
And prepare_to_swait_event() can return -ERESTARTSYS on signal_pending_state()
otherwise it returns 0 ! So indeed -ERESTARTSYS is possible.
But what about other errors ? Considering the above it would seem then we
actually can only get -ERESTARTSYS or whatever schedule_timeout() returns.
schedule_timeout() is documented indicating it returns 0 when the timer has
expired otherwise the remaining time in jiffies will be returned. It further
clarifies that the return value is guaranteed to be be non-negative. And
___wait_is_interruptible() is:
#define ___wait_is_interruptible(state) \
(!__builtin_constant_p(state) || \
state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE) \
So this piece of code:
\
if (___wait_is_interruptible(state) && __int) { \
__ret = __int; \
break; \
} \
Since ___wait_is_interruptible() is always true for us since we use
__swait_event_interruptible_timeout() the -ERESTARTSYS will always be sent if a
signal was sent.
In all this light then the patch is correct for upstream however let's consider
stable now. At first glance this seems like a fix for an old patch,
respectively commit 0542ad88fbdd81bb ("firmware loader: Fix
_request_firmware_load() return val for fw load abort" by Shuah Khan which was
merged since v3.17, *but* back then just used wait_for_completion() and ignored
any signals here, they were just not part of our semantics. But its important
to note then we always returned -ENOMEM before that patch and then at least
returned -EAGAIN in other cases. The next thing to note is commit
5d47ec02c37ea632398c ("firmware: Correct handling of fw_state_wait() return
value") by Bjorn Andersson. This took place *after* the swait changes. Bjorn
fixed an issue but also forgot to address the special case of -ERESTARTSYS,
given right below fw_state_wait_timeout() on _request_firmware_load() the
return value would be lost. It would seem Bjorn assumed the return value would
be propagated but did not notice the error special casing below which would
loose it.
So before and after Bjorn's changes we were still *trying* to propagate the
-ERESTARTSYS error but it was still lost.
The -ERESTARTSYS from signals was still something we were capturing even prior
to the swait changes, see the kernel commit prior to the swait changes (0430cafcc4fb
"firmware: drop bit ops in favor of simple state machine"), if you git blame there,
will find a series of commits with the -ERESTARTSYS handled... I can trace back
to commit 68ff2a00dbf ("firmware_loader: handle timeout via
wait_for_completion_interruptible_timeout()") as having the -ERESTARTSYS check but
it had lost that error on _request_firmware_load() due to the :
if (retval == -ERESTARTSYS || !retval) {
mutex_lock(&fw_lock);
fw_load_abort(fw_priv);
mutex_unlock(&fw_lock);
}
if (is_fw_load_aborted(buf))
retval = -EAGAIN;
else if (!buf->data)
retval = -ENOMEM;
As noted earlier the above piece of code lost the error because of
0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val for
fw load abort" which was merged since v3.17, but back then it *did not*
propagate the error. So it would be incorrect to say that your patch fixes
commit 0542ad88fbdd81bb by Shuah Khan... We'd have to ask ourselves when
such an error actually became relevant.
It would seem the -ERESTARTSYS signal took effect first via commit 0cb64249ca500
("firmware_loader: abort request if wait_for_completion is interrupted") added
upstream via v4.0 and since it was introduced the error code was lost given the
-EAGAIN overwrite added *earlier* by Shuah Khan when such error codes were not
even relevant. So prior to v4.0 were we not even aborting due to signals.
As such this as far as I can tell this is a fix for a fix for this commit.
So we should use:
Fixes: 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion is interrupted")
Cc: stable@vger.kernel.org # 4.0
Also a more reflective subject and commit log would be appreciated, you can add
my Acked-by given I have also now tested it with all the test drivers and
test scripts:
==========================================================================
firmware: fix sending -ERESTARTSYS due to signal on fallback
Commit 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion
is interrupted") added via 4.0 added support to abort the fallback mechanism
when a signal was detected and wait_for_completion_interruptible() returned
-ERESTARTSYS. Although the abort was effective we were unfortunately never
really propagating this error though and as such userspace could not know
why the abort happened.
The error code was always being lost to an even older change, commit
0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val
for fw load abort") by Shuah Khan which was merged since v3.17. Back then
though we never were capturing these signals or bailing on a signal. After
this change though only only -EAGAIN was being relayed back to userspace
on non-memory errors including signals trying to interrupt our fallback
process.
It only makes sense to fix capturing -ERESTARTSYS since we were capturing
the error but when it was actually effective, since commit 0cb64249ca500
("firmware_loader: abort request if wait_for_completion is interrupted").
Only distributions relying on the fallback mechanism are impacted. An
example issue is on Android, when request_firmware() is called through
the firmware fallback mechanism -- syfs write call, sysfs .store(), and
Android sends a SIGCHLD to fail the write call -- in such cases the fact
that we failed due to a signal is lost.
Fix this and ensure we propagate -ERESTARTSYS so that handlers can whether
or not to restart write calls.
Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Cc: stable@vger.kernel.org # 4.0
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
==========================================================================
> I have already applied a similar patch to my 4.4 tree.
You might also be intersted in commit 2e700f8d85975 ("firmware: fix usermode
helper fallback loading".
> The exact same patch won't apply since the code has changed a bit since.
>
> So I was planning on sending for -stable-4.4 once it's in mainline.
Appreciated, after this is merged of course.
Luis
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2017-05-24 23:00 +0200 |
| Message-ID | <tKLxU-7aj-11@gated-at.bofh.it> |
| In reply to | #1648378 |
On Tue, May 23, 2017 at 09:55:33PM +0200, Luis R. Rodriguez wrote:
> On Tue, May 23, 2017 at 04:32:49PM +0200, Martin Fuzzey wrote:
> > On 23/05/17 15:31, Greg Kroah-Hartman wrote:
> > > On Tue, May 23, 2017 at 03:16:07PM +0200, Martin Fuzzey wrote:
> > > > When -ERESTARTSYS is returned by wait_* due to a signal this should
> > > > be returned from request_firmware() so that the syscall may be
> > > > restarted if necessary.
> > > >
> > > >
> > > > Nice find, should this go to the stable kernels as well?
> >
> > Yes I think it should.
>
> Thanks for the patch !
>
> Just a bit of more nose diving validating this.
>
> We actually used to send -ENOMEM for a long time always, and now you are
> special-casing to allow -ERESTARTSYS -- we we must ask ourselves -- why
> not other errors ?
>
> Let us consider what is upstream only please and focus on stable later.
>
> We use:
>
> static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
> {
> long ret;
>
> ret = swait_event_interruptible_timeout(fw_st->wq,
> __fw_state_is_done(READ_ONCE(fw_st->status)),
> timeout);
> if (ret != 0 && fw_st->status == FW_STATUS_ABORTED)
> return -ENOENT;
> if (!ret)
> return -ETIMEDOUT;
>
> return ret < 0 ? ret : 0;
> }
>
> What can swait_event_interruptible_timeout() return ? It can return the value
> of a timeout or whatever __swait_event_interruptible_timeout() returns.
> __swait_event_interruptible_timeout() in turn uses ___swait_event() as
> follows:
>
> #define __swait_event_interruptible_timeout(wq, condition, timeout) \
> ___swait_event(wq, ___wait_cond_timeout(condition), \
> TASK_INTERRUPTIBLE, timeout, \
> __ret = schedule_timeout(__ret))
>
> So this ultimately use ___swait_event():
>
> /* as per ___wait_event() but for swait, therefore "exclusive == 0" */
> #define ___swait_event(wq, condition, state, ret, cmd) \
> ({ \
> struct swait_queue __wait; \
> long __ret = ret; \
> \
> INIT_LIST_HEAD(&__wait.task_list); \
> for (;;) { \
> long __int = prepare_to_swait_event(&wq, &__wait, state);\
> \
> if (condition) \
> break; \
> \
> if (___wait_is_interruptible(state) && __int) { \
> __ret = __int; \
> break; \
> } \
> \
> cmd; \
> } \
> finish_swait(&wq, &__wait); \
> __ret; \
> })
>
> And prepare_to_swait_event() can return -ERESTARTSYS on signal_pending_state()
> otherwise it returns 0 ! So indeed -ERESTARTSYS is possible.
>
> But what about other errors ? Considering the above it would seem then we
> actually can only get -ERESTARTSYS or whatever schedule_timeout() returns.
> schedule_timeout() is documented indicating it returns 0 when the timer has
> expired otherwise the remaining time in jiffies will be returned. It further
> clarifies that the return value is guaranteed to be be non-negative. And
> ___wait_is_interruptible() is:
>
> #define ___wait_is_interruptible(state) \
> (!__builtin_constant_p(state) || \
> state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE) \
>
> So this piece of code:
> \
> if (___wait_is_interruptible(state) && __int) { \
> __ret = __int; \
> break; \
> } \
>
> Since ___wait_is_interruptible() is always true for us since we use
> __swait_event_interruptible_timeout() the -ERESTARTSYS will always be sent if a
> signal was sent.
>
> In all this light then the patch is correct for upstream however let's consider
> stable now. At first glance this seems like a fix for an old patch,
> respectively commit 0542ad88fbdd81bb ("firmware loader: Fix
> _request_firmware_load() return val for fw load abort" by Shuah Khan which was
> merged since v3.17, *but* back then just used wait_for_completion() and ignored
> any signals here, they were just not part of our semantics. But its important
> to note then we always returned -ENOMEM before that patch and then at least
> returned -EAGAIN in other cases. The next thing to note is commit
> 5d47ec02c37ea632398c ("firmware: Correct handling of fw_state_wait() return
> value") by Bjorn Andersson. This took place *after* the swait changes. Bjorn
> fixed an issue but also forgot to address the special case of -ERESTARTSYS,
> given right below fw_state_wait_timeout() on _request_firmware_load() the
> return value would be lost. It would seem Bjorn assumed the return value would
> be propagated but did not notice the error special casing below which would
> loose it.
>
> So before and after Bjorn's changes we were still *trying* to propagate the
> -ERESTARTSYS error but it was still lost.
>
> The -ERESTARTSYS from signals was still something we were capturing even prior
> to the swait changes, see the kernel commit prior to the swait changes (0430cafcc4fb
> "firmware: drop bit ops in favor of simple state machine"), if you git blame there,
> will find a series of commits with the -ERESTARTSYS handled... I can trace back
> to commit 68ff2a00dbf ("firmware_loader: handle timeout via
> wait_for_completion_interruptible_timeout()") as having the -ERESTARTSYS check but
> it had lost that error on _request_firmware_load() due to the :
>
> if (retval == -ERESTARTSYS || !retval) {
> mutex_lock(&fw_lock);
> fw_load_abort(fw_priv);
> mutex_unlock(&fw_lock);
> }
>
> if (is_fw_load_aborted(buf))
> retval = -EAGAIN;
> else if (!buf->data)
> retval = -ENOMEM;
>
> As noted earlier the above piece of code lost the error because of
> 0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val for
> fw load abort" which was merged since v3.17, but back then it *did not*
> propagate the error. So it would be incorrect to say that your patch fixes
> commit 0542ad88fbdd81bb by Shuah Khan... We'd have to ask ourselves when
> such an error actually became relevant.
>
> It would seem the -ERESTARTSYS signal took effect first via commit 0cb64249ca500
> ("firmware_loader: abort request if wait_for_completion is interrupted") added
> upstream via v4.0 and since it was introduced the error code was lost given the
> -EAGAIN overwrite added *earlier* by Shuah Khan when such error codes were not
> even relevant. So prior to v4.0 were we not even aborting due to signals.
>
> As such this as far as I can tell this is a fix for a fix for this commit.
>
> So we should use:
>
> Fixes: 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion is interrupted")
> Cc: stable@vger.kernel.org # 4.0
>
> Also a more reflective subject and commit log would be appreciated, you can add
> my Acked-by given I have also now tested it with all the test drivers and
> test scripts:
>
> ==========================================================================
> firmware: fix sending -ERESTARTSYS due to signal on fallback
>
> Commit 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion
> is interrupted") added via 4.0 added support to abort the fallback mechanism
> when a signal was detected and wait_for_completion_interruptible() returned
> -ERESTARTSYS. Although the abort was effective we were unfortunately never
> really propagating this error though and as such userspace could not know
> why the abort happened.
>
> The error code was always being lost to an even older change, commit
> 0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val
> for fw load abort") by Shuah Khan which was merged since v3.17. Back then
> though we never were capturing these signals or bailing on a signal. After
> this change though only only -EAGAIN was being relayed back to userspace
> on non-memory errors including signals trying to interrupt our fallback
> process.
>
> It only makes sense to fix capturing -ERESTARTSYS since we were capturing
> the error but when it was actually effective, since commit 0cb64249ca500
> ("firmware_loader: abort request if wait_for_completion is interrupted").
>
> Only distributions relying on the fallback mechanism are impacted. An
> example issue is on Android, when request_firmware() is called through
> the firmware fallback mechanism -- syfs write call, sysfs .store(), and
> Android sends a SIGCHLD to fail the write call -- in such cases the fact
> that we failed due to a signal is lost.
>
> Fix this and ensure we propagate -ERESTARTSYS so that handlers can whether
> or not to restart write calls.
>
> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Cc: stable@vger.kernel.org # 4.0
> Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ==========================================================================
>
> > I have already applied a similar patch to my 4.4 tree.
>
> You might also be intersted in commit 2e700f8d85975 ("firmware: fix usermode
> helper fallback loading".
>
> > The exact same patch won't apply since the code has changed a bit since.
> >
> > So I was planning on sending for -stable-4.4 once it's in mainline.
>
> Appreciated, after this is merged of course.
I'll just send this with the commit log change myself.
Luis
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2017-05-24 23:50 +0200 |
| Subject | [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback |
| Message-ID | <tKMki-7HS-25@gated-at.bofh.it> |
| In reply to | #1649949 |
From: Martin Fuzzey <mfuzzey@parkeon.com>
Commit 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion
is interrupted") added via 4.0 added support to abort the fallback mechanism
when a signal was detected and wait_for_completion_interruptible() returned
-ERESTARTSYS. Although the abort was effective we were unfortunately never
really propagating this error though and as such userspace could not know
why the abort happened.
The error code was always being lost to an even older change, commit
0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val
for fw load abort") by Shuah Khan which was merged since v3.17. Back then
though we never were capturing these signals or bailing on a signal. After
this change though only only -EAGAIN was being relayed back to userspace
on non-memory errors including signals trying to interrupt our fallback
process.
It only makes sense to fix capturing -ERESTARTSYS since we were capturing
the error but when it was actually effective, since commit 0cb64249ca500
("firmware_loader: abort request if wait_for_completion is interrupted").
Only distributions relying on the fallback mechanism are impacted. An
example issue is on Android, when request_firmware() is called through
the firmware fallback mechanism -- syfs write call, sysfs .store(), and
Android sends a SIGCHLD to fail the write call -- in such cases the fact
that we failed due to a signal is lost.
Fix this and ensure we propagate -ERESTARTSYS so that handlers can
whether or not to restart write calls.
Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Cc: stable <stable@vger.kernel.org> # 4.0
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: gave the commit log some serious love]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
For those following drivers/base/firmware_class.c development -- I've pushed
out two new branches based on linux-next tag next-20170524, one is just the
driver-data API [0], and the other one just has this patch applied on top
of the driver-data API [1]. You'll want to use driver-data-stable if you are
working on the cache or the fallback mechanism since this fix does provide
a fix for the cache / fallback mechanism.
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170524-driver-data
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170524-driver-data-stable
Luis
drivers/base/firmware_class.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 7af430a2d656..77c0e0792c30 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1309,9 +1309,10 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
mutex_unlock(&fw_lock);
}
- if (fw_state_is_aborted(&buf->fw_st))
- retval = -EAGAIN;
- else if (buf->is_paged_buf && !buf->data)
+ if (fw_state_is_aborted(&buf->fw_st)) {
+ if (retval != -ERESTARTSYS)
+ retval = -EAGAIN;
+ } else if (buf->is_paged_buf && !buf->data)
retval = -ENOMEM;
device_del(f_dev);
--
2.10.2
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-05-25 00:10 +0200 |
| Subject | Re: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback |
| Message-ID | <tKMDD-84u-1@gated-at.bofh.it> |
| In reply to | #1649968 |
On Wed, May 24, 2017 at 2:40 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> From: Martin Fuzzey <mfuzzey@parkeon.com>
>
> Commit 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion
> is interrupted") added via 4.0 added support to abort the fallback mechanism
> when a signal was detected and wait_for_completion_interruptible() returned
> -ERESTARTSYS. Although the abort was effective we were unfortunately never
> really propagating this error though and as such userspace could not know
> why the abort happened.
Can you give a simple example of what's going on and why it matters?
ERESTARTSYS and friends are highly magical, and I'm not convinced that
allowing _request_firmware_load to return -ERESTARTSYS is actually a
good idea. What if there are system calls that can't handle this
style of restart that start being restarted as a result?
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web