[PATCH] sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

[PATCH] sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

[Benjamin Herrenschmidt]

The sym53c8xx driver, for some reason, seems to mostly re-implement
linux/list.h with added bogosity. The main one is it's implementation
of sym_que_entry which spits warnings with recent gcc's on some
64 bits architectures.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Can somebody with that HW test the patch in case I did something
stupid ? I don't have a symbios controller at hand, it's just that
this driver is part of our defconfig :-)

 drivers/scsi/sym53c8xx_2/sym_misc.h |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- linux-work.orig/drivers/scsi/sym53c8xx_2/sym_misc.h	2008-07-07 10:42:03.000000000 +1000
+++ linux-work/drivers/scsi/sym53c8xx_2/sym_misc.h	2008-07-07 10:44:41.000000000 +1000
@@ -121,9 +121,7 @@ static __inline void sym_que_move(struct
 	}
 }
 
-#define sym_que_entry(ptr, type, member) \
-	((type *)((char *)(ptr)-(unsigned int)(&((type *)0)->member)))
-
+#define sym_que_entry(ptr, type, member) container_of(ptr, type, member)
 
 #define sym_insque(new, pos)		__sym_que_add(new, pos, (pos)->flink)
 

Re: [PATCH] sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

[Grant Grundler]

{+ willy]

On Sun, Jul 6, 2008 at 5:50 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> The sym53c8xx driver, for some reason, seems to mostly re-implement
> linux/list.h with added bogosity. The main one is it's implementation
> of sym_que_entry which spits warnings with recent gcc's on some
> 64 bits architectures.

Yup - the implementation doesn't look so good to me either.

>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Can somebody with that HW test the patch in case I did something
> stupid? I don't have a symbios controller at hand, it's just that
> this driver is part of our defconfig :-)

If willy can't test this soon, I can give it a shot.

>  drivers/scsi/sym53c8xx_2/sym_misc.h |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> --- linux-work.orig/drivers/scsi/sym53c8xx_2/sym_misc.h 2008-07-07 10:42:03.000000000 +1000
> +++ linux-work/drivers/scsi/sym53c8xx_2/sym_misc.h      2008-07-07 10:44:41.000000000 +1000
> @@ -121,9 +121,7 @@ static __inline void sym_que_move(struct
>        }
>  }
>
> -#define sym_que_entry(ptr, type, member) \
> -       ((type *)((char *)(ptr)-(unsigned int)(&((type *)0)->member)))
> -
> +#define sym_que_entry(ptr, type, member) container_of(ptr, type, member)

There are 11 uses of sym_que_entry() macro. Is that too many to
completely remove it's use?
(question for willy to decide I guess).

cheers,
grant

>
>  #define sym_insque(new, pos)           __sym_que_add(new, pos, (pos)->flink)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

[Benjamin Herrenschmidt]

On Sun, 2008-07-06 at 23:35 -0700, Grant Grundler wrote:
> > --- linux-work.orig/drivers/scsi/sym53c8xx_2/sym_misc.h 2008-07-07 10:42:03.000000000 +1000
> > +++ linux-work/drivers/scsi/sym53c8xx_2/sym_misc.h      2008-07-07 10:44:41.000000000 +1000
> > @@ -121,9 +121,7 @@ static __inline void sym_que_move(struct
> >        }
> >  }
> >
> > -#define sym_que_entry(ptr, type, member) \
> > -       ((type *)((char *)(ptr)-(unsigned int)(&((type *)0)->member)))
> > -
> > +#define sym_que_entry(ptr, type, member) container_of(ptr, type, member)
> 
> There are 11 uses of sym_que_entry() macro. Is that too many to
> completely remove it's use?
> (question for willy to decide I guess).

Willy wants to completely remove the queue management. While that might
be the right thing to do, I'd like the small fixup to go in to at least
remove the warnings in the meantime.

Cheers,
Ben.

Re: [PATCH] sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

[Grant Grundler]

On Sun, Jul 6, 2008 at 11:43 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sun, 2008-07-06 at 23:35 -0700, Grant Grundler wrote:
...
>> There are 11 uses of sym_que_entry() macro. Is that too many to
>> completely remove it's use?
>> (question for willy to decide I guess).
>
> Willy wants to completely remove the queue management. While that might
> be the right thing to do, I'd like the small fixup to go in to at least
> remove the warnings in the meantime.

ok - I didn't know the long term plan. That seems reasonable to me.

BTW, I realized this morning sym_que_entry() should really be an
inline function and drop the "struct sym_ccb" argument.

thanks,
grant