Re: bitmaps/bitops

Re: bitmaps/bitops

[Pete Zaitcev]

> but the prototype for test_and_set_bit() depends on $(ARCH), and it's
> not consistent, with the second arg (bitmap address) being one of:
>   volatile void *
>   void *
>   volatile unsigned long *

It should be unsigned long pointer. I have no idea why
volatile is still alive. Perhaps Linus can remember why he
left it in on is386. Other arch maintainers midnlessly ape him
in this area. I think I even kept his e-mail where he explains
why volatile has to go.

-- Pete

Re: bitmaps/bitops

[Paul Mackerras]

Pete Zaitcev writes:

> > but the prototype for test_and_set_bit() depends on $(ARCH), and it's
> > not consistent, with the second arg (bitmap address) being one of:
> >   volatile void *
> >   void *
> >   volatile unsigned long *
> 
> It should be unsigned long pointer. I have no idea why
> volatile is still alive. Perhaps Linus can remember why he
> left it in on is386. Other arch maintainers midnlessly ape him
> in this area. I think I even kept his e-mail where he explains
> why volatile has to go.

The volatile is there (at least in ppc) because without it you get
compile warnings.  I don't see why the bitops particularly should be
the anti-volatile police.  I agree that using volatile is usually a
bad idea (unless you are accessing a memory-mapped I/O device).

Paul.

Re: bitmaps/bitops

[Alan Cox]

On Sun, 2003-03-16 at 01:31, Pete Zaitcev wrote:
> > but the prototype for test_and_set_bit() depends on $(ARCH), and it's
> > not consistent, with the second arg (bitmap address) being one of:
> >   volatile void *
> >   void *
> >   volatile unsigned long *
> 
> It should be unsigned long pointer. I have no idea why
> volatile is still alive. Perhaps Linus can remember why he
> left it in on is386. Other arch maintainers midnlessly ape him
> in this area. I think I even kept his e-mail where he explains
> why volatile has to go.

Several 2.4 drivers assume the test_and_set point is a memory
barrier for locking. Lots of

	if(test_and_set_bit(0, &foo))
	{
		x=foodev->blah

Since its inline code and not a memory barrier otherwise there
is little stop the compile doing

	x=foodev->blah

first

bitmaps/bitops

[Randy.Dunlap]

Hi,

Not picking on this code, but as an example:

drivers/ieee1394/ieee1394_transactions.c, line 152 (in 2.5.64),
uses:  test_and_set_bit(bit_number, tp->pool),

where tp->pool is declared by using a DECLARE_BITMAP(), like so:
struct hpsb_tlabel_pool {
	DECLARE_BITMAP(pool, 64);

That makes sense (at least to me), but gcc complains about the
type of <pool> when used in test_and_set_bit():
  drivers/ieee1394/ieee1394_transactions.c:152: warning: passing arg 2 of
`test_and_set_bit' from incompatible pointer type

Is this an error, a bug, a nuisance, or just another "ignore gcc 2.96"
problem?

For reference, DECLARE_BITMAP() generates an array of unsigned longs:
#define DECLARE_BITMAP(name,bits) \
	unsigned long name[BITS_TO_LONGS(bits)]
but the prototype for test_and_set_bit() depends on $(ARCH), and it's
not consistent, with the second arg (bitmap address) being one of:
  volatile void *
  void *
  volatile unsigned long *


If there's (kernel) surgery required here, unless it's trivial, I think
that I'd leave it for early 2.7.

~Randy

Re: bitmaps/bitops

[Geert Uytterhoeven]

On Sat, 15 Mar 2003, Randy.Dunlap wrote:
> Not picking on this code, but as an example:
> 
> drivers/ieee1394/ieee1394_transactions.c, line 152 (in 2.5.64),
> uses:  test_and_set_bit(bit_number, tp->pool),
> 
> where tp->pool is declared by using a DECLARE_BITMAP(), like so:
> struct hpsb_tlabel_pool {
> 	DECLARE_BITMAP(pool, 64);
> 
> That makes sense (at least to me), but gcc complains about the
> type of <pool> when used in test_and_set_bit():
>   drivers/ieee1394/ieee1394_transactions.c:152: warning: passing arg 2 of
> `test_and_set_bit' from incompatible pointer type
> 
> Is this an error, a bug, a nuisance, or just another "ignore gcc 2.96"
> problem?
> 
> For reference, DECLARE_BITMAP() generates an array of unsigned longs:
> #define DECLARE_BITMAP(name,bits) \
> 	unsigned long name[BITS_TO_LONGS(bits)]
> but the prototype for test_and_set_bit() depends on $(ARCH), and it's
> not consistent, with the second arg (bitmap address) being one of:
>   volatile void *
>   void *
>   volatile unsigned long *
> 
> 
> If there's (kernel) surgery required here, unless it's trivial, I think
> that I'd leave it for early 2.7.

Bitops operate on long. Architectures for which it really matters have used
`unsigned long *' in there function signatures since quite some time.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds