linux-fusion: fix several build issues

PATCH New Mason 2 participants 6 messages Jan 12, 2016 – Jan 16, 2016

lore

Share & tools

lore

Mason

author

2016-01-12 15:09 UTC #0 lore raw

PATCH 1/1 linux-fusion: fix several build issues

The first patch merely fixes a warning. The other two are required to use the module on Linux 4.x

Signed-off-by Mason <[email protected]>

---
IMO, reading patches of patches is not very convenient. I can attach the original patches if necessary.

---
 ...tched-conversion-spec-and-value-in-printk.patch |  30 +++++++
 ...fusion-Unknown-symbol-tasklist_lock-err-0.patch |  39 ++++++++
 .../0004-Port-one-one_udp.c-to-Linux-4.1.patch     | 100 +++++++++++++++++++++
 3 files changed, 169 insertions(+)
 create mode 100644 package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch
 create mode 100644 package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch
 create mode 100644 package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +30 -0

new file mode 100644
index 000000000000..18fd2395e0f7
@@ -0,0 +1,30 @@
+From debb9cafe9b7cc2b286399ecc8e3210480061c70 Mon Sep 17 00:00:00 2001
+From: Mason <[email protected]>
+Date: Mon, 11 Jan 2016 15:42:16 +0100
+Subject: [PATCH 2/4] Fix mismatched conversion spec and value in printk
+
+linux/drivers/char/fusion/fusiondev.c:775:38: warning:
+format '%ld' expects argument of type 'long int', but argument 7 has type 'int'
+
+Subtracting two pointers yields a ptrdiff_t value, and ptrdiff_t is not
+necessarily an alias for long int. Cast the value to long int.
+---
+ linux/drivers/char/fusion/fusiondev.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/linux/drivers/char/fusion/fusiondev.c b/linux/drivers/char/fusion/fusiondev.c
+index 7003407f7e1e..dfb5f8ecb81a 100644
+--- a/linux/drivers/char/fusion/fusiondev.c
++++ b/linux/drivers/char/fusion/fusiondev.c
+@@ -775,7 +775,7 @@ call_ioctl(FusionDev * dev, Fusionee * fusionee,
+                               printk( KERN_ERR "fusion: FUSION_CALL_EXECUTE3 with errorneous call (failed on previous ioctl call), "
+                                                "call id %d, flags 0x%08x, arg %d, length %u, serial %u,  %ld\n",
+                                       execute3.call_id, execute3.flags, execute3.call_arg, execute3.length, execute3.ret_length,
+-                                      (execute3_bin - (FusionCallExecute3 *) arg) );
++                                      (long int)(execute3_bin - (FusionCallExecute3 *) arg) );
+                               return -EIO;
+                          }
+ 
+-- 
+2.6.3
+

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +39 -0

new file mode 100644
index 000000000000..191e47d1d367
@@ -0,0 +1,39 @@
+From a6e950ea4827813c70fafa912c5c3d1f8a2ccaf4 Mon Sep 17 00:00:00 2001
+From: Mason <[email protected]>
+Date: Mon, 11 Jan 2016 16:45:30 +0100
+Subject: [PATCH 3/4] Fix fusion: Unknown symbol tasklist_lock (err 0)
+
+Commit 28f6569ab7d0 renamed TREE_PREEMPT_RCU to PREEMPT_RCU in 3.19
+As a result, the code incorrectly falls back to using tasklist_lock
+(which was made private in 2.6.18)
+
+Always use rcu_read_lock on modern kernels.
+---
+ linux/drivers/char/fusion/fusionee.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/linux/drivers/char/fusion/fusionee.c b/linux/drivers/char/fusion/fusionee.c
+index ef067f5bc831..adaabaedb355 100644
+--- a/linux/drivers/char/fusion/fusionee.c
++++ b/linux/drivers/char/fusion/fusionee.c
+@@ -925,7 +925,7 @@ fusionee_kill(FusionDev * dev,
+                if (f != fusionee && (!target || target == f->id)) {
+                     struct task_struct *p;
+ 
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_lock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,18) || defined(rcu_read_lock)
+                     rcu_read_lock();
+ #else
+                     read_lock(&tasklist_lock);
+@@ -946,7 +946,7 @@ fusionee_kill(FusionDev * dev,
+                          }
+                     }
+ 
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_unlock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,18) || defined(rcu_read_lock)
+                     rcu_read_unlock();
+ #else
+                     read_unlock(&tasklist_lock);
+-- 
+2.6.3
+

package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch new file +100 -0

100 lines not on this page · raw message

Signature · 1 line

2.6.3

Thomas Petazzoni

2016-01-12 16:42 UTC #1 lore raw

PATCH 1/1 linux-fusion: fix several build issues

Hello,

Quoted 4 lines · Mason

The first patch merely fixes a warning. The other two are required to use the module on Linux 4.x
Signed-off-by Mason <[email protected]>

We need your real name here to accept your patches. See
http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L448.

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch line 4

Quoted 10 lines

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +4 -0

new file mode 100644
index 000000000000..18fd2395e0f7
@@ -0,0 +1,30 @@
+From debb9cafe9b7cc2b286399ecc8e3210480061c70 Mon Sep 17 00:00:00 2001
+From: Mason <[email protected]>
+Date: Mon, 11 Jan 2016 15:42:16 +0100
+Subject: [PATCH 2/4] Fix mismatched conversion spec and value in printk

Please generate the patches with 'git format-patch -N' so that we don't have numbered patches.

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch line 10

Quoted 6 lines

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +6 -0

+
+linux/drivers/char/fusion/fusiondev.c:775:38: warning:
+format '%ld' expects argument of type 'long int', but argument 7 has type 'int'
+
+Subtracting two pointers yields a ptrdiff_t value, and ptrdiff_t is not
+necessarily an alias for long int. Cast the value to long int.

We also need a SoB line inside the patches.

Quoted 4 lines

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +4 -0
+---
+ linux/drivers/char/fusion/fusiondev.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch line 24

Quoted 10 lines

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +10 -0

+diff --git a/linux/drivers/char/fusion/fusiondev.c b/linux/drivers/char/fusion/fusiondev.c
+index 7003407f7e1e..dfb5f8ecb81a 100644
+--- a/linux/drivers/char/fusion/fusiondev.c
++++ b/linux/drivers/char/fusion/fusiondev.c
+@@ -775,7 +775,7 @@ call_ioctl(FusionDev * dev, Fusionee * fusionee,
+                               printk( KERN_ERR "fusion: FUSION_CALL_EXECUTE3 with errorneous call (failed on previous ioctl call), "
+                                                "call id %d, flags 0x%08x, arg %d, length %u, serial %u,  %ld\n",
+                                       execute3.call_id, execute3.flags, execute3.call_arg, execute3.length, execute3.ret_length,
+-                                      (execute3_bin - (FusionCallExecute3 *) arg) );
++                                      (long int)(execute3_bin - (FusionCallExecute3 *) arg) );

Can you try instead to replace '%ld' by '%tu'. Apparently, according to lib/vsprintf.c, %t is the proper format specifier for ptrdiff_t.

It is for example used here:

mm/slub.c: pr_err("INFO: Object 0x%p @offset=%tu\n", p, p - addr);

Or maybe you need %td if the value can be negative.

Quoted 6 lines

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +0 -0
new file mode 100644
index 000000000000..191e47d1d367

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch line 10

Quoted 10 lines

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +10 -0

