linux-next: build warnings after merge of the akpm tree

linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]

Hi Andrew,

After merging the akpm tree, today's linux-next build (powerpc
ppc64_defconfig) produced these warnings:

drivers/infiniband/hw/cxgb3/cxio_resource.c: In function '__cxio_init_resource_fifo':
drivers/infiniband/hw/cxgb3/cxio_resource.c:62:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:74:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:81:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:86:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:89:7: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_init_qpid_fifo':
drivers/infiniband/hw/cxgb3/cxio_resource.c:123:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_get_resource':
drivers/infiniband/hw/cxgb3/cxio_resource.c:184:6: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_put_resource':
drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]

Probably introduced by commit "include/linux/kfifo.h: replace open-coded
type check code with typecheck()".
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3674 bytes --]

Hi Andrew,

On Thu, 25 Oct 2012 14:28:54 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the akpm tree, today's linux-next build (powerpc
> ppc64_defconfig) produced these warnings:
> 
> drivers/infiniband/hw/cxgb3/cxio_resource.c: In function '__cxio_init_resource_fifo':
> drivers/infiniband/hw/cxgb3/cxio_resource.c:62:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:74:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:81:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:86:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:89:7: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_init_qpid_fifo':
> drivers/infiniband/hw/cxgb3/cxio_resource.c:123:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_get_resource':
> drivers/infiniband/hw/cxgb3/cxio_resource.c:184:6: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_put_resource':
> drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> 
> Probably introduced by commit "include/linux/kfifo.h: replace open-coded
> type check code with typecheck()".

Also:

drivers/scsi/libiscsi.c: In function 'iscsi_free_task':
drivers/scsi/libiscsi.c:507:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi.c: In function 'iscsi_pool_init':
drivers/scsi/libiscsi.c:2510:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi.c: In function 'iscsi_conn_setup':
drivers/scsi/libiscsi.c:2881:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi.c: In function 'iscsi_conn_teardown':
drivers/scsi/libiscsi.c:2944:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_cleanup_task':
drivers/scsi/libiscsi_tcp.c:462:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c:469:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_r2t_rsp':
drivers/scsi/libiscsi_tcp.c:570:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c:586:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c:596:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_get_curr_r2t':
drivers/scsi/libiscsi_tcp.c:998:5: warning: comparison of distinct pointer types lacks a cast [enabled by default]

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

Hi Richard,

On Thu, 25 Oct 2012 22:12:32 +0800 Richard Yang <weiyang@linux.vnet.ibm.com> wrote:
>
> I did a quick check. 
> 
> The root reason is, 
> 1. kfifo_in() second parameter should be type "const void *"
> 2. kfifo_out_locked() second parameter should be type "void *"
> 3. kfifo_in_locked() second parameter should be type "const void *"
> 
> And I am curious about why the original code couldn't detect this type
> mismatch.  
> 
> The reason is: the original code use = not ==. 
> so when a const void * is assigned another pointer type, there is no warning.
> 
> While I write this test code, there is still no warning. 
> int   main()
> {
> 	int *a; 
> 	void* b;
> 	b = a;  // this should be ok.
> 	a = b;  // this should need the type transfer, but still no warning.
> 	return 0;
> } 
> 
> Hmm... not understand. Any error in my test code?

Yes, any pointer can be assigned to a void pointer and a void pointer can
be assigned to any pointer, so neither of the above should complain.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

Hi Richard,

On Thu, 25 Oct 2012 21:09:52 +0800 Richard Yang <weiyang@linux.vnet.ibm.com> wrote:
>
> On Thu, Oct 25, 2012 at 02:28:54PM +1100, Stephen Rothwell wrote:
> >
> >After merging the akpm tree, today's linux-next build (powerpc
> >ppc64_defconfig) produced these warnings:
> >
> >drivers/infiniband/hw/cxgb3/cxio_resource.c: In function '__cxio_init_resource_fifo':
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:62:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:74:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:81:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:86:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:89:7: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_init_qpid_fifo':
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:123:4: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_get_resource':
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:184:6: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c: In function 'cxio_hal_put_resource':
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/infiniband/hw/cxgb3/cxio_resource.c:193:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >
> >Probably introduced by commit "include/linux/kfifo.h: replace open-coded
> >type check code with typecheck()".
> 
> You test this on ppc64 platform?

I use a ppc64 cross compiler.

> As I reading the code, the second parameter of kfifo_in() should be type
> "const void *". 

That is probably true.

> Hmm. I am not sure this error is there before the patch?

There was no warning before the patch.   There are also lots of similar
new warnings in a x86_64 allmodconfig build.

Given that your patch just exposes already existing problems, we need
patches that fix those problems.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build warnings after merge of the akpm tree

[Andrew Morton]

On Thu, 25 Oct 2012 22:12:32 +0800
Richard Yang <weiyang@linux.vnet.ibm.com> wrote:

> >drivers/scsi/libiscsi.c: In function 'iscsi_free_task':
> >drivers/scsi/libiscsi.c:507:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi.c: In function 'iscsi_pool_init':
> >drivers/scsi/libiscsi.c:2510:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi.c: In function 'iscsi_conn_setup':
> >drivers/scsi/libiscsi.c:2881:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi.c: In function 'iscsi_conn_teardown':
> >drivers/scsi/libiscsi.c:2944:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_cleanup_task':
> >drivers/scsi/libiscsi_tcp.c:462:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c:469:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_r2t_rsp':
> >drivers/scsi/libiscsi_tcp.c:570:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c:586:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c:596:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_get_curr_r2t':
> >drivers/scsi/libiscsi_tcp.c:998:5: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> >
> 
> ...
>
> I did a quick check. 
> 
> The root reason is, 
> 1. kfifo_in() second parameter should be type "const void *"
> 2. kfifo_out_locked() second parameter should be type "void *"
> 3. kfifo_in_locked() second parameter should be type "const void *"
> 
> And I am curious about why the original code couldn't detect this type
> mismatch.  

Well, just looking at __cxio_init_resource_fifo():

	u32 entry = 0;
	
	...
	
	kfifo_in(fifo, (unsigned char *) &entry, sizeof(u32));

This is silly - we shouldn't warn about this code.  There is no reason
for the kfifo library code to require that the *caller*'s storage be
const!

kfifo_in() is (or should be) simulating a C function with the interface:

unsigned int kfifo_in(struct kfifo *fifo, const void *buf, size_t n);

This states "this function does not modify the memory at *buf" and it's
perfectly OK for a caller to pass a non-const pointer into kfifo_in(). 
We shouldn't warn about this.

In fact, the __cxio_init_resource_fifo() shouldn't need to cast `buf'
to uchar* either.  The need to add that cast is a shortcoming in the
existing kfifo code.

We should be able to pass *any* pointer, of any type, const or
non-const into code which expects a const void *, without any warning.

And holy cow that code is hard to read :( Why was kfifo_in()
implemented as a macro, anyway?  AFAICT all its args have a known type,
so we could have used a proper C interface, which would have fixed all
this nicely.

Anyway,
include-linux-kfifoh-replace-open-coded-type-check-code-with-typecheck.patch
takes the kfifo typechecking from "too strict" to "far too strict", so
I'll drop that patch.

Re: linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

Hi Andrew,

On Thu, 25 Oct 2012 15:23:37 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> Anyway,
> include-linux-kfifoh-replace-open-coded-type-check-code-with-typecheck.patch
> takes the kfifo typechecking from "too strict" to "far too strict", so
> I'll drop that patch.

I have dropped it from the akpm tree in linux-next today.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build warnings after merge of the akpm tree

[Stefani Seibold]

Am Freitag, den 26.10.2012, 06:36 +0800 schrieb Richard Yang:

> >
> >And holy cow that code is hard to read :( Why was kfifo_in()
> >implemented as a macro, anyway?  AFAICT all its args have a known type,
> >so we could have used a proper C interface, which would have fixed all
> >this nicely.
> 

Thats simple for performance reasons, the compiler remove most of the
code during the compile stage, so no runtime checks are necessary. And
it is the only way since C does not provides templates like C++.
 
> Hmm, move the definition of kfifo_in()/kfifo_out() into the kfifo.c?
> 

Don't do it. this will result in a performance degradation. Look at the
disassembled code by each change in code and compare it with the
previous one. I don't believe that you can produce better code.

linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

Hi Andrew,

After merging the akpm tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

In file included from include/linux/printk.h:6:0,
                 from include/linux/kernel.h:13,
                 from include/linux/list.h:8,
                 from mm/z3fold.c:26:
mm/z3fold.c: In function 'init_z3fold':
include/linux/kern_levels.h:4:18: warning: format '%d' expects argument of type 'int', but argument 2 has type 'long unsigned int' [-Wformat=]
 #define KERN_SOH "\001"  /* ASCII Start Of Header */
                  ^
include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
 #define KERN_ERR KERN_SOH "3" /* error conditions */
                  ^
include/linux/printk.h:292:9: note: in expansion of macro 'KERN_ERR'
  printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         ^
mm/z3fold.c:887:3: note: in expansion of macro 'pr_err'
   pr_err("z3fold: z3fold_header size (%d) is bigger than "
   ^

Introduced by patch

  "z3fold: don't fail kernel build if z3fold_header is too big"

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build warnings after merge of the akpm tree

[Lars Ellenberg]

On Thu, Nov 29, 2012 at 05:19:39PM +1100, Stephen Rothwell wrote:
> Hi Andrew,
> 
> After merging the akpm tree, today's linux-next build (x86_64
> allmodconfig) produced these warnings:


> In file included from drivers/block/drbd/drbd_bitmap.c:32:0:
> drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
> include/linux/wait.h:554:0: note: this is the location of the previous definition

> Introduced by the interaction of commit c1fd29a11f43 ("drbd: Fix a race
> condition that can lead to a BUG()") from the block tree and commit
> "wait: add wait_event_lock_irq() interface" from the akpm tree.

Thanks.
We can just drop our copy-n-paste-from-md.h then.

	Lars

---------------------------

commit 2712ab592def9f0171b6d47349879e975db57657
Author: Lars Ellenberg <lars.ellenberg@linbit.com>
Date:   Thu Nov 29 08:20:27 2012 +0100

    drbd: fix merge clash: wait_event_lock_irq() is now defined in wait.h
    
    Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index ef72a72..6b51afa 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -2334,30 +2334,3 @@ static inline void drbd_md_flush(struct drbd_conf *mdev)
 }
 
 #endif
-
-/* This is defined in drivers/md/md.h as well. Should go into wait.h */
-#define __wait_event_lock_irq(wq, condition, lock, cmd) 		\
-do {									\
-	wait_queue_t __wait;						\
-	init_waitqueue_entry(&__wait, current);				\
-									\
-	add_wait_queue(&wq, &__wait);					\
-	for (;;) {							\
-		set_current_state(TASK_UNINTERRUPTIBLE);		\
-		if (condition)						\
-			break;						\
-		spin_unlock_irq(&lock);					\
-		cmd;							\
-		schedule();						\
-		spin_lock_irq(&lock);					\
-	}								\
-	current->state = TASK_RUNNING;					\
-	remove_wait_queue(&wq, &__wait);				\
-} while (0)
-
-#define wait_event_lock_irq(wq, condition, lock, cmd) 			\
-do {									\
-	if (condition)	 						\
-		break;							\
-	__wait_event_lock_irq(wq, condition, lock, cmd);		\
-} while (0)
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 69ef352..53bf618 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1800,8 +1800,7 @@ _conn_request_state(struct drbd_tconn *tconn, union drbd_state mask, union drbd_
 		spin_lock_irq(&tconn->req_lock);
 		wait_event_lock_irq(tconn->ping_wait,
 				(rv = _conn_rq_cond(tconn, mask, val)),
-				tconn->req_lock,
-				);
+				tconn->req_lock);
 		clear_bit(CONN_WD_ST_CHG_REQ, &tconn->flags);
 		if (rv < SS_SUCCESS)
 			goto abort;

linux-next: build warnings after merge of the akpm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]

Hi Andrew,

After merging the akpm tree, today's linux-next build (x86_64
allmodconfig) produced these warnings:

In file included from drivers/block/drbd/drbd_bitmap.c:32:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition
In file included from drivers/block/drbd/drbd_receiver.c:46:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition
In file included from drivers/block/drbd/drbd_proc.c:34:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition
In file included from drivers/block/drbd/drbd_worker.c:38:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition
In file included from drivers/block/drbd/drbd_actlog.c:31:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition
In file included from drivers/block/drbd/drbd_req.c:30:0:
drivers/block/drbd/drbd_int.h:2339:0: warning: "__wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:554:0: note: this is the location of the previous definition
drivers/block/drbd/drbd_int.h:2358:0: warning: "wait_event_lock_irq" redefined [enabled by default]
include/linux/wait.h:621:0: note: this is the location of the previous definition

Introduced by the interaction of commit c1fd29a11f43 ("drbd: Fix a race
condition that can lead to a BUG()") from the block tree and commit
"wait: add wait_event_lock_irq() interface" from the akpm tree.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]