[PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Grazvydas Ignotas]

Missing uevent_suppress is causing two uevents instead of one, which is
confusing udev and sometimes causing firmware load to fail due to race
condition, so let's add it.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 compat/compat_firmware_class.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/compat/compat_firmware_class.c b/compat/compat_firmware_class.c
index ce937a3..54ee8b9 100644
--- a/compat/compat_firmware_class.c
+++ b/compat/compat_firmware_class.c
@@ -432,6 +432,8 @@ static int fw_register_device(struct device **dev_p, const char *fw_name,
 	dev_set_drvdata(f_dev, fw_priv);
 #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
 	dev_set_uevent_suppress(f_dev, 1);
+#else
+	f_dev->uevent_suppress = 1;
 #endif
 	retval = device_register(f_dev);
 	if (retval) {
@@ -479,9 +481,11 @@ static int fw_setup_device(struct firmware *fw, struct device **dev_p,
 		goto error_unreg;
 	}
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
 	if (uevent)
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
 		dev_set_uevent_suppress(f_dev, 0);
+#else
+		f_dev->uevent_suppress = 0;
 #endif
 	*dev_p = f_dev;
 	goto out;
-- 
1.7.0.2

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Walter Goldens]

I reported this nearly a month ago here: http://comments.gmane.org/gmane.linux.kernel.wireless.general/48401

Had a chance to test your patch and everything's fine now. This bug has been existent since January. Great job!


Walter.

--- On Thu, 4/22/10, Grazvydas Ignotas <notasas@gmail.com> wrote:

> From: Grazvydas Ignotas <notasas@gmail.com>
> Subject: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels
> To: "Luis R. Rodriguez" <mcgrof@gmail.com>
> Cc: linux-wireless@vger.kernel.org, "Grazvydas Ignotas" <notasas@gmail.com>
> Date: Thursday, April 22, 2010, 9:54 AM
> Missing uevent_suppress is causing
> two uevents instead of one, which is
> confusing udev and sometimes causing firmware load to fail
> due to race
> condition, so let's add it.
> 
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> ---
>  compat/compat_firmware_class.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/compat/compat_firmware_class.c
> b/compat/compat_firmware_class.c
> index ce937a3..54ee8b9 100644
> --- a/compat/compat_firmware_class.c
> +++ b/compat/compat_firmware_class.c
> @@ -432,6 +432,8 @@ static int fw_register_device(struct
> device **dev_p, const char *fw_name,
>      dev_set_drvdata(f_dev, fw_priv);
>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>      dev_set_uevent_suppress(f_dev, 1);
> +#else
> +    f_dev->uevent_suppress = 1;
>  #endif
>      retval = device_register(f_dev);
>      if (retval) {
> @@ -479,9 +481,11 @@ static int fw_setup_device(struct
> firmware *fw, struct device **dev_p,
>          goto error_unreg;
>      }
>  
> -#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>      if (uevent)
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>         
> dev_set_uevent_suppress(f_dev, 0);
> +#else
> +       
> f_dev->uevent_suppress = 0;
>  #endif
>      *dev_p = f_dev;
>      goto out;
> -- 
> 1.7.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


      

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Luis R. Rodriguez]

On Thu, Apr 22, 2010 at 6:54 AM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> Missing uevent_suppress is causing two uevents instead of one, which is
> confusing udev and sometimes causing firmware load to fail due to race
> condition, so let's add it.
>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> ---
>  compat/compat_firmware_class.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/compat/compat_firmware_class.c b/compat/compat_firmware_class.c
> index ce937a3..54ee8b9 100644
> --- a/compat/compat_firmware_class.c
> +++ b/compat/compat_firmware_class.c
> @@ -432,6 +432,8 @@ static int fw_register_device(struct device **dev_p, const char *fw_name,
>        dev_set_drvdata(f_dev, fw_priv);
>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>        dev_set_uevent_suppress(f_dev, 1);
> +#else
> +       f_dev->uevent_suppress = 1;
>  #endif

How was this compiling for older kernels before then?

  Luis

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Pavel Roskin]

On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:

> How was this compiling for older kernels before then?

Compiling was fine, but loading the firmware was failing sometimes.  I'm
glad somebody figured it out.  Thank you, Grazvydas!

I would prefer to have replacement functions in compat-2.6.30.h rather
than ifdefs in the code.  We might want to resync compat_firmware with
the kernel one day, and ifdefs would stand in the way.

-- 
Regards,
Pavel Roskin

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Luis R. Rodriguez]

On Thu, Apr 22, 2010 at 9:42 AM, Pavel Roskin <proski@gnu.org> wrote:
> On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:
>
>> How was this compiling for older kernels before then?
>
> Compiling was fine, but loading the firmware was failing sometimes.  I'm
> glad somebody figured it out.  Thank you, Grazvydas!

OK Applied. I'm still puzzled, if it compiled, then that means
dev_set_uevent_suppress() was being defined somehow for older kernels.

> I would prefer to have replacement functions in compat-2.6.30.h rather
> than ifdefs in the code.  We might want to resync compat_firmware with
> the kernel one day, and ifdefs would stand in the way.

Good point, I merged the patch for now given that that file does't
change that often upstream.

  Luis

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Grazvydas Ignotas]

On Thu, Apr 22, 2010 at 11:33 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Thu, Apr 22, 2010 at 9:42 AM, Pavel Roskin <proski@gnu.org> wrote:
>> On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:
>>
>>> How was this compiling for older kernels before then?
>>
>> Compiling was fine, but loading the firmware was failing sometimes.  I'm
>> glad somebody figured it out.  Thank you, Grazvydas!
>
> OK Applied. I'm still puzzled, if it compiled, then that means
> dev_set_uevent_suppress() was being defined somehow for older kernels.

there was already ifdef, old code:
 #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
       dev_set_uevent_suppress(f_dev, 1);
 #endif

new code:
 #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
       dev_set_uevent_suppress(f_dev, 1);
#else
       f_dev->uevent_suppress = 1;
 #endif

>> I would prefer to have replacement functions in compat-2.6.30.h rather
>> than ifdefs in the code.  We might want to resync compat_firmware with
>> the kernel one day, and ifdefs would stand in the way.
>
> Good point, I merged the patch for now given that that file does't
> change that often upstream.
>
>  Luis
>

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Johannes Berg]

On Thu, 2010-04-22 at 23:47 +0300, Grazvydas Ignotas wrote:
> On Thu, Apr 22, 2010 at 11:33 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> > On Thu, Apr 22, 2010 at 9:42 AM, Pavel Roskin <proski@gnu.org> wrote:
> >> On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:
> >>
> >>> How was this compiling for older kernels before then?
> >>
> >> Compiling was fine, but loading the firmware was failing sometimes.  I'm
> >> glad somebody figured it out.  Thank you, Grazvydas!
> >
> > OK Applied. I'm still puzzled, if it compiled, then that means
> > dev_set_uevent_suppress() was being defined somehow for older kernels.
> 
> there was already ifdef, old code:
>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>        dev_set_uevent_suppress(f_dev, 1);
>  #endif
> 
> new code:
>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>        dev_set_uevent_suppress(f_dev, 1);
> #else
>        f_dev->uevent_suppress = 1;
>  #endif

And the argument is that the new code should be

	dev_set_uevent_suppress(f_dev, 1);

and some header file should provide

static inline void dev_set_uevent_suppress(...)
{
	f_dev->uevent_suppress = 1;
}

which makes perfect sense. Although Luis merged it, a fix would be
useful.

Luis, a patch changing firmware_class.c just entered the tree :P I made
it.

johannes

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Luis R. Rodriguez]

On Thu, Apr 22, 2010 at 10:57 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2010-04-22 at 23:47 +0300, Grazvydas Ignotas wrote:
>> On Thu, Apr 22, 2010 at 11:33 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> > On Thu, Apr 22, 2010 at 9:42 AM, Pavel Roskin <proski@gnu.org> wrote:
>> >> On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:
>> >>
>> >>> How was this compiling for older kernels before then?
>> >>
>> >> Compiling was fine, but loading the firmware was failing sometimes.  I'm
>> >> glad somebody figured it out.  Thank you, Grazvydas!
>> >
>> > OK Applied. I'm still puzzled, if it compiled, then that means
>> > dev_set_uevent_suppress() was being defined somehow for older kernels.
>>
>> there was already ifdef, old code:
>>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>>        dev_set_uevent_suppress(f_dev, 1);
>>  #endif
>>
>> new code:
>>  #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30))
>>        dev_set_uevent_suppress(f_dev, 1);
>> #else
>>        f_dev->uevent_suppress = 1;
>>  #endif
>
> And the argument is that the new code should be
>
>        dev_set_uevent_suppress(f_dev, 1);
>
> and some header file should provide
>
> static inline void dev_set_uevent_suppress(...)
> {
>        f_dev->uevent_suppress = 1;
> }
>
> which makes perfect sense. Although Luis merged it, a fix would be
> useful.
>
> Luis, a patch changing firmware_class.c just entered the tree :P I made
> it.

Damn you, had to be you :)

  Luis

Re: [PATCH] compat: fix uevent_suppress on 2.6.29 or older kernels

[Pavel Roskin]

On Thu, 2010-04-22 at 13:33 -0700, Luis R. Rodriguez wrote:
> On Thu, Apr 22, 2010 at 9:42 AM, Pavel Roskin <proski@gnu.org> wrote:
> > On Thu, 2010-04-22 at 09:33 -0700, Luis R. Rodriguez wrote:
> >
> >> How was this compiling for older kernels before then?
> >
> > Compiling was fine, but loading the firmware was failing sometimes.  I'm
> > glad somebody figured it out.  Thank you, Grazvydas!
> 
> OK Applied. I'm still puzzled, if it compiled, then that means
> dev_set_uevent_suppress() was being defined somehow for older kernels.

It's wasn't defined.  The function just wasn't called for older kernels.
"#if" was there; the patch added "#else".

It's a great example how simply-minded commenting out missing functions
can lead to a hard problem, a race condition.

-- 
Regards,
Pavel Roskin