2010-10-23 18:14:15

by Hauke Mehrtens

[permalink] [raw]
Subject: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

CONFIG_NONE and CONFIG_PREEMPT_VOLUNTARY are not added by the last RT
patch. I have not found any references to CONFIG_NONE, probably
CONFIG_PREEMPT_NONE was meant, but that is also wrong like
CONFIG_PREEMPT_VOLUNTARY. These two options are also in the normal kernel
config system without the rt patch.

This patch only checks for KConfig options added by the rt patch, but a
rt-patch user could also select PREEMPT_NONE or PREEMPT_VOLUNTARY, and
compat-wireless will not build. I do not think this will hapen often, so leave
it like this.

This patch fixes build with all non rt-kernels.

CC: Blaise Gassend <[email protected]>
Signed-off-by: Hauke Mehrtens <[email protected]>
---
include/linux/compat-2.6.34.h | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/linux/compat-2.6.34.h b/include/linux/compat-2.6.34.h
index 1cfd6e5..f710d08 100644
--- a/include/linux/compat-2.6.34.h
+++ b/include/linux/compat-2.6.34.h
@@ -142,8 +142,7 @@ do { \

static inline void device_lock(struct device *dev)
{
-#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
- defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
+#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
mutex_lock(&dev->parent->mutex);
#else
down(&dev->sem);
@@ -152,8 +151,7 @@ static inline void device_lock(struct device *dev)

static inline int device_trylock(struct device *dev)
{
-#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
- defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
+#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
return mutex_trylock(&dev->mutex);
#else
return down_trylock(&dev->sem);
@@ -162,8 +160,7 @@ static inline int device_trylock(struct device *dev)

static inline void device_unlock(struct device *dev)
{
-#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
- defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
+#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
mutex_unlock(&dev->mutex);
#else
up(&dev->sem);
--
1.7.0.4



2010-10-23 18:14:20

by Hauke Mehrtens

[permalink] [raw]
Subject: [PATCH 3/3] compat: add empty implementation for usb_enable_autosuspend

This is needed by drivers/bluetooth/btusb.c

Signed-off-by: Hauke Mehrtens <[email protected]>
---
include/linux/compat-2.6.34.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/compat-2.6.34.h b/include/linux/compat-2.6.34.h
index 763abca..93fc4ef 100644
--- a/include/linux/compat-2.6.34.h
+++ b/include/linux/compat-2.6.34.h
@@ -6,6 +6,7 @@
#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,34))

#include <linux/netdevice.h>
+#include <linux/usb.h>

#define netdev_mc_count(dev) ((dev)->mc_count)
#define netdev_mc_empty(dev) (netdev_mc_count(dev) == 0)
@@ -236,6 +237,12 @@ static inline int dma_set_coherent_mask(struct device *dev, u64 mask)
return 0;
}

+/* USB autosuspend and autoresume */
+static inline int usb_enable_autosuspend(struct usb_device *udev)
+{ return 0; }
+static inline int usb_disable_autosuspend(struct usb_device *udev)
+{ return 0; }
+
#endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,34)) */

#endif /* LINUX_26_34_COMPAT_H */
--
1.7.0.4


2010-10-25 19:25:11

by Blaise Gassend

[permalink] [raw]
Subject: Re: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

Correct, as far as I know. However, after I saw the parent->mutex bug,
I did some digging, and found that in compat-wireless only one Atheros
driver, which I do not use, actually uses those function calls (and
doesn't use the trylock one). So I can't say that this code has
actually been run.

On Mon, Oct 25, 2010 at 9:49 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Sun, Oct 24, 2010 at 1:41 PM, Blaise Gassend <[email protected]> wrote:
>> Hi Hauke,
>>
>>> in the Ubuntu 10.04 default kernel config CONFIG_PREEMPT_VOLUNTARY=y is
>>> set, but it does not use the RT patches, so compat-wireless build
>>> failed. If you are using a mainline kernel you can choose between
>>> CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT (all
>>> localed in kernel/Kconfig.preempt for a least kernel 2.6.24), so these
>>> options are not indicating that the RT patches are applied. The RT patch
>>> adds CONFIG_PREEMPT_RT and CONFIG_PREEMPT_DESKTOP and removes
>>> CONFIG_PREEMPT, so choosing one of these options to detect that the RT
>>> patch is applied is not a good idea. I do not had the time to look
>>> through the hole rt patch to find a better indicator if it is applied so
>>> I just removed the ones preventing compiling on normal kernels. I just
>>> looked into patch-2.6.33.7-rt29.bz2 and not the other RT patches.
>>
>> OK, I see your point now. I did not realize that the mainstream
>> kernels also had those two options in them. I haven't had much luck
>> finding an config option that is always on in the RT kernels. For now
>> your fix sounds like the right tradeoff.
>
> Just so we're clear, no further patch is needed ay?
>
> ?Luis
>

2010-10-24 19:39:55

by Hauke Mehrtens

[permalink] [raw]
Subject: Re: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

On 10/24/2010 09:19 PM, Blaise Gassend wrote:
> I agree that CONFIG_NONE should have been CONFIG_PREEMPT_NONE. On the
> other hand, I disagree that CONFIG_PREEMPT_NONE and
> CONFIG_PREEMPT_VOLUNTARY should be removed. They might not be in the
> latest RT patch, but they are in older patches (2.6.29 in particular).
> Leaving them in will not hinder kernels that no longer have
> CONFIG_PREEMPT_NONE and CONFIG_PREEMPT_VOLUNTARY. Removing them will
> cause the build to fail for some configurations of 2.6.29-rt, and
> probably others.

Hi Blaise,

in the Ubuntu 10.04 default kernel config CONFIG_PREEMPT_VOLUNTARY=y is
set, but it does not use the RT patches, so compat-wireless build
failed. If you are using a mainline kernel you can choose between
CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT (all
localed in kernel/Kconfig.preempt for a least kernel 2.6.24), so these
options are not indicating that the RT patches are applied. The RT patch
adds CONFIG_PREEMPT_RT and CONFIG_PREEMPT_DESKTOP and removes
CONFIG_PREEMPT, so choosing one of these options to detect that the RT
patch is applied is not a good idea. I do not had the time to look
through the hole rt patch to find a better indicator if it is applied so
I just removed the ones preventing compiling on normal kernels. I just
looked into patch-2.6.33.7-rt29.bz2 and not the other RT patches.

It will be nice if you could find a better option which is set in the rt
patch every time, but is never set in a normal kernel. As I see it none
of the CONFIG_PREEMPT_ is of that type.

compat.wireless focus at first to build on mainline kernels and if it is
no problem we are able to support some kernels with extra patches like
the rt patch, because most of the users are using a mainline kernel.

Hauke

> On Sat, Oct 23, 2010 at 11:13 AM, Hauke Mehrtens <[email protected]> wrote:
>> CONFIG_NONE and CONFIG_PREEMPT_VOLUNTARY are not added by the last RT
>> patch. I have not found any references to CONFIG_NONE, probably
>> CONFIG_PREEMPT_NONE was meant, but that is also wrong like
>> CONFIG_PREEMPT_VOLUNTARY. These two options are also in the normal kernel
>> config system without the rt patch.
>>
>> This patch only checks for KConfig options added by the rt patch, but a
>> rt-patch user could also select PREEMPT_NONE or PREEMPT_VOLUNTARY, and
>> compat-wireless will not build. I do not think this will hapen often, so leave
>> it like this.
>>
>> This patch fixes build with all non rt-kernels.
>>
>> CC: Blaise Gassend <[email protected]>
>> Signed-off-by: Hauke Mehrtens <[email protected]>
>> ---
>> include/linux/compat-2.6.34.h | 9 +++------
>> 1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/compat-2.6.34.h b/include/linux/compat-2.6.34.h
>> index 1cfd6e5..f710d08 100644
>> --- a/include/linux/compat-2.6.34.h
>> +++ b/include/linux/compat-2.6.34.h
>> @@ -142,8 +142,7 @@ do { \
>>
>> static inline void device_lock(struct device *dev)
>> {
>> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
>> - defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
>> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
>> mutex_lock(&dev->parent->mutex);
>> #else
>> down(&dev->sem);
>> @@ -152,8 +151,7 @@ static inline void device_lock(struct device *dev)
>>
>> static inline int device_trylock(struct device *dev)
>> {
>> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
>> - defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
>> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
>> return mutex_trylock(&dev->mutex);
>> #else
>> return down_trylock(&dev->sem);
>> @@ -162,8 +160,7 @@ static inline int device_trylock(struct device *dev)
>>
>> static inline void device_unlock(struct device *dev)
>> {
>> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
>> - defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
>> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
>> mutex_unlock(&dev->mutex);
>> #else
>> up(&dev->sem);

2010-10-24 20:42:00

by Blaise Gassend

[permalink] [raw]
Subject: Re: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

Hi Hauke,

> in the Ubuntu 10.04 default kernel config CONFIG_PREEMPT_VOLUNTARY=y is
> set, but it does not use the RT patches, so compat-wireless build
> failed. If you are using a mainline kernel you can choose between
> CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT (all
> localed in kernel/Kconfig.preempt for a least kernel 2.6.24), so these
> options are not indicating that the RT patches are applied. The RT patch
> adds CONFIG_PREEMPT_RT and CONFIG_PREEMPT_DESKTOP and removes
> CONFIG_PREEMPT, so choosing one of these options to detect that the RT
> patch is applied is not a good idea. I do not had the time to look
> through the hole rt patch to find a better indicator if it is applied so
> I just removed the ones preventing compiling on normal kernels. I just
> looked into patch-2.6.33.7-rt29.bz2 and not the other RT patches.

OK, I see your point now. I did not realize that the mainstream
kernels also had those two options in them. I haven't had much luck
finding an config option that is always on in the RT kernels. For now
your fix sounds like the right tradeoff.

Blaise

2010-10-25 16:50:08

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

On Sun, Oct 24, 2010 at 1:41 PM, Blaise Gassend <[email protected]> wrote:
> Hi Hauke,
>
>> in the Ubuntu 10.04 default kernel config CONFIG_PREEMPT_VOLUNTARY=y is
>> set, but it does not use the RT patches, so compat-wireless build
>> failed. If you are using a mainline kernel you can choose between
>> CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT (all
>> localed in kernel/Kconfig.preempt for a least kernel 2.6.24), so these
>> options are not indicating that the RT patches are applied. The RT patch
>> adds CONFIG_PREEMPT_RT and CONFIG_PREEMPT_DESKTOP and removes
>> CONFIG_PREEMPT, so choosing one of these options to detect that the RT
>> patch is applied is not a good idea. I do not had the time to look
>> through the hole rt patch to find a better indicator if it is applied so
>> I just removed the ones preventing compiling on normal kernels. I just
>> looked into patch-2.6.33.7-rt29.bz2 and not the other RT patches.
>
> OK, I see your point now. I did not realize that the mainstream
> kernels also had those two options in them. I haven't had much luck
> finding an config option that is always on in the RT kernels. For now
> your fix sounds like the right tradeoff.

Just so we're clear, no further patch is needed ay?

Luis

2010-10-24 19:19:52

by Blaise Gassend

[permalink] [raw]
Subject: Re: [PATCH 1/3] compat: fix device_lock functions on non rt kernel

I agree that CONFIG_NONE should have been CONFIG_PREEMPT_NONE. On the
other hand, I disagree that CONFIG_PREEMPT_NONE and
CONFIG_PREEMPT_VOLUNTARY should be removed. They might not be in the
latest RT patch, but they are in older patches (2.6.29 in particular).
Leaving them in will not hinder kernels that no longer have
CONFIG_PREEMPT_NONE and CONFIG_PREEMPT_VOLUNTARY. Removing them will
cause the build to fail for some configurations of 2.6.29-rt, and
probably others.

On Sat, Oct 23, 2010 at 11:13 AM, Hauke Mehrtens <[email protected]> wrote:
> CONFIG_NONE and CONFIG_PREEMPT_VOLUNTARY are not added by the last RT
> patch. I have not found any references to CONFIG_NONE, probably
> CONFIG_PREEMPT_NONE was meant, but that is also wrong like
> CONFIG_PREEMPT_VOLUNTARY. These two options are also in the normal kernel
> config system without the rt patch.
>
> This patch only checks for KConfig options added by the rt patch, but a
> rt-patch user could also select PREEMPT_NONE or PREEMPT_VOLUNTARY, and
> compat-wireless will not build. I do not think this will hapen often, so leave
> it like this.
>
> This patch fixes build with all non rt-kernels.
>
> CC: Blaise Gassend <[email protected]>
> Signed-off-by: Hauke Mehrtens <[email protected]>
> ---
> ?include/linux/compat-2.6.34.h | ? ?9 +++------
> ?1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/compat-2.6.34.h b/include/linux/compat-2.6.34.h
> index 1cfd6e5..f710d08 100644
> --- a/include/linux/compat-2.6.34.h
> +++ b/include/linux/compat-2.6.34.h
> @@ -142,8 +142,7 @@ do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>
> ?static inline void device_lock(struct device *dev)
> ?{
> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
> - ? ?defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
> ? ? ? ? mutex_lock(&dev->parent->mutex);
> ?#else
> ? ? ? ?down(&dev->sem);
> @@ -152,8 +151,7 @@ static inline void device_lock(struct device *dev)
>
> ?static inline int device_trylock(struct device *dev)
> ?{
> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
> - ? ?defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
> ? ? ? ?return mutex_trylock(&dev->mutex);
> ?#else
> ? ? ? ?return down_trylock(&dev->sem);
> @@ -162,8 +160,7 @@ static inline int device_trylock(struct device *dev)
>
> ?static inline void device_unlock(struct device *dev)
> ?{
> -#if defined(CONFIG_NONE) || defined(CONFIG_PREEMPT_RT) || \
> - ? ?defined(CONFIG_PREEMPT_VOLUNTARY) || defined(CONFIG_PREEMPT_DESKTOP)
> +#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
> ? ? ? ? mutex_unlock(&dev->mutex);
> ?#else
> ? ? ? ?up(&dev->sem);
> --
> 1.7.0.4
>
>

2010-10-23 20:01:26

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 2/3] compat: lock correct mutex in device_lock

On Sat, Oct 23, 2010 at 11:14 AM, Hauke Mehrtens <[email protected]> wrote:
> This seams to be an error as all the other functions are working on the
> other struct.
>
> CC: Blaise Gassend <[email protected]>
> Signed-off-by: Hauke Mehrtens <[email protected]>

Thanks applied and pushed out all 3 patches.

Luis

2010-10-23 18:14:17

by Hauke Mehrtens

[permalink] [raw]
Subject: [PATCH 2/3] compat: lock correct mutex in device_lock

This seams to be an error as all the other functions are working on the
other struct.

CC: Blaise Gassend <[email protected]>
Signed-off-by: Hauke Mehrtens <[email protected]>
---
include/linux/compat-2.6.34.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/compat-2.6.34.h b/include/linux/compat-2.6.34.h
index f710d08..763abca 100644
--- a/include/linux/compat-2.6.34.h
+++ b/include/linux/compat-2.6.34.h
@@ -143,7 +143,7 @@ do { \
static inline void device_lock(struct device *dev)
{
#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT_DESKTOP)
- mutex_lock(&dev->parent->mutex);
+ mutex_lock(&dev->mutex);
#else
down(&dev->sem);
#endif
--
1.7.0.4