From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
To: Andy Lutomirski <luto@amacapital.net>
CC: Sam Protsenko <semen.protsenko@linaro.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        "Fleming, Matt" <matt.fleming@intel.com>,
        "Ong, Boon Leong" <boon.leong.ong@intel.com>,
        Ming Lei <ming.lei@canonical.com>,
        "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>
Subject: RE: [PATCH v2 3/3] efi: Capsule update with user helper interface
Thread-Topic: [PATCH v2 3/3] efi: Capsule update with user helper interface
Thread-Index: AQHP9+iuHUs9oVBcgkKjHBOxgX5ANZxPaE0AgACL8cD//4fqAIAAgjoAgAAK9YCAAAv/gIAAD3kAgANofuD//8DLgIAGNTzQ
Date: Mon, 10 Nov 2014 08:31:56 +0000
Message-ID: <F54AEECA5E2B9541821D670476DAE19C2B851058@PGSMSX102.gar.corp.intel.com>
References: <1414984030-13859-1-git-send-email-hock.leong.kweh@intel.com>
 <1414984030-13859-4-git-send-email-hock.leong.kweh@intel.com>
 <20141104043247.GA23418@kroah.com>
 <CALCETrXjJju+cMWJfB6W5kSD=Ox5y+Hj_AfmbbbOTOh8CV8eaQ@mail.gmail.com>
 <F54AEECA5E2B9541821D670476DAE19C2B80EAE4@PGSMSX102.gar.corp.intel.com>
 <CALCETrU2wk_FdcD6E+oMcp-tF97zN7f+w7q5rfPWBaNKrrmxVg@mail.gmail.com>
 <1415110688.26277.36.camel@mfleming-mobl1.ger.corp.intel.com>
 <CALCETrW1UaOCcb-32SOUhi_Zh5vLzV+VwyffThVvso23z04SHA@mail.gmail.com>
 <20141104154017.GA28113@kroah.com>
 <CALCETrV6M5D4iPjxPwNkMWTfVYDf3wSuZ9P23miUSR-jMGRyvg@mail.gmail.com>
 <F54AEECA5E2B9541821D670476DAE19C2B80F3A9@PGSMSX102.gar.corp.intel.com>
 <CALCETrUi0ssZbOpHyVmh6jx68Oj=0h4BwfM0uLio8Qbaeu9gJw@mail.gmail.com>
In-Reply-To: <CALCETrUi0ssZbOpHyVmh6jx68Oj=0h4BwfM0uLio8Qbaeu9gJw@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit

> -----Original Message-----
> From: Andy Lutomirski [mailto:luto@amacapital.net]
> > #!/bin/sh
> >
> > old=$(cat
> > /sys/devices/platform/efi_capsule_user_helper/capsule_loaded)
> >
> > for arg in "$@"
> > do
> >         if [ -f $arg ]
> >         then
> >                 echo 1 > /sys/class/firmware/efi-capsule-file/loading
> >                 cat $arg > /sys/class/firmware/efi-capsule-file/data
> >                 echo 0 > /sys/class/firmware/efi-capsule-file/loading
> 
> I think you have a race.  Try putting msleep(1000) after the
> request_firmware_nowait call, and I bet this will fail on the second try.

Sorry for the late response. I don't really catch the race condition that
you are referring? Are you trying to tell that the user script could run faster
before the previous callback function actually end? Will such scenario happen?
In the callback function, after the request_firmware_nowait(), I don't have
any codes will delay the callback function to end. Besides, there is a mutex_lock
protecting the request_firmware_nowait() calling. Won't that take care of the
issue?

> 
> >
> >                 oldtime=$(date +%S)
> >                 oldtime=$(((time + 2) % 60))
> >                 until [ -f /sys/class/firmware/efi-capsule-file/loading ]
> >                 do
> >                         newtime=$(date +%S)
> >                         if [ $newtime -eq $oldtime ]
> >                         then
> >                                 break
> >                         fi
> >                 done
> >
> >                 old=$((old + 1))
> >                 new=$(cat
> > /sys/devices/platform/efi_capsule_user_helper/capsule_loaded)
> 
> I think that firmware_class doesn't call the callback until after loading is closed
> for the second time.  If so, then this is racy.  Try inserting msleep(1000) at the
> beginning of your callback and uploading a capsule that should load
> successfully -- this will report failure, but a future upload may get very
> confused. Also, what does the firmware class do when simultaneous
> uploads of the same file with different contents are in flight?  Is that possible?

Sorry again, I can't really catch you on this race condition statement. Are you
trying to tell if user is doing this:

echo 1 > /sys/class/firmware/efi-capsule-file/loading
cat capsule1 > /sys/class/firmware/efi-capsule-file/data
cat capsule2 > /sys/class/firmware/efi-capsule-file/data
echo 0 > /sys/class/firmware/efi-capsule-file/loading

If so, capsule2 will be the one we will obtain in the callback function.


Regards,
Wilson

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?