Skip to contentSkip to conversation

On patch "Remove all #inclusions of asm/system.h"

DISCUSSION Stefan Richter 2 participants 4 messages Mar 29, 2012 – Apr 1, 2012

lore
Share & tools
lore
#0 lore raw

Hi David,

commit 9ffc93f2 "Remove all #inclusions of asm/system.h" removes a lot of includes while its parent commit 96f951ed "Add #includes needed to permit the removal of asm/system.h" adds only a few. Some of the files modified by commit 9ffc93f2 now only work due to indirect inclusion, don't they?

For example, drivers/firewire/{core-device,core-topology,ohci,sbp2}.c use smp_rmb()¹ and used to include asm/system.h. Now they do not include asm/barrier.h. Is this by mistake or on purpose?

(I only noticed the commit come in; I did not get any build error yet. The mentioned files now get asm/barrier.h via linux/spinlock.h and certainly via other ways as well.)

--------
¹) so does drivers/firewire/net.c but it missed to include asm/system.h

Signature · 3 lines
Stefan Richter
-=====-===-- --== ===-=
http://arcgraph.de/sr/
#1 lore raw
Quoted 8 lines · Stefan Richter

commit 9ffc93f2 "Remove all #inclusions of asm/system.h" removes a lot of includes while its parent commit 96f951ed "Add #includes needed to permit the removal of asm/system.h" adds only a few. Some of the files modified by commit 9ffc93f2 now only work due to indirect inclusion, don't they?

For example, drivers/firewire/{core-device,core-topology,ohci,sbp2}.c use smp_rmb()¹ and used to include asm/system.h. Now they do not include asm/barrier.h. Is this by mistake or on purpose?

I tried to make sure allyesconfig worked for x86_64 and a bunch of defconfigs worked. I can't guarantee that that got 100% coverage. I also knew there would be some breakage from the base Linux kernel having moved on by the time Linus pulled myu patches - though I don't know if this is the case here (I suspect not).

David

#2 lore raw
Quoted 10 lines · On Mar 31 David Howells wrote:
Quoted 3 lines · Stefan Richter

For example, drivers/firewire/{core-device,core-topology,ohci,sbp2}.c use smp_rmb()¹ and used to include asm/system.h. Now they do not include asm/barrier.h. Is this by mistake or on purpose?

I tried to make sure allyesconfig worked for x86_64 and a bunch of defconfigs worked. I can't guarantee that that got 100% coverage. I also knew there would be some breakage from the base Linux kernel having moved on by the time Linus pulled myu patches - though I don't know if this is the case here (I suspect not).

It's not a problem; they now get barrier.h via spinlock.h. I just wondered whether absence of #include <asm/barrier.h> in those files had a deeper non-obvious meaning. Since it doesn't I will just put that include in them next time around when I do some housekeeping there.

Signature · 3 lines
Stefan Richter
-=====-===-- --== =====
http://arcgraph.de/sr/
#3 lore raw
Quoted 4 lines · Stefan Richter

It's not a problem; they now get barrier.h via spinlock.h. I just wondered whether absence of #include <asm/barrier.h> in those files had a deeper non-obvious meaning. Since it doesn't I will just put that include in them next time around when I do some housekeeping there.

I suspect they should #include asm/barrier.h directly rather than relying on spinlock.h. There's no guarantee that on every arch spinlock.h will pull in asm/barrier.h, I think...

David