2000-12-09 09:15:34

by Abramo Bagnara

[permalink] [raw]
Subject: [2*PATCH] alpha I/O access and mb()

--- linux/include/asm-alpha/core_t2.h.~1~ Fri Mar 17 22:01:38 2000
+++ linux/include/asm-alpha/core_t2.h Sat Dec 9 09:07:35 2000
@@ -533,6 +533,21 @@
#define __ioremap(a) t2_ioremap((unsigned long)(a))
#define __is_ioaddr(a) t2_is_ioaddr((unsigned long)(a))

+#define inb(p) __inb(p)
+#define inw(p) __inw(p)
+#define inl(p) __inl(p)
+#define outb(x,p) __outb((x),(p))
+#define outw(x,p) __outw((x),(p))
+#define outl(x,p) __outl((x),(p))
+#define __raw_readb(a) __readb(a)
+#define __raw_readw(a) __readw(a)
+#define __raw_readl(a) __readl(a)
+#define __raw_readq(a) __readq(a)
+#define __raw_writeb(v,a) __writeb((v),(a))
+#define __raw_writew(v,a) __writew((v),(a))
+#define __raw_writel(v,a) __writel((v),(a))
+#define __raw_writeq(v,a) __writeq((v),(a))
+
#endif /* __WANT_IO_DEF */

#ifdef __IO_EXTERN_INLINE


Attachments:
alpha-mb-2.2.diff (6.62 kB)
alpha-mb-2.4.diff (837.00 B)
Download all attachments

2000-12-09 11:43:53

by Gérard Roudier

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()



On Sat, 9 Dec 2000, Abramo Bagnara wrote:

>
> Here you are two patches:
>
> alpha-mb-2.2.diff add the missing mb() to the cores that still lack it
> (against 2.2.18pre25)
>
> alpha-mb-2.4.diff add missing defines from core_t2.h for non generic
> kernel (against 2.4.0test11)
>
> Please apply on your trees.
>
> I've also noted that 2.4 uses mb() after read[bwlq] while 2.2 don't.
> Who's right?

Let me howl for a minute. :-)