@@ -0,0 +1,39 @@
+From a6e950ea4827813c70fafa912c5c3d1f8a2ccaf4 Mon Sep 17 00:00:00 2001
+From: Mason <[email protected]>
+Date: Mon, 11 Jan 2016 16:45:30 +0100
+Subject: [PATCH 3/4] Fix fusion: Unknown symbol tasklist_lock (err 0)
+
+Commit 28f6569ab7d0 renamed TREE_PREEMPT_RCU to PREEMPT_RCU in 3.19
+As a result, the code incorrectly falls back to using tasklist_lock
+(which was made private in 2.6.18)
+
+Always use rcu_read_lock on modern kernels.

Missing SoB line.

Quoted 4 lines

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +4 -0
+---
+ linux/drivers/char/fusion/fusionee.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch line 24

Quoted 10 lines

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +10 -0

+diff --git a/linux/drivers/char/fusion/fusionee.c b/linux/drivers/char/fusion/fusionee.c
+index ef067f5bc831..adaabaedb355 100644
+--- a/linux/drivers/char/fusion/fusionee.c
++++ b/linux/drivers/char/fusion/fusionee.c
+@@ -925,7 +925,7 @@ fusionee_kill(FusionDev * dev,
+                if (f != fusionee && (!target || target == f->id)) {
+                     struct task_struct *p;
+ 
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_lock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,18) || defined(rcu_read_lock)

Are you sure it is good to keep the defined(rcu_read_lock) test ? This only works if rcu_read_lock is a macro, not if it's an inline function. If tasklisk_lock is available up to 2.6.18, then why not simply keep the version-based condition?

Quoted 20 lines

package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch new file +14 -0

new file mode 100644
index 000000000000..1e05808e9bc2
@@ -0,0 +1,100 @@
+From cd420f7ed42e4a580edbeb3bc49b8492f2c71b91 Mon Sep 17 00:00:00 2001
+From: Mason <[email protected]>
+Date: Tue, 12 Jan 2016 14:01:42 +0100
+Subject: [PATCH 4/4] Port one/one_udp.c to Linux 4.1
+
+Kernel commit c0371da6047a replaced msg_iov and msg_iovlen with msg_iter
+in struct msghdr since 3.19
+
+one/one_udp.c: In function 'ksocket_send_iov':
+one/one_udp.c:186:9: error: 'struct msghdr' has no member named 'msg_iov'
+one/one_udp.c:187:9: error: 'struct msghdr' has no member named 'msg_iovlen'
+
+one/one_udp.c: In function 'ksocket_receive':
+one/one_udp.c:221:9: error: 'struct msghdr' has no member named 'msg_iov'

package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch line 24

Quoted 10 lines

package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch new file +10 -0

+one/one_udp.c:222:9: error: 'struct msghdr' has no member named 'msg_iovlen'
+
+The iov_iter interface
+https://lwn.net/Articles/625077/
+
+Kernel commit d8725c86aeba dropped the len parameter in sock_sendmsg
+since 4.1
+
+one/one_udp.c: In function 'ksocket_send_iov':
+one/one_udp.c:192:13: error: too many arguments to function 'sock_sendmsg'

Missing SoB.

Thanks,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Mason

author

2016-01-12 22:20 UTC #2 lore raw

PATCH 1/1 linux-fusion: fix several build issues

Quoted 2 lines · Thomas Petazzoni

We need your real name here to accept your patches. See
http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L448.

Do you mean my full name, not just my first name?

Quoted 2 lines

Please generate the patches with 'git format-patch -N' so that we don't have numbered patches.

OK.

Quoted 1 line

We also need a SoB line inside the patches.

OK.

Quoted 2 lines

Can you try instead to replace '%ld' by '%tu'. Apparently, according to lib/vsprintf.c, %t is the proper format specifier for ptrdiff_t.

Erf, I had missed that, as the documentation isn't up-to-date.
https://www.kernel.org/doc/Documentation/printk-formats.txt

However, if I do use a specific kernel feature, the module will no longer build for earlier kernels, while a cast works "everywhere".

I tried tracing the history of the %t spec in printk. I found 4370aa4aa753 (dated 2009-03-06) i.e. since 2.6.30

What do you think?

Quoted 7 lines

Quoted 2 lines
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_lock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,18) || defined(rcu_read_lock)
Are you sure it is good to keep the defined(rcu_read_lock) test ? This only works if rcu_read_lock is a macro, not if it's an inline function. If tasklisk_lock is available up to 2.6.18, then why not simply keep the version-based condition?

AFAICT, the rcu_read_lock macro was introduced in the 2.5 dev cycle. bc33f24bdca8 changed rcu_read_lock from a macro to an inline function since 2.6.32

It's possible e.g. kernel 2.6.16 (one of the first LTS kernels IIUC) had both rcu_read_lock available, and tasklist_lock exported.

On the other hand, if both methods are available, I'm not sure which was better. Do you have a strong opinion on the subject?

Thanks for the review.

Regards.

Thomas Petazzoni

2016-01-13 08:23 UTC #3 lore raw

PATCH 1/1 linux-fusion: fix several build issues

Hello,

Quoted 4 lines · Mason

Quoted 2 lines
We need your real name here to accept your patches. See
http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L448.
Do you mean my full name, not just my first name?

Yes, your full name (first name + last name).

Quoted 6 lines

Quoted 2 lines
Can you try instead to replace '%ld' by '%tu'. Apparently, according to lib/vsprintf.c, %t is the proper format specifier for ptrdiff_t.
Erf, I had missed that, as the documentation isn't up-to-date.
https://www.kernel.org/doc/Documentation/printk-formats.txt

Quoted 8 lines

However, if I do use a specific kernel feature, the module will no longer build for earlier kernels, while a cast works "everywhere".
I tried tracing the history of the %t spec in printk. I found 4370aa4aa753 (dated 2009-03-06) i.e. since 2.6.30
What do you think?

Keep your cast, it's simpler and will remain compatible with kernels earlier than 2.6.30.

Quoted 12 lines

Quoted 7 lines
Quoted 2 lines
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_lock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,18) || defined(rcu_read_lock)
Are you sure it is good to keep the defined(rcu_read_lock) test ? This only works if rcu_read_lock is a macro, not if it's an inline function. If tasklisk_lock is available up to 2.6.18, then why not simply keep the version-based condition?
AFAICT, the rcu_read_lock macro was introduced in the 2.5 dev cycle. bc33f24bdca8 changed rcu_read_lock from a macro to an inline function since 2.6.32

Quoted 5 lines

It's possible e.g. kernel 2.6.16 (one of the first LTS kernels IIUC) had both rcu_read_lock available, and tasklist_lock exported.
On the other hand, if both methods are available, I'm not sure which was better. Do you have a strong opinion on the subject?

No, I don't have a strong opinion on this, since I don't fully grok the details. Though I would assume that if tasklist_lock was changed to be no longer exported, then presumably using rcu_read_lock() is a better solution. But again I'm just guessing.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Mason

author

2016-01-15 16:12 UTC #4 lore raw

PATCH v2 1/1 linux-fusion: fix several build issues

From: Marc Gonzalez <[email protected]>

The first patch merely fixes a warning. The other two are required to use the module on Linux 4.x

Signed-off-by Marc Gonzalez <[email protected]>

---
 .../0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch     | 29 ++++++++++
 .../0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch          | 38 +++++++++++++
 package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch       | 99 +++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+)

package/linux-fusion/0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch new file +29 -0

