linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] firmware: fix fallback mechanism by ignoring SIGCHLD
@ 2017-06-14 22:20 Luis R. Rodriguez
  2017-06-14 22:20 ` [PATCH 1/4] test_firmware: add test case for SIGCHLD on sync fallback Luis R. Rodriguez
                   ` (5 more replies)
  0 siblings, 6 replies; 38+ messages in thread
From: Luis R. Rodriguez @ 2017-06-14 22:20 UTC (permalink / raw)
  To: gregkh
  Cc: mfuzzey, ebiederm, dmitry.torokhov, wagi, dwmw2, jewalt, rafal,
	arend.vanspriel, rjw, yi1.li, atull, moritz.fischer, pmladek,
	johannes.berg, emmanuel.grumbach, luciano.coelho, kvalo, luto,
	torvalds, keescook, takahiro.akashi, dhowells, pjones, hdegoede,
	alan, tytso, mtk.manpages, paul.gortmaker, mtosatti, mawilcox,
	linux-api, linux-fsdevel, linux-kernel, L

Martin reported an issue with Android where if sysfs is used to trigger a sync
fw load which *relies* on the fallback mechanism and a background job completes
while the trigger is ongoing in the foreground it will immediately fail the fw
request.  The issue can be observed in this simple test script using the
test_firmware driver:

	set -e
	/etc/init.d/udev stop
	modprobe test_firmware
	DIR=/sys/devices/virtual/misc/test_firmware
	echo 10 >/sys/class/firmware/timeout
	sleep 2 &
	echo -n "does-not-exist-file.bin" > "$DIR"/trigger_request

The background sleep triggers the SIGCHLD signal and we fail the firmware
request on the fallback mechanism. This was due to the type of wait used which
captures all signals, but we currently lack the killable swaits which would
only allow killing the fallback wait on SIGKILL. This adds the missing killable
swaits and fixes the firmware API to use it. This goes along with a test case to
demo the issue clearly and how its fixed afterwards.  Lastly, ensure to use
-EINTR when interrupted so callers can distinguish between an interrupted
failure and other types of failure.

As suggested I've tagged the addition of the killable swaits and the firmware
fix as stable. Between v4.0 and v4.10 the stable fix is to instead change
the firmware to use wait_for_completion_killable_timeout() as the firmware
API was only converted to swait as of v4.10.

The last patch must be applied only after the killable wait is applied to
ensure only SIGKILL triggers an interruption. Otherwise it has been observed
using -EINTR on other signals like SIGCHLD will trigger a restart of the call,
if a sysfs write() is used to trigger the sync firmware request. This might be
due what signal(7) says about using the SA_RESTART flag when write() is called,
in such cases the call will be automatically restarted after the signal handler
returns. The excemption here naturally seems to be on SIGKILL.

Note that although I *feared* this might implicate any use of non-killable waits
on other system calls, such as finit_module(), initial testing confirms this to
not be the case.  For instance replacing the echo with modprobe on a module
which does the same on init does not present the same issues. This could be due
to the special SA_RESTART flag case on write() as noted above and sysfs...
however, its not perfectly clear yet to me.

As usual these patches are on my linux-next git tree, they are on the
20170614-fw-fixes branch based on linux-next 20170614 [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170614-fw-fixes

Luis R. Rodriguez (4):
  test_firmware: add test case for SIGCHLD on sync fallback
  swait: add the missing killable swaits
  firmware: avoid invalid fallback aborts by using killable swait
  firmware: send -EINTR on signal abort on fallback mechanism

 drivers/base/firmware_class.c                   | 11 +++++----
 include/linux/swait.h                           | 25 ++++++++++++++++++++
 tools/testing/selftests/firmware/fw_fallback.sh | 31 +++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 4 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2017-07-12 21:33 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 22:20 [PATCH 0/4] firmware: fix fallback mechanism by ignoring SIGCHLD Luis R. Rodriguez
2017-06-14 22:20 ` [PATCH 1/4] test_firmware: add test case for SIGCHLD on sync fallback Luis R. Rodriguez
2017-06-14 22:20 ` [PATCH 2/4] swait: add the missing killable swaits Luis R. Rodriguez
     [not found]   ` <20170614222017.14653-3-mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-06-29 12:54     ` Greg KH
     [not found]       ` <20170629125402.GH26046-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2017-06-29 13:05         ` Thomas Gleixner
2017-06-29 13:35           ` Greg KH
2017-06-29 13:46             ` Thomas Gleixner
2017-06-29 16:13               ` Linus Torvalds
2017-06-29 16:31                 ` Matthew Wilcox
2017-06-29 17:29                   ` Luis R. Rodriguez
2017-06-29 17:40                 ` Davidlohr Bueso
2017-06-29 17:57                   ` Linus Torvalds
2017-06-29 18:33                     ` Davidlohr Bueso
     [not found]                       ` <20170629183339.GD3954-3dK4OQgjB4rH06JGZaSw0A@public.gmane.org>
2017-06-29 18:59                         ` Linus Torvalds
     [not found]                           ` <CA+55aFz8Mhx+A-g-5yOG-O1ZLRUR_fpeeA4iBNGH8EnDBZEdpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-29 19:40                             ` Luis R. Rodriguez
2017-06-29 19:44                               ` Luis R. Rodriguez
     [not found]                                 ` <20170629194455.GR21846-B4tOwbsTzaBolqkO4TVVkw@public.gmane.org>
2017-06-29 20:58                                   ` Jakub Kicinski
2017-06-29 22:50                                     ` Luis R. Rodriguez
2017-06-29 22:53                                       ` Jakub Kicinski
2017-06-29 23:00                                         ` Luis R. Rodriguez
2017-06-29 23:06                                           ` Jakub Kicinski
     [not found]                                       ` <20170629225003.GU21846-B4tOwbsTzaBolqkO4TVVkw@public.gmane.org>
2017-07-12 21:33                                         ` Luis R. Rodriguez
     [not found]                               ` <20170629194015.GQ21846-B4tOwbsTzaBolqkO4TVVkw@public.gmane.org>
2017-06-29 20:57                                 ` Linus Torvalds
2017-07-05  2:06                       ` Davidlohr Bueso
     [not found]                         ` <20170705020635.GD11168-3dK4OQgjB4rH06JGZaSw0A@public.gmane.org>
2017-07-07 19:58                           ` Linus Torvalds
2017-07-07 22:27                             ` Davidlohr Bueso
2017-07-07 22:48                               ` Linus Torvalds
2017-06-29 19:15                 ` Marcelo Tosatti
     [not found]                   ` <20170629191506.GB12368-I4X2Mt4zSy4@public.gmane.org>
2017-06-30  4:03                     ` Linus Torvalds
2017-06-30 11:55                       ` Marcelo Tosatti
2017-06-30 11:57                       ` Marcelo Tosatti
2017-06-30 17:30                       ` Krister Johansen
2017-06-14 22:20 ` [PATCH 3/4] firmware: avoid invalid fallback aborts by using killable swait Luis R. Rodriguez
2017-06-14 22:20 ` [PATCH 4/4] firmware: send -EINTR on signal abort on fallback mechanism Luis R. Rodriguez
2017-06-15  7:49 ` [PATCH 0/4] firmware: fix fallback mechanism by ignoring SIGCHLD Martin Fuzzey
     [not found] ` <20170614222017.14653-1-mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-06-26 21:19   ` Luis R. Rodriguez
2017-06-29 15:14   ` Greg KH
     [not found]     ` <20170629151442.GA4880-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2017-06-29 17:29       ` Luis R. Rodriguez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).