Re: [PATCH/RFC] m68k/bitops: Make bitmap data pointer of atomic ops volatile

2011-06-07 Thread Geert Uytterhoeven
On Mon, Jun 6, 2011 at 22:11, Arnd Bergmann a...@arndb.de wrote:
 On Monday 06 June 2011 22:07:53 Geert Uytterhoeven wrote:

 This fixes a.o.

 drivers/ide/ide-io.c: In function ‘ide_lock_host’:
 drivers/ide/ide-io.c:415: warning: passing argument 2 of 
 ‘__constant_test_and_set_bit’ discards qualifiers from pointer target type
 drivers/ide/ide-io.c:415: warning: passing argument 2 of 
 ‘__generic_test_and_set_bit’ discards qualifiers from pointer target type

 Suggested-by: Ben Hutchings b...@decadent.org.uk
 Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org

 I think the correct fix would be to mark the variable not volatile, as it
 clearly has no business be marked as such. That doesn't mean your patch

You mean the host_busy variable in the IDE code?
That would also apply to context_flag in the DRM code:

drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
‘__constant_test_and_set_bit’ discards qualifiers from pointer target
type
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
‘__generic_test_and_set_bit’ discards qualifiers from pointer target
type

 is wrong, though. It probably doesn't hurt to do both.

asm-generic/bitops/atomic.h has the volatiles everywhere. That's why
I'm wondering.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH/RFC] m68k/bitops: Make bitmap data pointer of atomic ops volatile

2011-06-07 Thread Arnd Bergmann
On Tuesday 07 June 2011, Geert Uytterhoeven wrote:
 You mean the host_busy variable in the IDE code?
 That would also apply to context_flag in the DRM code:
 
 drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
 ‘__constant_test_and_set_bit’ discards qualifiers from pointer target
 type
 drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
 ‘__generic_test_and_set_bit’ discards qualifiers from pointer target
 type

Yes, that fits the same category.

  is wrong, though. It probably doesn't hurt to do both.
 
 asm-generic/bitops/atomic.h has the volatiles everywhere. That's why
 I'm wondering.

I guess what happened is that some variables are traditionally marked
as volatile although they shouldn't be, and most architectures have
adapted their bitops to make the warnings go away. If you see more
warnings of that kind, it's probably fine to just do the same on m68k.
The volatile modifier doesn't really hurt in this case.

Arnd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH/RFC] m68k/bitops: Make bitmap data pointer of atomic ops volatile

2011-06-07 Thread Ben Hutchings
On Tue, Jun 07, 2011 at 01:22:29PM +0200, Arnd Bergmann wrote:
 On Tuesday 07 June 2011, Geert Uytterhoeven wrote:
  You mean the host_busy variable in the IDE code?
  That would also apply to context_flag in the DRM code:
  
  drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
  ‘__constant_test_and_set_bit’ discards qualifiers from pointer target
  type
  drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
  ‘__generic_test_and_set_bit’ discards qualifiers from pointer target
  type
 
 Yes, that fits the same category.
 
   is wrong, though. It probably doesn't hurt to do both.
  
  asm-generic/bitops/atomic.h has the volatiles everywhere. That's why
  I'm wondering.
 
 I guess what happened is that some variables are traditionally marked
 as volatile although they shouldn't be, and most architectures have
 adapted their bitops to make the warnings go away. If you see more
 warnings of that kind, it's probably fine to just do the same on m68k.
 The volatile modifier doesn't really hurt in this case.
 
These operations are required to be atomic and therefore they
must be suitable for use with volatile-qualified variables.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
  - Albert Camus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH/RFC] m68k/bitops: Make bitmap data pointer of atomic ops volatile

2011-06-07 Thread Arnd Bergmann
On Tuesday 07 June 2011, Ben Hutchings wrote:
 On Tue, Jun 07, 2011 at 01:22:29PM +0200, Arnd Bergmann wrote:

  I guess what happened is that some variables are traditionally marked
  as volatile although they shouldn't be, and most architectures have
  adapted their bitops to make the warnings go away. If you see more
  warnings of that kind, it's probably fine to just do the same on m68k.
  The volatile modifier doesn't really hurt in this case.
  
 These operations are required to be atomic and therefore they
 must be suitable for use with volatile-qualified variables.

As I said, it's not wrong for them to have a volatile qualifier in the
argument list. However, there should also not be the need for the
qualifier in any of the callers, because the bitops only work if
all accesses to the data are done through bitops functions, and that
means that the qualifier on the variable is completely meaningless.

Arnd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel