Re: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback

Adding fsdevel folks.

On Tue, Jun 06, 2017 at 03:53:16PM +0100, Alan Cox wrote:

quoted

"Unix tradition (and thus almost all applications) believe file store
writes to
be non signal interruptible. It would not be safe or practical to
change that
guarantee."

Yep everyone codes

	write(disk_file, "foo", 3);

not while(..) blah around it.

Thanks for the confirmation! That's a simple enough explanation.

quoted

For these two reasons then it would seem best we do two things
actually:

1) return -EINTR instead of -EAGAIN when we detect
swait_event_interruptible_timeout()
got interrupted by a signal (it returns -ERESTARTSYS)
2) Do as you note below and add wait_event_killable_timeout()

Pedantic detail that I don't think affects you

If you have completed a part of the I/O then you should return the byte
processed count not EINTR, but -1,EINTR if no progress was made.

You are right with some new exceptions and with regards to the future:

The syfs loading interface for firmware currently goes through the
data file exposed on syfs, the respective write op firmware_data_write()
only checks for signals at the beginning. After that its a full one
swoop try to write if you are following the old tradition and are using
a buffer allocated by the firmware API.

If you are using the relatively new request_firmware_into_buf() added
by Stephen Boyd which lets the driver provide the allocated buffer then
we have a loop in firmware_rw() which should be fixed to:

1) Check for signals
2) Do what you noted above.

Furthermore Yi Li over at Intel is adding some new API calls which would
re-use some of this for FPGA firmwares which are also very large, that
work should consider the above and fix appropriately as well.

  Luis