new file mode 100644
index 000000000000..a3f8bb7812bf
@@ -0,0 +1,29 @@
+From debb9cafe9b7cc2b286399ecc8e3210480061c70 Mon Sep 17 00:00:00 2001
+From: Marc Gonzalez <[email protected]>
+Date: Mon, 11 Jan 2016 15:42:16 +0100
+Subject: [PATCH] Fix mismatched conversion spec and value in printk
+
+linux/drivers/char/fusion/fusiondev.c:775:38: warning:
+format '%ld' expects argument of type 'long int', but argument 7 has type 'int'
+
+Subtracting two pointers yields a ptrdiff_t value, and ptrdiff_t is not
+necessarily an alias for long int. Cast the value to long int.
+
+Signed-off-by: Marc Gonzalez <[email protected]>
+---
+ linux/drivers/char/fusion/fusiondev.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/linux/drivers/char/fusion/fusiondev.c b/linux/drivers/char/fusion/fusiondev.c
+index 7003407f7e1e..dfb5f8ecb81a 100644
+--- a/linux/drivers/char/fusion/fusiondev.c
++++ b/linux/drivers/char/fusion/fusiondev.c
+@@ -775,7 +775,7 @@ call_ioctl(FusionDev * dev, Fusionee * fusionee,
+                               printk( KERN_ERR "fusion: FUSION_CALL_EXECUTE3 with errorneous call (failed on previous ioctl call), "
+                                                "call id %d, flags 0x%08x, arg %d, length %u, serial %u,  %ld\n",
+                                       execute3.call_id, execute3.flags, execute3.call_arg, execute3.length, execute3.ret_length,
+-                                      (execute3_bin - (FusionCallExecute3 *) arg) );
++                                      (long int)(execute3_bin - (FusionCallExecute3 *) arg) );
+                               return -EIO;
+                          }
+

package/linux-fusion/0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch new file +38 -0

new file mode 100644
index 000000000000..b14ec9d90af3
@@ -0,0 +1,38 @@
+From 9fcd5003c0363af140a06aba94e62c9e1ea0381e Mon Sep 17 00:00:00 2001
+From: Marc Gonzalez <[email protected]>
+Date: Mon, 11 Jan 2016 16:45:30 +0100
+Subject: [PATCH] Fix fusion: Unknown symbol tasklist_lock (err 0)
+
+Commit 28f6569ab7d0 renamed TREE_PREEMPT_RCU to PREEMPT_RCU in 3.19
+As a result, the code incorrectly falls back to using tasklist_lock
+(which was made private in 2.6.18)
+
+Always use rcu_read_lock on modern kernels.
+
+Signed-off-by: Marc Gonzalez <[email protected]>
+---
+ linux/drivers/char/fusion/fusionee.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/linux/drivers/char/fusion/fusionee.c b/linux/drivers/char/fusion/fusionee.c
+index ef067f5bc831..70cd0a665f98 100644
+--- a/linux/drivers/char/fusion/fusionee.c
++++ b/linux/drivers/char/fusion/fusionee.c
+@@ -925,7 +925,7 @@ fusionee_kill(FusionDev * dev,
+                if (f != fusionee && (!target || target == f->id)) {
+                     struct task_struct *p;
+ 
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_lock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+                     rcu_read_lock();
+ #else
+                     read_lock(&tasklist_lock);
+@@ -946,7 +946,7 @@ fusionee_kill(FusionDev * dev,
+                          }
+                     }
+ 
+-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_TREE_PREEMPT_RCU) || defined(CONFIG_TINY_RCU) || defined(rcu_read_unlock)
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
+                     rcu_read_unlock();
+ #else
+                     read_unlock(&tasklist_lock);

package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch new file +99 -0

99 lines not on this page · raw message

Thomas Petazzoni

Applied

2016-01-16 13:04 UTC #5 lore raw

PATCH v2 1/1 linux-fusion: fix several build issues

Dear Mason,

Quoted 11 lines · Mason

From: Marc Gonzalez <[email protected]>
The first patch merely fixes a warning. The other two are required to use the module on Linux 4.x
Signed-off-by Marc Gonzalez <[email protected]>
---
 .../0002-Fix-mismatched-conversion-spec-and-value-in-printk.patch     | 29 ++++++++++
 .../0003-Fix-fusion-Unknown-symbol-tasklist_lock-err-0.patch          | 38 +++++++++++++
 package/linux-fusion/0004-Port-one-one_udp.c-to-Linux-4.1.patch       | 99 +++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+)

Applied, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com