diff mbox

[v3,4/4] firmware: send -EINTR on signal abort on fallback mechanism

Message ID 20170629205151.5329-5-mcgrof@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Luis Chamberlain June 29, 2017, 8:51 p.m. UTC
Right now we send -EAGAIN to a syfs write which got interrupted.
Userspace can't tell what happened though, send -EINTR if we
were killed due to a signal so userspace can tell things apart.

This is only applicable to the fallback mechanism.

Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_class.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Greg Kroah-Hartman July 17, 2017, 2 p.m. UTC | #1
On Thu, Jun 29, 2017 at 01:51:51PM -0700, Luis R. Rodriguez wrote:
> Right now we send -EAGAIN to a syfs write which got interrupted.
> Userspace can't tell what happened though, send -EINTR if we
> were killed due to a signal so userspace can tell things apart.
> 
> This is only applicable to the fallback mechanism.
> 
> Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  drivers/base/firmware_class.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

This doesn't need to go into 4.13-final, right?  Can it wait for
4.14-rc1?

thanks,

greg k-h
Luis Chamberlain July 17, 2017, 4:04 p.m. UTC | #2
On Mon, Jul 17, 2017 at 04:00:07PM +0200, Greg KH wrote:
> On Thu, Jun 29, 2017 at 01:51:51PM -0700, Luis R. Rodriguez wrote:
> > Right now we send -EAGAIN to a syfs write which got interrupted.
> > Userspace can't tell what happened though, send -EINTR if we
> > were killed due to a signal so userspace can tell things apart.
> > 
> > This is only applicable to the fallback mechanism.
> > 
> > Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > ---
> >  drivers/base/firmware_class.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> This doesn't need to go into 4.13-final, right? 

Nope.

> Can it wait for 4.14-rc1?

Yes. The bigger issue was the unexpected signals causing interruption.
This I consider a secondary and not critical issue so I did not Cc stable.

  Luis
Greg Kroah-Hartman July 17, 2017, 4:20 p.m. UTC | #3
On Mon, Jul 17, 2017 at 06:04:33PM +0200, Luis R. Rodriguez wrote:
> On Mon, Jul 17, 2017 at 04:00:07PM +0200, Greg KH wrote:
> > On Thu, Jun 29, 2017 at 01:51:51PM -0700, Luis R. Rodriguez wrote:
> > > Right now we send -EAGAIN to a syfs write which got interrupted.
> > > Userspace can't tell what happened though, send -EINTR if we
> > > were killed due to a signal so userspace can tell things apart.
> > > 
> > > This is only applicable to the fallback mechanism.
> > > 
> > > Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
> > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > > ---
> > >  drivers/base/firmware_class.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > This doesn't need to go into 4.13-final, right? 
> 
> Nope.
> 
> > Can it wait for 4.14-rc1?
> 
> Yes. The bigger issue was the unexpected signals causing interruption.
> This I consider a secondary and not critical issue so I did not Cc stable.

Ok, can you then take your two series of patches, and split them up into
one for 4.13-final and one for 4.14-rc1 so that I know for sure which is
which?  As it is, I have to guess :(

thanks,

greg k-h
Luis Chamberlain July 18, 2017, 12:27 a.m. UTC | #4
On Mon, Jul 17, 2017 at 06:20:37PM +0200, Greg KH wrote:
> On Mon, Jul 17, 2017 at 06:04:33PM +0200, Luis R. Rodriguez wrote:
> > On Mon, Jul 17, 2017 at 04:00:07PM +0200, Greg KH wrote:
> > > On Thu, Jun 29, 2017 at 01:51:51PM -0700, Luis R. Rodriguez wrote:
> > > > Right now we send -EAGAIN to a syfs write which got interrupted.
> > > > Userspace can't tell what happened though, send -EINTR if we
> > > > were killed due to a signal so userspace can tell things apart.
> > > > 
> > > > This is only applicable to the fallback mechanism.
> > > > 
> > > > Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
> > > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > > > ---
> > > >  drivers/base/firmware_class.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > This doesn't need to go into 4.13-final, right? 
> > 
> > Nope.
> > 
> > > Can it wait for 4.14-rc1?
> > 
> > Yes. The bigger issue was the unexpected signals causing interruption.
> > This I consider a secondary and not critical issue so I did not Cc stable.
> 
> Ok, can you then take your two series of patches, and split them up into
> one for 4.13-final and one for 4.14-rc1 so that I know for sure which is
> which?
>
> As it is, I have to guess :(

Sure thing.

  Luis
diff mbox

Patch

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index cff33fb609e7..d3d071dbd2a5 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1086,9 +1086,12 @@  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 = -EINTR;
+		else
+			retval = -EAGAIN;
+	} else if (buf->is_paged_buf && !buf->data)
 		retval = -ENOMEM;
 
 	device_del(f_dev);