Messages in this thread | | | Date | Tue, 6 Jun 2017 18:47:34 +0200 | From | "Luis R. Rodriguez" <> | Subject | 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: > > "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.
> > 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
| |