Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751578AbdFFRyT (ORCPT ); Tue, 6 Jun 2017 13:54:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:49697 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751432AbdFFRyQ (ORCPT ); Tue, 6 Jun 2017 13:54:16 -0400 Date: Tue, 6 Jun 2017 19:54:13 +0200 From: "Luis R. Rodriguez" To: linux-fsdevel@vger.kernel.org Cc: Alan Cox , fsdevel@vger.kernel.org, "Luis R. Rodriguez" , Stephen Boyd , "Li, Yi" , Dmitry Torokhov , Peter Zijlstra , Jonathan Corbet , "Eric W. Biederman" , "Michael Kerrisk (man-pages)" , Andy Lutomirski , Greg KH , "Fuzzey, Martin" , Linux API , Daniel Wagner , David Woodhouse , jewalt@lgsinnovations.com, rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , atull@opensource.altera.com, Moritz Fischer , Petr Mladek , Johannes Berg , Emmanuel Grumbach , Luca Coelho , Kalle Valo , Linus Torvalds , Kees Cook , AKASHI Takahiro , David Howells , Peter Jones , Hans de G oede , "Ted Ts'o" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback Message-ID: <20170606175413.GF27288@wotan.suse.de> References: <87fufr3mdy.fsf@xmission.com> <20170526194640.GS8951@wotan.suse.de> <20170526215518.GB40877@dtor-ws> <20170605202410.GQ8951@wotan.suse.de> <1496760796.5682.48.camel@linux.intel.com> <20170606164734.GB27288@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170606164734.GB27288@wotan.suse.de> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2116 Lines: 60 Using the right linux-fsdevel this time also, this was the second reply. Luis On Tue, Jun 06, 2017 at 06:47:34PM +0200, Luis R. Rodriguez wrote: > 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 > -- Luis Rodriguez, SUSE LINUX GmbH Maxfeldstrasse 5; D-90409 Nuernberg