From: Yuanhan Liu <[email protected]>
Firstly, this kind of type check doesn't work. It does something similay
like following:
void * __dummy = NULL;
__buf = __dummy;
__dummy is defined as void *. Thus it will not trigger warnings as
expected.
Second, we don't need that kind of check. Since the prototype
of __kfifo_out is:
unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
buf is defined as void *, so we don't need do the type check. Remove it.
LINK: https://lkml.org/lkml/2012/10/25/386
LINK: https://lkml.org/lkml/2012/10/25/584
Cc: Andrew Morton <[email protected]>
Cc: Wei Yang <[email protected]>
Cc: Stefani Seibold <[email protected]>
Cc: Fengguang Wu <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Signed-off-by: Yuanhan Liu <[email protected]>
---
include/linux/kfifo.h | 20 --------------------
1 file changed, 20 deletions(-)
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 10308c6..b8c1d03 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -390,10 +390,6 @@ __kfifo_int_must_check_helper( \
unsigned int __ret; \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) { \
- typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
- __dummy = (typeof(__val))NULL; \
- } \
if (__recsize) \
__ret = __kfifo_in_r(__kfifo, __val, sizeof(*__val), \
__recsize); \
@@ -432,8 +428,6 @@ __kfifo_uint_must_check_helper( \
unsigned int __ret; \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) \
- __val = (typeof(__tmp->ptr))0; \
if (__recsize) \
__ret = __kfifo_out_r(__kfifo, __val, sizeof(*__val), \
__recsize); \
@@ -473,8 +467,6 @@ __kfifo_uint_must_check_helper( \
unsigned int __ret; \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) \
- __val = (typeof(__tmp->ptr))NULL; \
if (__recsize) \
__ret = __kfifo_out_peek_r(__kfifo, __val, sizeof(*__val), \
__recsize); \
@@ -512,10 +504,6 @@ __kfifo_uint_must_check_helper( \
unsigned long __n = (n); \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) { \
- typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
- __dummy = (typeof(__buf))NULL; \
- } \
(__recsize) ?\
__kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
__kfifo_in(__kfifo, __buf, __n); \
@@ -565,10 +553,6 @@ __kfifo_uint_must_check_helper( \
unsigned long __n = (n); \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) { \
- typeof(__tmp->ptr) __dummy = NULL; \
- __buf = __dummy; \
- } \
(__recsize) ?\
__kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
__kfifo_out(__kfifo, __buf, __n); \
@@ -777,10 +761,6 @@ __kfifo_uint_must_check_helper( \
unsigned long __n = (n); \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) { \
- typeof(__tmp->ptr) __dummy __attribute__ ((unused)) = NULL; \
- __buf = __dummy; \
- } \
(__recsize) ? \
__kfifo_out_peek_r(__kfifo, __buf, __n, __recsize) : \
__kfifo_out_peek(__kfifo, __buf, __n); \
--
1.7.11.7
Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> From: Yuanhan Liu <[email protected]>
>
> Firstly, this kind of type check doesn't work. It does something similay
> like following:
> void * __dummy = NULL;
> __buf = __dummy;
>
> __dummy is defined as void *. Thus it will not trigger warnings as
> expected.
>
> Second, we don't need that kind of check. Since the prototype
> of __kfifo_out is:
> unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
>
> buf is defined as void *, so we don't need do the type check. Remove it.
>
> LINK: https://lkml.org/lkml/2012/10/25/386
> LINK: https://lkml.org/lkml/2012/10/25/584
>
> Cc: Andrew Morton <[email protected]>
> Cc: Wei Yang <[email protected]>
> Cc: Stefani Seibold <[email protected]>
> Cc: Fengguang Wu <[email protected]>
> Cc: Stephen Rothwell <[email protected]>
> Signed-off-by: Yuanhan Liu <[email protected]>
> ---
> include/linux/kfifo.h | 20 --------------------
> 1 file changed, 20 deletions(-)
>
> diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> index 10308c6..b8c1d03 100644
> --- a/include/linux/kfifo.h
> +++ b/include/linux/kfifo.h
> @@ -390,10 +390,6 @@ __kfifo_int_must_check_helper( \
> unsigned int __ret; \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) { \
> - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> - __dummy = (typeof(__val))NULL; \
> - } \
> if (__recsize) \
> __ret = __kfifo_in_r(__kfifo, __val, sizeof(*__val), \
> __recsize); \
> @@ -432,8 +428,6 @@ __kfifo_uint_must_check_helper( \
> unsigned int __ret; \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) \
> - __val = (typeof(__tmp->ptr))0; \
> if (__recsize) \
> __ret = __kfifo_out_r(__kfifo, __val, sizeof(*__val), \
> __recsize); \
> @@ -473,8 +467,6 @@ __kfifo_uint_must_check_helper( \
> unsigned int __ret; \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) \
> - __val = (typeof(__tmp->ptr))NULL; \
> if (__recsize) \
> __ret = __kfifo_out_peek_r(__kfifo, __val, sizeof(*__val), \
> __recsize); \
> @@ -512,10 +504,6 @@ __kfifo_uint_must_check_helper( \
> unsigned long __n = (n); \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) { \
> - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> - __dummy = (typeof(__buf))NULL; \
> - } \
> (__recsize) ?\
> __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> __kfifo_in(__kfifo, __buf, __n); \
> @@ -565,10 +553,6 @@ __kfifo_uint_must_check_helper( \
> unsigned long __n = (n); \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) { \
> - typeof(__tmp->ptr) __dummy = NULL; \
> - __buf = __dummy; \
> - } \
> (__recsize) ?\
> __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
> __kfifo_out(__kfifo, __buf, __n); \
> @@ -777,10 +761,6 @@ __kfifo_uint_must_check_helper( \
> unsigned long __n = (n); \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) { \
> - typeof(__tmp->ptr) __dummy __attribute__ ((unused)) = NULL; \
> - __buf = __dummy; \
> - } \
> (__recsize) ? \
> __kfifo_out_peek_r(__kfifo, __buf, __n, __recsize) : \
> __kfifo_out_peek(__kfifo, __buf, __n); \
Did you tried to compile the whole kernel including all the drivers with
your patch?
On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > From: Yuanhan Liu <[email protected]>
> >
> > Firstly, this kind of type check doesn't work. It does something similay
> > like following:
> > void * __dummy = NULL;
> > __buf = __dummy;
> >
> > __dummy is defined as void *. Thus it will not trigger warnings as
> > expected.
> >
> > Second, we don't need that kind of check. Since the prototype
> > of __kfifo_out is:
> > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> >
> > buf is defined as void *, so we don't need do the type check. Remove it.
> >
> > LINK: https://lkml.org/lkml/2012/10/25/386
> > LINK: https://lkml.org/lkml/2012/10/25/584
> >
> > Cc: Andrew Morton <[email protected]>
> > Cc: Wei Yang <[email protected]>
> > Cc: Stefani Seibold <[email protected]>
> > Cc: Fengguang Wu <[email protected]>
> > Cc: Stephen Rothwell <[email protected]>
> > Signed-off-by: Yuanhan Liu <[email protected]>
> > ---
> > include/linux/kfifo.h | 20 --------------------
> > 1 file changed, 20 deletions(-)
> >
> > diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> > index 10308c6..b8c1d03 100644
> > --- a/include/linux/kfifo.h
> > +++ b/include/linux/kfifo.h
> > @@ -390,10 +390,6 @@ __kfifo_int_must_check_helper( \
> > unsigned int __ret; \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) { \
> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> > - __dummy = (typeof(__val))NULL; \
> > - } \
> > if (__recsize) \
> > __ret = __kfifo_in_r(__kfifo, __val, sizeof(*__val), \
> > __recsize); \
> > @@ -432,8 +428,6 @@ __kfifo_uint_must_check_helper( \
> > unsigned int __ret; \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) \
> > - __val = (typeof(__tmp->ptr))0; \
> > if (__recsize) \
> > __ret = __kfifo_out_r(__kfifo, __val, sizeof(*__val), \
> > __recsize); \
> > @@ -473,8 +467,6 @@ __kfifo_uint_must_check_helper( \
> > unsigned int __ret; \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) \
> > - __val = (typeof(__tmp->ptr))NULL; \
> > if (__recsize) \
> > __ret = __kfifo_out_peek_r(__kfifo, __val, sizeof(*__val), \
> > __recsize); \
> > @@ -512,10 +504,6 @@ __kfifo_uint_must_check_helper( \
> > unsigned long __n = (n); \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) { \
> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> > - __dummy = (typeof(__buf))NULL; \
> > - } \
> > (__recsize) ?\
> > __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> > __kfifo_in(__kfifo, __buf, __n); \
> > @@ -565,10 +553,6 @@ __kfifo_uint_must_check_helper( \
> > unsigned long __n = (n); \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) { \
> > - typeof(__tmp->ptr) __dummy = NULL; \
> > - __buf = __dummy; \
> > - } \
> > (__recsize) ?\
> > __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
> > __kfifo_out(__kfifo, __buf, __n); \
> > @@ -777,10 +761,6 @@ __kfifo_uint_must_check_helper( \
> > unsigned long __n = (n); \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) { \
> > - typeof(__tmp->ptr) __dummy __attribute__ ((unused)) = NULL; \
> > - __buf = __dummy; \
> > - } \
> > (__recsize) ? \
> > __kfifo_out_peek_r(__kfifo, __buf, __n, __recsize) : \
> > __kfifo_out_peek(__kfifo, __buf, __n); \
>
> Did you tried to compile the whole kernel including all the drivers with
> your patch?
Hi Stefani,
I did a build test, it did't introduce any new compile errors and
warnings. While, I haven't tried make allmodconfig then. Does this patch
seems wrong to you?
Thanks,
Yuanhan Liu
Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > From: Yuanhan Liu <[email protected]>
> > >
> > > Firstly, this kind of type check doesn't work. It does something similay
> > > like following:
> > > void * __dummy = NULL;
> > > __buf = __dummy;
> > >
> > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > expected.
> > >
> > > Second, we don't need that kind of check. Since the prototype
> > > of __kfifo_out is:
> > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > >
> > > buf is defined as void *, so we don't need do the type check. Remove it.
> > >
> > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > LINK: https://lkml.org/lkml/2012/10/25/584
> > >
> > > Cc: Andrew Morton <[email protected]>
> > > Cc: Wei Yang <[email protected]>
> > > Cc: Stefani Seibold <[email protected]>
> > > Cc: Fengguang Wu <[email protected]>
> > > Cc: Stephen Rothwell <[email protected]>
> > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > ---
> >
> > Did you tried to compile the whole kernel including all the drivers with
> > your patch?
>
> Hi Stefani,
>
> I did a build test, it did't introduce any new compile errors and
> warnings. While, I haven't tried make allmodconfig then. Does this patch
> seems wrong to you?
>
> Thanks,
> Yuanhan Liu
Hi Liu,
no the patch seems not wrong to me. But as you see with the previous
patch it is not easy to predict the side effects.
An allmodconfig together with C=2 is necessary to check if there is no
side effects which current users of the kfifo API. That is exactly what
i did again and again as i developed the kfifo API.
Also you have to build the kfifo samples, since this example code use
all features of the kfifo API.
And again: The kfifo is designed to do the many things at compile time,
not at runtime. If you modify the code, you have to check the compiler
assembler output for no degradation, especially in kfifo_put, kfifo_get,
kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
if you can do it at compile time. This is the basic reasons to do it in
macros.
Greetings,
Stefani
On Fri, Oct 26, 2012 at 03:17:57PM +0800, Yuanhan Liu wrote:
> On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > > From: Yuanhan Liu <[email protected]>
> > > > >
> > > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > > like following:
> > > > > void * __dummy = NULL;
> > > > > __buf = __dummy;
> > > > >
> > > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > > expected.
> > > > >
> > > > > Second, we don't need that kind of check. Since the prototype
> > > > > of __kfifo_out is:
> > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > > >
> > > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > > >
> > > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > > >
> > > > > Cc: Andrew Morton <[email protected]>
> > > > > Cc: Wei Yang <[email protected]>
> > > > > Cc: Stefani Seibold <[email protected]>
> > > > > Cc: Fengguang Wu <[email protected]>
> > > > > Cc: Stephen Rothwell <[email protected]>
> > > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > > ---
> >
> > > >
> > > > Did you tried to compile the whole kernel including all the drivers with
> > > > your patch?
> > >
> > > Hi Stefani,
> > >
> > > I did a build test, it did't introduce any new compile errors and
> > > warnings. While, I haven't tried make allmodconfig then. Does this patch
> > > seems wrong to you?
> > >
> > > Thanks,
> > > Yuanhan Liu
> >
> > Hi Liu,
> >
> > no the patch seems not wrong to me. But as you see with the previous
> > patch it is not easy to predict the side effects.
> >
> > An allmodconfig together with C=2 is necessary to check if there is no
> > side effects which current users of the kfifo API.
>
> Hi Stefani,
>
> Make with C=2 will produce tons of warnings, hard to tell it introduces
> new warnings or not. I build some drivers used kfifo and samples as you
> suggested with C=2, find no new warnings. I will build all drivers that
> used kfifo with C=2 later, and will post the result here.
Hi Stefani,
Done build all drivers used kfifo and kfifo samples with C=2, none new
warnigs and erros found :D
Thanks,
Yuanhan Liu
>
> > That is exactly what
> > i did again and again as i developed the kfifo API.
> >
> > Also you have to build the kfifo samples, since this example code use
> > all features of the kfifo API.
> >
> > And again: The kfifo is designed to do the many things at compile time,
> > not at runtime. If you modify the code, you have to check the compiler
> > assembler output for no degradation, especially in kfifo_put, kfifo_get,
> > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> > if you can do it at compile time. This is the basic reasons to do it in
> > macros.
>
> Is it enought to check kernel/kfifo.o only? I build that file with
> and without this patch. And then dump it by objdump -D kernel/fifo.o to
> /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
> two dump file are exactly same.
>
> Thanks,
> Yuanhan Liu
On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > From: Yuanhan Liu <[email protected]>
> > > >
> > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > like following:
> > > > void * __dummy = NULL;
> > > > __buf = __dummy;
> > > >
> > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > expected.
> > > >
> > > > Second, we don't need that kind of check. Since the prototype
> > > > of __kfifo_out is:
> > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > >
> > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > >
> > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > >
> > > > Cc: Andrew Morton <[email protected]>
> > > > Cc: Wei Yang <[email protected]>
> > > > Cc: Stefani Seibold <[email protected]>
> > > > Cc: Fengguang Wu <[email protected]>
> > > > Cc: Stephen Rothwell <[email protected]>
> > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > ---
>
> > >
> > > Did you tried to compile the whole kernel including all the drivers with
> > > your patch?
> >
> > Hi Stefani,
> >
> > I did a build test, it did't introduce any new compile errors and
> > warnings. While, I haven't tried make allmodconfig then. Does this patch
> > seems wrong to you?
> >
> > Thanks,
> > Yuanhan Liu
>
> Hi Liu,
>
> no the patch seems not wrong to me. But as you see with the previous
> patch it is not easy to predict the side effects.
>
> An allmodconfig together with C=2 is necessary to check if there is no
> side effects which current users of the kfifo API.
Hi Stefani,
Make with C=2 will produce tons of warnings, hard to tell it introduces
new warnings or not. I build some drivers used kfifo and samples as you
suggested with C=2, find no new warnings. I will build all drivers that
used kfifo with C=2 later, and will post the result here.
> That is exactly what
> i did again and again as i developed the kfifo API.
>
> Also you have to build the kfifo samples, since this example code use
> all features of the kfifo API.
>
> And again: The kfifo is designed to do the many things at compile time,
> not at runtime. If you modify the code, you have to check the compiler
> assembler output for no degradation, especially in kfifo_put, kfifo_get,
> kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> if you can do it at compile time. This is the basic reasons to do it in
> macros.
Is it enought to check kernel/kfifo.o only? I build that file with
and without this patch. And then dump it by objdump -D kernel/fifo.o to
/tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
two dump file are exactly same.
Thanks,
Yuanhan Liu
Am Freitag, den 26.10.2012, 15:17 +0800 schrieb Yuanhan Liu:
> On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > > From: Yuanhan Liu <[email protected]>
> > > > >
> > > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > > like following:
> > > > > void * __dummy = NULL;
> > > > > __buf = __dummy;
> > > > >
> > > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > > expected.
> > > > >
> > > > > Second, we don't need that kind of check. Since the prototype
> > > > > of __kfifo_out is:
> > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > > >
> > > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > > >
> > > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > > >
> > > > > Cc: Andrew Morton <[email protected]>
> > > > > Cc: Wei Yang <[email protected]>
> > > > > Cc: Stefani Seibold <[email protected]>
> > > > > Cc: Fengguang Wu <[email protected]>
> > > > > Cc: Stephen Rothwell <[email protected]>
> > > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > > ---
> >
> > > >
> > > > Did you tried to compile the whole kernel including all the drivers with
> > > > your patch?
> > >
> > > Hi Stefani,
> > >
> > > I did a build test, it did't introduce any new compile errors and
> > > warnings. While, I haven't tried make allmodconfig then. Does this patch
> > > seems wrong to you?
> > >
> > > Thanks,
> > > Yuanhan Liu
> >
> > Hi Liu,
> >
> > no the patch seems not wrong to me. But as you see with the previous
> > patch it is not easy to predict the side effects.
> >
> > An allmodconfig together with C=2 is necessary to check if there is no
> > side effects which current users of the kfifo API.
>
> Hi Stefani,
>
> Make with C=2 will produce tons of warnings, hard to tell it introduces
> new warnings or not. I build some drivers used kfifo and samples as you
> suggested with C=2, find no new warnings. I will build all drivers that
> used kfifo with C=2 later, and will post the result here.
>
That will be great...
> >
> > Also you have to build the kfifo samples, since this example code use
> > all features of the kfifo API.
> >
> > And again: The kfifo is designed to do the many things at compile time,
> > not at runtime. If you modify the code, you have to check the compiler
> > assembler output for no degradation, especially in kfifo_put, kfifo_get,
> > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> > if you can do it at compile time. This is the basic reasons to do it in
> > macros.
>
> Is it enought to check kernel/kfifo.o only? I build that file with
> and without this patch. And then dump it by objdump -D kernel/fifo.o to
> /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
> two dump file are exactly same.
>
No, since most of the code is inlined due performace reasons, you have
to hack the kfifo examples output code for regressions and code
increase.
Greetings,
Stefani
On Fri, Oct 26, 2012 at 05:52:44PM +0800, Richard Yang wrote:
> On Fri, Oct 26, 2012 at 02:11:45PM +0800, Yuanhan Liu wrote:
> >On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> >> Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> >> > From: Yuanhan Liu <[email protected]>
> >> >
> >> > Firstly, this kind of type check doesn't work. It does something similay
> >> > like following:
> >> > void * __dummy = NULL;
> >> > __buf = __dummy;
> >> >
> >> > __dummy is defined as void *. Thus it will not trigger warnings as
> >> > expected.
> >> >
> >> > Second, we don't need that kind of check. Since the prototype
> >> > of __kfifo_out is:
> >> > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> >> >
> >> > buf is defined as void *, so we don't need do the type check. Remove it.
> >> >
> >> > LINK: https://lkml.org/lkml/2012/10/25/386
> >> > LINK: https://lkml.org/lkml/2012/10/25/584
> >> >
> >> > Cc: Andrew Morton <[email protected]>
> >> > Cc: Wei Yang <[email protected]>
> >> > Cc: Stefani Seibold <[email protected]>
> >> > Cc: Fengguang Wu <[email protected]>
> >> > Cc: Stephen Rothwell <[email protected]>
> >> > Signed-off-by: Yuanhan Liu <[email protected]>
> >> > ---
> >> > include/linux/kfifo.h | 20 --------------------
> >> > 1 file changed, 20 deletions(-)
> >> >
> >> > diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> >> > index 10308c6..b8c1d03 100644
> >> > --- a/include/linux/kfifo.h
> >> > +++ b/include/linux/kfifo.h
> >> > @@ -390,10 +390,6 @@ __kfifo_int_must_check_helper( \
> >> > unsigned int __ret; \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) { \
> >> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> >> > - __dummy = (typeof(__val))NULL; \
> >> > - } \
> >> > if (__recsize) \
> >> > __ret = __kfifo_in_r(__kfifo, __val, sizeof(*__val), \
> >> > __recsize); \
> >> > @@ -432,8 +428,6 @@ __kfifo_uint_must_check_helper( \
> >> > unsigned int __ret; \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) \
> >> > - __val = (typeof(__tmp->ptr))0; \
> >> > if (__recsize) \
> >> > __ret = __kfifo_out_r(__kfifo, __val, sizeof(*__val), \
> >> > __recsize); \
> >> > @@ -473,8 +467,6 @@ __kfifo_uint_must_check_helper( \
> >> > unsigned int __ret; \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) \
> >> > - __val = (typeof(__tmp->ptr))NULL; \
> >> > if (__recsize) \
> >> > __ret = __kfifo_out_peek_r(__kfifo, __val, sizeof(*__val), \
> >> > __recsize); \
> >> > @@ -512,10 +504,6 @@ __kfifo_uint_must_check_helper( \
> >> > unsigned long __n = (n); \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) { \
> >> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> >> > - __dummy = (typeof(__buf))NULL; \
> >> > - } \
> >> > (__recsize) ?\
> >> > __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> >> > __kfifo_in(__kfifo, __buf, __n); \
> >> > @@ -565,10 +553,6 @@ __kfifo_uint_must_check_helper( \
> >> > unsigned long __n = (n); \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) { \
> >> > - typeof(__tmp->ptr) __dummy = NULL; \
> >> > - __buf = __dummy; \
> >> > - } \
> >> > (__recsize) ?\
> >> > __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
> >> > __kfifo_out(__kfifo, __buf, __n); \
> >> > @@ -777,10 +761,6 @@ __kfifo_uint_must_check_helper( \
> >> > unsigned long __n = (n); \
> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> > - if (0) { \
> >> > - typeof(__tmp->ptr) __dummy __attribute__ ((unused)) = NULL; \
> >> > - __buf = __dummy; \
> >> > - } \
> >> > (__recsize) ? \
> >> > __kfifo_out_peek_r(__kfifo, __buf, __n, __recsize) : \
> >> > __kfifo_out_peek(__kfifo, __buf, __n); \
> >>
> >> Did you tried to compile the whole kernel including all the drivers with
> >> your patch?
> >
> >Hi Stefani,
> >
> >I did a build test, it did't introduce any new compile errors and
> >warnings. While, I haven't tried make allmodconfig then. Does this patch
> >seems wrong to you?
>
> Hmm, in my mind, those warnings are produced by the code you removed.
> So it is reasonable that you see no new warnings.
Hi Yang,
Nope. It's not hard to tell they are old warnings. And as you can see from
the email I posted, I did a build compile with and without applying this
patch, and does make C=2 2>/tmp/out.{before,after}. I then compared the
two file, they are exactly same. So, no new warnings introduced.
Thanks,
Yuanhan Liu
On Fri, Oct 26, 2012 at 11:26:31AM +0200, Stefani Seibold wrote:
> Am Freitag, den 26.10.2012, 15:17 +0800 schrieb Yuanhan Liu:
> > On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> > > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > > > From: Yuanhan Liu <[email protected]>
> > > > > >
> > > > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > > > like following:
> > > > > > void * __dummy = NULL;
> > > > > > __buf = __dummy;
> > > > > >
> > > > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > > > expected.
> > > > > >
> > > > > > Second, we don't need that kind of check. Since the prototype
> > > > > > of __kfifo_out is:
> > > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > > > >
> > > > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > > > >
> > > > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > > > >
> > > > > > Cc: Andrew Morton <[email protected]>
> > > > > > Cc: Wei Yang <[email protected]>
> > > > > > Cc: Stefani Seibold <[email protected]>
> > > > > > Cc: Fengguang Wu <[email protected]>
> > > > > > Cc: Stephen Rothwell <[email protected]>
> > > > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > > > ---
> > >
[snip]...
> > >
> > > Also you have to build the kfifo samples, since this example code use
> > > all features of the kfifo API.
> > >
> > > And again: The kfifo is designed to do the many things at compile time,
> > > not at runtime. If you modify the code, you have to check the compiler
> > > assembler output for no degradation, especially in kfifo_put, kfifo_get,
> > > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> > > if you can do it at compile time. This is the basic reasons to do it in
> > > macros.
> >
> > Is it enought to check kernel/kfifo.o only? I build that file with
> > and without this patch. And then dump it by objdump -D kernel/fifo.o to
> > /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
> > two dump file are exactly same.
> >
>
> No, since most of the code is inlined due performace reasons, you have
> to hack the kfifo examples output code for regressions and code
> increase.
In my test, this patch doesn't change anything. Here are some data to
prove that:
$ make samples/kfifo/
$ cp samples/kfifo/*.o /tmp/before/
$ git am this-patch
$ make samples/kfifo/
$ cp samples/kfifo/*.o /tmp/after/
$ for i in /tmp/before/*.o; do size $i /tmp/after/`basename $i`; done
text data bss dec hex filename
1939 464 456 2859 b2b /tmp/before/bytestream-example.o
1939 464 456 2859 b2b /tmp/after/bytestream-example.o
text data bss dec hex filename
1423 112 296 1831 727 /tmp/before/dma-example.o
1423 112 296 1831 727 /tmp/after/dma-example.o
text data bss dec hex filename
1864 624 376 2864 b30 /tmp/before/inttype-example.o
1864 624 376 2864 b30 /tmp/after/inttype-example.o
text data bss dec hex filename
1916 464 472 2852 b24 /tmp/before/record-example.o
1916 464 472 2852 b24 /tmp/after/record-example.o
# You will see that it changed nothing.
$ objdump -d /tmp/before/bytestream-example.o >/tmp/bytestream-example.before
$ objdump -d /tmp/after/bytestream-example.o >/tmp/bytestream-example.after
$ diff /tmp/bytestream.before /tmp/bytestream.after -urN
--- bytestream.before 2012-10-26 20:55:33.645578668 +0800
+++ bytestream.after 2012-10-26 20:55:26.520578669 +0800
@@ -1,5 +1,5 @@
-/tmp/bytestream-example.o: file format elf64-x86-64
+/tmp/bytestream-example.o: file format elf64-x86-64
# So, as you can see, expect the filename, they are same.
So, Stefani, is it what you want? Does this looks OK to you?
Thanks,
Yuanhan Liu
Am Freitag, den 26.10.2012, 21:04 +0800 schrieb Yuanhan Liu:
> On Fri, Oct 26, 2012 at 11:26:31AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 15:17 +0800 schrieb Yuanhan Liu:
> > > On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> > > > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > > > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > > > > From: Yuanhan Liu <[email protected]>
> > > > > > >
> > > > > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > > > > like following:
> > > > > > > void * __dummy = NULL;
> > > > > > > __buf = __dummy;
> > > > > > >
> > > > > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > > > > expected.
> > > > > > >
> > > > > > > Second, we don't need that kind of check. Since the prototype
> > > > > > > of __kfifo_out is:
> > > > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > > > > >
> > > > > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > > > > >
> > > > > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > > > > >
> > > > > > > Cc: Andrew Morton <[email protected]>
> > > > > > > Cc: Wei Yang <[email protected]>
> > > > > > > Cc: Stefani Seibold <[email protected]>
> > > > > > > Cc: Fengguang Wu <[email protected]>
> > > > > > > Cc: Stephen Rothwell <[email protected]>
> > > > > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > > > > ---
> > > >
>
> [snip]...
>
> > > >
> > > > Also you have to build the kfifo samples, since this example code use
> > > > all features of the kfifo API.
> > > >
> > > > And again: The kfifo is designed to do the many things at compile time,
> > > > not at runtime. If you modify the code, you have to check the compiler
> > > > assembler output for no degradation, especially in kfifo_put, kfifo_get,
> > > > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> > > > if you can do it at compile time. This is the basic reasons to do it in
> > > > macros.
> > >
> > > Is it enought to check kernel/kfifo.o only? I build that file with
> > > and without this patch. And then dump it by objdump -D kernel/fifo.o to
> > > /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
> > > two dump file are exactly same.
> > >
> >
> > No, since most of the code is inlined due performace reasons, you have
> > to hack the kfifo examples output code for regressions and code
> > increase.
>
> In my test, this patch doesn't change anything. Here are some data to
> prove that:
>
> $ make samples/kfifo/
> $ cp samples/kfifo/*.o /tmp/before/
>
> $ git am this-patch
> $ make samples/kfifo/
> $ cp samples/kfifo/*.o /tmp/after/
>
> $ for i in /tmp/before/*.o; do size $i /tmp/after/`basename $i`; done
> text data bss dec hex filename
> 1939 464 456 2859 b2b /tmp/before/bytestream-example.o
> 1939 464 456 2859 b2b /tmp/after/bytestream-example.o
> text data bss dec hex filename
> 1423 112 296 1831 727 /tmp/before/dma-example.o
> 1423 112 296 1831 727 /tmp/after/dma-example.o
> text data bss dec hex filename
> 1864 624 376 2864 b30 /tmp/before/inttype-example.o
> 1864 624 376 2864 b30 /tmp/after/inttype-example.o
> text data bss dec hex filename
> 1916 464 472 2852 b24 /tmp/before/record-example.o
> 1916 464 472 2852 b24 /tmp/after/record-example.o
> # You will see that it changed nothing.
>
>
> $ objdump -d /tmp/before/bytestream-example.o >/tmp/bytestream-example.before
> $ objdump -d /tmp/after/bytestream-example.o >/tmp/bytestream-example.after
> $ diff /tmp/bytestream.before /tmp/bytestream.after -urN
> --- bytestream.before 2012-10-26 20:55:33.645578668 +0800
> +++ bytestream.after 2012-10-26 20:55:26.520578669 +0800
> @@ -1,5 +1,5 @@
>
> -/tmp/bytestream-example.o: file format elf64-x86-64
> +/tmp/bytestream-example.o: file format elf64-x86-64
>
> # So, as you can see, expect the filename, they are same.
>
>
> So, Stefani, is it what you want? Does this looks OK to you?
Perfect. It looks okay for me and i hope for you too ;-)
Acked by [email protected]
Greetings,
Stefani
On Fri, Oct 26, 2012 at 03:42:44PM +0200, Stefani Seibold wrote:
> Am Freitag, den 26.10.2012, 21:04 +0800 schrieb Yuanhan Liu:
> > On Fri, Oct 26, 2012 at 11:26:31AM +0200, Stefani Seibold wrote:
> > > Am Freitag, den 26.10.2012, 15:17 +0800 schrieb Yuanhan Liu:
> > > > On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote:
> > > > > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> > > > > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > > > > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > > > > > > From: Yuanhan Liu <[email protected]>
> > > > > > > >
> > > > > > > > Firstly, this kind of type check doesn't work. It does something similay
> > > > > > > > like following:
> > > > > > > > void * __dummy = NULL;
> > > > > > > > __buf = __dummy;
> > > > > > > >
> > > > > > > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > > > > > > expected.
> > > > > > > >
> > > > > > > > Second, we don't need that kind of check. Since the prototype
> > > > > > > > of __kfifo_out is:
> > > > > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > > > > > > >
> > > > > > > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > > > > > >
> > > > > > > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > > > > > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > > > > > >
> > > > > > > > Cc: Andrew Morton <[email protected]>
> > > > > > > > Cc: Wei Yang <[email protected]>
> > > > > > > > Cc: Stefani Seibold <[email protected]>
> > > > > > > > Cc: Fengguang Wu <[email protected]>
> > > > > > > > Cc: Stephen Rothwell <[email protected]>
> > > > > > > > Signed-off-by: Yuanhan Liu <[email protected]>
> > > > > > > > ---
> > > > >
> >
> > [snip]...
> >
> > > > >
> > > > > Also you have to build the kfifo samples, since this example code use
> > > > > all features of the kfifo API.
> > > > >
> > > > > And again: The kfifo is designed to do the many things at compile time,
> > > > > not at runtime. If you modify the code, you have to check the compiler
> > > > > assembler output for no degradation, especially in kfifo_put, kfifo_get,
> > > > > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
> > > > > if you can do it at compile time. This is the basic reasons to do it in
> > > > > macros.
> > > >
> > > > Is it enought to check kernel/kfifo.o only? I build that file with
> > > > and without this patch. And then dump it by objdump -D kernel/fifo.o to
> > > > /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the
> > > > two dump file are exactly same.
> > > >
> > >
> > > No, since most of the code is inlined due performace reasons, you have
> > > to hack the kfifo examples output code for regressions and code
> > > increase.
> >
> > In my test, this patch doesn't change anything. Here are some data to
> > prove that:
> >
> > $ make samples/kfifo/
> > $ cp samples/kfifo/*.o /tmp/before/
> >
> > $ git am this-patch
> > $ make samples/kfifo/
> > $ cp samples/kfifo/*.o /tmp/after/
> >
> > $ for i in /tmp/before/*.o; do size $i /tmp/after/`basename $i`; done
> > text data bss dec hex filename
> > 1939 464 456 2859 b2b /tmp/before/bytestream-example.o
> > 1939 464 456 2859 b2b /tmp/after/bytestream-example.o
> > text data bss dec hex filename
> > 1423 112 296 1831 727 /tmp/before/dma-example.o
> > 1423 112 296 1831 727 /tmp/after/dma-example.o
> > text data bss dec hex filename
> > 1864 624 376 2864 b30 /tmp/before/inttype-example.o
> > 1864 624 376 2864 b30 /tmp/after/inttype-example.o
> > text data bss dec hex filename
> > 1916 464 472 2852 b24 /tmp/before/record-example.o
> > 1916 464 472 2852 b24 /tmp/after/record-example.o
> > # You will see that it changed nothing.
> >
> >
> > $ objdump -d /tmp/before/bytestream-example.o >/tmp/bytestream-example.before
> > $ objdump -d /tmp/after/bytestream-example.o >/tmp/bytestream-example.after
> > $ diff /tmp/bytestream.before /tmp/bytestream.after -urN
> > --- bytestream.before 2012-10-26 20:55:33.645578668 +0800
> > +++ bytestream.after 2012-10-26 20:55:26.520578669 +0800
> > @@ -1,5 +1,5 @@
> >
> > -/tmp/bytestream-example.o: file format elf64-x86-64
> > +/tmp/bytestream-example.o: file format elf64-x86-64
> >
> > # So, as you can see, expect the filename, they are same.
> >
> >
> > So, Stefani, is it what you want? Does this looks OK to you?
>
> Perfect. It looks okay for me and i hope for you too ;-)
>
> Acked by [email protected]
Thanks for that. Well, I checked this code a bit more, and found I
forgot to remove something: ptr and const_ptr, which were used for type
checking only. Since we are gonna remove type checking, we don't need
them, too.
I did same allmodconfig build, size, objdump test to all kfifo users for
v2 patch as well, and found no new warning, error or assembler changes.
Will sent out v2 soon. And sorry for not noticing this issue early. :(
Thanks,
Yuanhan Liu
On Sat, Oct 27, 2012 at 09:55:58AM +0800, Richard Yang wrote:
> On Fri, Oct 26, 2012 at 08:31:38PM +0800, Yuanhan Liu wrote:
> >On Fri, Oct 26, 2012 at 05:52:44PM +0800, Richard Yang wrote:
> >> On Fri, Oct 26, 2012 at 02:11:45PM +0800, Yuanhan Liu wrote:
> >> >On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> >> >> Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> >> >> > From: Yuanhan Liu <[email protected]>
> >> >> >
> >> >> > Firstly, this kind of type check doesn't work. It does something similay
> >> >> > like following:
> >> >> > void * __dummy = NULL;
> >> >> > __buf = __dummy;
> >> >> >
> >> >> > __dummy is defined as void *. Thus it will not trigger warnings as
> >> >> > expected.
> >> >> >
> >> >> > Second, we don't need that kind of check. Since the prototype
> >> >> > of __kfifo_out is:
> >> >> > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> >> >> >
> >> >> > buf is defined as void *, so we don't need do the type check. Remove it.
> >> >> >
> >> >> > LINK: https://lkml.org/lkml/2012/10/25/386
> >> >> > LINK: https://lkml.org/lkml/2012/10/25/584
> >> >> >
> >> >> > Cc: Andrew Morton <[email protected]>
> >> >> > Cc: Wei Yang <[email protected]>
> >> >> > Cc: Stefani Seibold <[email protected]>
> >> >> > Cc: Fengguang Wu <[email protected]>
> >> >> > Cc: Stephen Rothwell <[email protected]>
> >> >> > Signed-off-by: Yuanhan Liu <[email protected]>
> >> >> > ---
> >> >> > include/linux/kfifo.h | 20 --------------------
> >> >> > 1 file changed, 20 deletions(-)
> >> >> >
> >> >> > diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> >> >> > index 10308c6..b8c1d03 100644
> >> >> > --- a/include/linux/kfifo.h
> >> >> > +++ b/include/linux/kfifo.h
> >> >> > @@ -390,10 +390,6 @@ __kfifo_int_must_check_helper( \
> >> >> > unsigned int __ret; \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) { \
> >> >> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> >> >> > - __dummy = (typeof(__val))NULL; \
> >> >> > - } \
> >> >> > if (__recsize) \
> >> >> > __ret = __kfifo_in_r(__kfifo, __val, sizeof(*__val), \
> >> >> > __recsize); \
> >> >> > @@ -432,8 +428,6 @@ __kfifo_uint_must_check_helper( \
> >> >> > unsigned int __ret; \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) \
> >> >> > - __val = (typeof(__tmp->ptr))0; \
> >> >> > if (__recsize) \
> >> >> > __ret = __kfifo_out_r(__kfifo, __val, sizeof(*__val), \
> >> >> > __recsize); \
> >> >> > @@ -473,8 +467,6 @@ __kfifo_uint_must_check_helper( \
> >> >> > unsigned int __ret; \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) \
> >> >> > - __val = (typeof(__tmp->ptr))NULL; \
> >> >> > if (__recsize) \
> >> >> > __ret = __kfifo_out_peek_r(__kfifo, __val, sizeof(*__val), \
> >> >> > __recsize); \
> >> >> > @@ -512,10 +504,6 @@ __kfifo_uint_must_check_helper( \
> >> >> > unsigned long __n = (n); \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) { \
> >> >> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> >> >> > - __dummy = (typeof(__buf))NULL; \
> >> >> > - } \
> >> >> > (__recsize) ?\
> >> >> > __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> >> >> > __kfifo_in(__kfifo, __buf, __n); \
> >> >> > @@ -565,10 +553,6 @@ __kfifo_uint_must_check_helper( \
> >> >> > unsigned long __n = (n); \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) { \
> >> >> > - typeof(__tmp->ptr) __dummy = NULL; \
> >> >> > - __buf = __dummy; \
> >> >> > - } \
> >> >> > (__recsize) ?\
> >> >> > __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
> >> >> > __kfifo_out(__kfifo, __buf, __n); \
> >> >> > @@ -777,10 +761,6 @@ __kfifo_uint_must_check_helper( \
> >> >> > unsigned long __n = (n); \
> >> >> > const size_t __recsize = sizeof(*__tmp->rectype); \
> >> >> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> >> >> > - if (0) { \
> >> >> > - typeof(__tmp->ptr) __dummy __attribute__ ((unused)) = NULL; \
> >> >> > - __buf = __dummy; \
> >> >> > - } \
> >> >> > (__recsize) ? \
> >> >> > __kfifo_out_peek_r(__kfifo, __buf, __n, __recsize) : \
> >> >> > __kfifo_out_peek(__kfifo, __buf, __n); \
> >> >>
> >> >> Did you tried to compile the whole kernel including all the drivers with
> >> >> your patch?
> >> >
> >> >Hi Stefani,
> >> >
> >> >I did a build test, it did't introduce any new compile errors and
> >> >warnings. While, I haven't tried make allmodconfig then. Does this patch
> >> >seems wrong to you?
> >>
> >> Hmm, in my mind, those warnings are produced by the code you removed.
> >> So it is reasonable that you see no new warnings.
> >
> >Hi Yang,
> >
> >Nope. It's not hard to tell they are old warnings. And as you can see from
> >the email I posted, I did a build compile with and without applying this
> >patch, and does make C=2 2>/tmp/out.{before,after}. I then compared the
> >two file, they are exactly same. So, no new warnings introduced.
> >
>
> I don't understand this mail https://lkml.org/lkml/2012/10/25/584.
>
> So Andrew means just remove the type check?
Hi,
What do you mean by 'just'?
Thanks,
Yuanhan Liu