The actual issue regarding ordering is generally to ensure something to
happen (i.e. to be seen to happen by other agents) _before_ something
else. As a result, what we have in mind is to insert a barrier _before_
this `something else'.

However, everything I seem to see about this issue on our planet and that
applies to IO subsystems is blindly inserting barriers _after_ the
'something'.

Is software getting sci. fi. ? ;-)

Based on that, let me claim that most of blind barriers inserted this way
are useless (thus sob-optimal) and may band-aid useful barriers that are
missing. The result is subtle bugs, hidden most of the time, that we will
have to suffer for decades.

The only way to do things right regarding ordering it to have device
drivers _aware_ of such issues. Now, if we are happy with broken portable
or platform-independant drivers that rely on broken hidden ordering
alchemy rather than on correctness, then it is another story.

G?rard.

2000-12-09 12:03:26

by Abramo Bagnara

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

G?rard Roudier wrote:
>
>
> Based on that, let me claim that most of blind barriers inserted this way
> are useless (thus sob-optimal) and may band-aid useful barriers that are
> missing. The result is subtle bugs, hidden most of the time, that we will
> have to suffer for decades.
>
> The only way to do things right regarding ordering it to have device
> drivers _aware_ of such issues. Now, if we are happy with broken portable
> or platform-independant drivers that rely on broken hidden ordering
> alchemy rather than on correctness, then it is another story.

I see perfectly your point and this is the reason why we have
__raw_write[bwlq] in 2.4, but write[bwlq] expected semantic is to ensure
that write *happens* and are visible by other agents.

You can tell me that almost nobody uses __raw_write now and this is bad
and I agree with you, but sometime this is not a perfect world ;-)

--
Abramo Bagnara mailto:[email protected]

Opera Unica Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is http://www.alsa-project.org
sponsored by SuSE Linux http://www.suse.com

It sounds good!

2000-12-09 15:17:42

by Gérard Roudier

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()



On Sat, 9 Dec 2000, Abramo Bagnara wrote:

> G?rard Roudier wrote:
> >
> >
> > Based on that, let me claim that most of blind barriers inserted this way
> > are useless (thus sob-optimal) and may band-aid useful barriers that are
> > missing. The result is subtle bugs, hidden most of the time, that we will
> > have to suffer for decades.
> >
> > The only way to do things right regarding ordering it to have device
> > drivers _aware_ of such issues. Now, if we are happy with broken portable
> > or platform-independant drivers that rely on broken hidden ordering
> > alchemy rather than on correctness, then it is another story.
>
> I see perfectly your point and this is the reason why we have
> __raw_write[bwlq] in 2.4, but write[bwlq] expected semantic is to ensure
> that write *happens* and are visible by other agents.

Ordering and flushing are different issues. Hardware may flush in order to
guarantee some order, but if nothing is to order it may not. A memory
barrier does not guarantee you that a write will go faster to the system
BUS. Basically, if no other agent does need the data and the data is
cacheable, the flushing is just useless. Confusing ordering and flushing
is a serious mistake in my humble opinion.

Speaking about MMIO which is not cacheable, indeed we want the data
targetting the MMIO area to be flushed quickly. But we also want the
device to have a consistent vue of the data in memory for all IOs it is
provided with. Usually, we have to deal with the following:

1) Prepare some DATA in cachable memory (DMA related)
2) Ensure ordering: i.e. may insert a MEMORY BARRIER
3) Tell the device about the IO to perform: write to MMIO

Drivers that are unaware of (2) _are_ broken.

Or:

1) Read device status register to know about IO that completed.
2) Ensure ordering of speculative reads against DMA from the device.
i.e. may insert a MEMORY (READ) BARRIER.
3) Look into memory for IOs that have completed.

Drivers that are unaware of (2) _are_ broken.

Since the hidden BUS stuff just puts its implicit barriers at the wrong
place regarding the above, any device driver that does DMA and that does
not use explicit barriers is likely to be broken even if it uses normal
IOs. Reason is that the PCI specifications also allow host bridges to post
IO transactions and thus assuming 15 years old ISA-like behaviour is plain
wrong.

> You can tell me that almost nobody uses __raw_write now and this is bad
> and I agree with you, but sometime this is not a perfect world ;-)

The various BUS abstractions I have to suffer of are indeed a great
demonstration of our world being not perfect. ;-)

Hehe... I read so often that most drivers are broken that shoe-horning
bunches or barriers, bus things and other band-aidings is probably the
only way to have some of them mostly usable. ;-)
Or could it be that current O/S guys are still ISA-bussed. ;-)

By the way, given our real world, your patch is probably quite
reasonnable. My point was not to disagree with it, in particular.

G?rard.

2000-12-10 00:41:02

by Richard Henderson

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

On Sat, Dec 09, 2000 at 09:43:00AM +0100, Abramo Bagnara wrote:
> alpha-mb-2.4.diff add missing defines from core_t2.h for non generic
> kernel (against 2.4.0test11)

These are not "missing". They are intentionally not present
so that stuff will be done out of line.


r~

2000-12-10 10:11:14

by Abramo Bagnara

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

Richard Henderson wrote:
>
> On Sat, Dec 09, 2000 at 09:43:00AM +0100, Abramo Bagnara wrote:
> > alpha-mb-2.4.diff add missing defines from core_t2.h for non generic
> > kernel (against 2.4.0test11)
>
> These are not "missing". They are intentionally not present
> so that stuff will be done out of line.

And this would be the only core_*.h files where this intention is
expressed?

It's hard to believe, without you explain why ;-)

--
Abramo Bagnara mailto:[email protected]

Opera Unica Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is http://www.alsa-project.org
sponsored by SuSE Linux http://www.suse.com

It sounds good!

2000-12-10 19:15:10

by Richard Henderson

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

On Sun, Dec 10, 2000 at 10:40:12AM +0100, Abramo Bagnara wrote:
> And this would be the only core_*.h files where this intention is
> expressed?

Not at all. See core_lca.h, jensen.h, core_cia.h, core_mcpcia.h.


r~

2000-12-10 19:35:14

by Abramo Bagnara

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

Richard Henderson wrote:
>
> On Sun, Dec 10, 2000 at 10:40:12AM +0100, Abramo Bagnara wrote:
> > And this would be the only core_*.h files where this intention is
> > expressed?
>
> Not at all. See core_lca.h, jensen.h, core_cia.h, core_mcpcia.h.

I think you're confusing the issues: the only missing things from
core_t2.h are some defines to permit to it to work when CONFIG_ALPHA_T2
is defined.

The with or without mb() issue is handled in io.h and it's common for
all cores.

--
Abramo Bagnara mailto:[email protected]

Opera Unica Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is http://www.alsa-project.org
sponsored by SuSE Linux http://www.suse.com

It sounds good!

2000-12-10 21:20:05

by Richard Henderson

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

On Sun, Dec 10, 2000 at 08:04:07PM +0100, Abramo Bagnara wrote:
> ... the only missing things from core_t2.h are some defines to
> permit to it to work when CONFIG_ALPHA_T2 is defined.

Have you tried it? I did. It works fine.

What _is_ defined in asm/core_t2.h is enough for alpha/lib/io.c to
define out of line functions that asm/io.h then uses.

This same mechanism is used on cia, mcpcia, lca, and jensen.
It has been in place in just this fashion since 2.1.something.


r~

2000-12-10 21:54:14

by Abramo Bagnara

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

Richard Henderson wrote:
>
> On Sun, Dec 10, 2000 at 08:04:07PM +0100, Abramo Bagnara wrote:
> > ... the only missing things from core_t2.h are some defines to
> > permit to it to work when CONFIG_ALPHA_T2 is defined.
>
> Have you tried it? I did. It works fine.
>
> What _is_ defined in asm/core_t2.h is enough for alpha/lib/io.c to
> define out of line functions that asm/io.h then uses.

asm/io.h uses out of line function only when CONFIG_ALPHA_GENERIC is
defined, otherwise it uses (take writel as an example) __raw_writel that
IMHO need to be defined in core_t2.h.

core_t2.h is the only core_*.h file that does not define it and this is
why I've proposed that patch.

Perhaps there is a reason I don't understand and in this case I want to
apologize and I'd like to ask you to explain me what I'm missing.

--
Abramo Bagnara mailto:[email protected]

Opera Unica Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is http://www.alsa-project.org
sponsored by SuSE Linux http://www.suse.com

It sounds good!

2000-12-11 00:50:51

by Richard Henderson

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()

On Sun, Dec 10, 2000 at 10:23:20PM +0100, Abramo Bagnara wrote:
> asm/io.h uses out of line function only when CONFIG_ALPHA_GENERIC is
> defined, otherwise it uses (take writel as an example) __raw_writel that
> IMHO need to be defined in core_t2.h.

Perhaps you should _show_ an actual failure rather than just guessing.

You are wildly incorrect asserting that out of line functions are used
only with CONFIG_ALPHA_GENERIC. You should examine

#ifndef __raw_writel
# define __raw_writel(v,a) ___raw_writel((v),(unsigned long)(a))
#endif

and suchlike definitions.

> core_t2.h is the only core_*.h file that does not define it and this is
> why I've proposed that patch.

FOR THE NTH TIME, NO IT IS NOT!

How often do I have to point you at the other files that do not
define (all of) these macros before you will believe me?

Jesus H Christ! Look at some preprocessor output why don't you?
Better yet, compile the kernel with CONFIG_ALPHA_T2. Notice how
it does not fail with undefined symbol errors. Notice how there
are non-trivial bit fiddling insns implementing the functions in
alpha/lib/io.o. That's the quickest way to see that I'm right
and you aren't.

End of discussion.


r~

2000-12-11 08:32:16

by Abramo Bagnara

[permalink] [raw]
Subject: Re: [2*PATCH] alpha I/O access and mb()


Before of all I want to publicly apologize with Richard as never my
intention was to exacerbate him.

>
> On Sun, Dec 10, 2000 at 10:23:20PM +0100, Abramo Bagnara wrote:
> > asm/io.h uses out of line function only when CONFIG_ALPHA_GENERIC is
> > defined, otherwise it uses (take writel as an example) __raw_writel that
> > IMHO need to be defined in core_t2.h.
>
> Perhaps you should _show_ an actual failure rather than just guessing.

I've not access to this specific hardware but I was trying to fix the
alpha case wrt write[bwlq] function as I've had a lot of trouble with
ALSA drivers and 2.2 (that still now is broken wrt mb() and I've sent a
patch to Alpha Processor guys). This is the reason why I've given a look
to 2.4.

>
> You are wildly incorrect asserting that out of line functions are used
> only with CONFIG_ALPHA_GENERIC. You should examine
>
> #ifndef __raw_writel
> # define __raw_writel(v,a) ___raw_writel((v),(unsigned long)(a))
> #endif
>
> and suchlike definitions.

Now I understand, thanks.

I see that some core*.h leaves out of line some functions because they
are more complex than a choosen threshold.

--
Abramo Bagnara mailto:[email protected]

Opera Unica Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is http://www.alsa-project.org
sponsored by SuSE Linux http://www.suse.com

It sounds good!