Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751515AbdFIB5d (ORCPT ); Thu, 8 Jun 2017 21:57:33 -0400 Received: from mail.kernel.org ([198.145.29.99]:48024 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751445AbdFIB5c (ORCPT ); Thu, 8 Jun 2017 21:57:32 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D33D423A02 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mcgrof@kernel.org MIME-Version: 1.0 In-Reply-To: References: <20170524205658.GK8951@wotan.suse.de> <20170524214027.7775-1-mcgrof@kernel.org> <20170607170858.GK27288@wotan.suse.de> <59383DDA.3040702@parkeon.com> From: "Luis R. Rodriguez" Date: Thu, 8 Jun 2017 18:57:09 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback To: Martin Fuzzey Cc: Linux FS Devel , Alan Cox , "Ted Ts'o" , Andy Lutomirski , Dmitry Torokhov , "Michael Kerrisk (man-pages)" , Linux API , Peter Zijlstra , Greg KH , Daniel Wagner , David Woodhouse , jewalt@lgsinnovations.com, rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , "Li, Yi" , 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 Goede , "linux-kernel@vger.kernel.org" , "Luis R. Rodriguez" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1870 Lines: 43 On Thu, Jun 8, 2017 at 6:10 PM, Luis R. Rodriguez wrote: > On Wed, Jun 7, 2017 at 10:54 AM, Martin Fuzzey wrote: >> On 07/06/17 19:08, Luis R. Rodriguez wrote: >>> >>> On Thu, May 25, 2017 at 10:28:38AM +0200, Fuzzey, Martin wrote: >>>> >>>> 1) Android init calls write() on the sysfs file >>>> 2) The sysfs .store() callback registered by a driver is called >>>> 3) The driver calls request_firmware() >>>> 4) request_firmware() sends the firmware load request to userspace and >>>> calls wait_for_completion_interruptible() >>> >>> Martin, just for completeness on documenting on the commit log of the next >>> swait proposed fix for this -- what signal did the process get from which >>> you >>> note the child dies below ? Exactly what in Android sent this signal ? >> >> >> Android didn't send the signal, the kernel did (SIGCHLD). >> >> Like this: >> >> 1) Android init (pid=1) fork()s (say pid=42) [this child process is totally >> unrelated to firmware loading] >> 2) Android init (pid=1) does a write() on a (driver custom) sysfs file which >> ends up calling request_firmware() kernel side >> 3) The firmware loading fallback mechanism is used, the request is sent to >> userspace and pid 1 waits in the kernel on wait_* >> 4) before firmware loading completes pid 42 dies (for any reason - in my >> case normal termination) Martin just to be clear, by "normal case termination" do you mean completing successfully ?? Ie the firmware actually did make it onto the device ? > Interesting, could one interpretation here be that the process > successfully finishing + the signal being sent beats out the timing of > the firmware_class syfs code detecting that the write completed ? > >> 5) Kernel delivers SIGCHLD to pid=1 to tell it a child has died, which >> causes -ERESTARTSYS to be returned from wait_* > > Luis