Re: [PATCH] arch/ppc/8xx_io/enet.c

Re: [PATCH] arch/ppc/8xx_io/enet.c

[Joakim Tjernlund]

On Monday 21 October 2002 15:13, Joakim Tjernlund wrote:
> Hi
>
> Here is a patch to drop the expensive memcpy of received ethernet frames in
> interrupt context. I have not done any bench marking, but mounting a NFS
> rootfs feels faster.
>
> I am using a heavily modified enet.c in my system, but I think I got the
> patch correct.
>
> Also fixed a bug in set_multicast_list(), move the dmi list forward when
> walking it(dmi = dmi->next;)
>
> Comments? Anyone care to do some benchmarking?

No comments so far, no one interested in this?

A question, why do enet.c pass bad frames to netif_rx? Only late collisions
are disregarded, the rest is passed on.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] arch/ppc/8xx_io/enet.c

[Ricardo Scop]

On Wednesday 23 October 2002 05:51, Joakim Tjernlund wrote:
> On Monday 21 October 2002 15:13, Joakim Tjernlund wrote:
> > Hi
> >
> > Here is a patch to drop the expensive memcpy of received ethernet frames
> > in interrupt context. I have not done any bench marking, but mounting a
> > NFS rootfs feels faster.
> >
> > I am using a heavily modified enet.c in my system, but I think I got the
> > patch correct.
> >
> > Also fixed a bug in set_multicast_list(), move the dmi list forward when
> > walking it(dmi = dmi->next;)
> >
> > Comments? Anyone care to do some benchmarking?
>
> No comments so far, no one interested in this?
I'm interested! Indeed, I adapted and tested your patch in a 8260 FCC fast
ethernet driver and it worked fine  I had a 20% increase in routing
throughput with the  patch installed!

The bug fix in set_multicast_list worked too. But, since there's a continue
clause inside the for command in this routine, I would suggest that the
(dmi=dmi->next) command go inside the for increment clause, instead of the
place you've put it; that is:
	for (...;...; i++, dmi = dmi->next) {...}
>
> A question, why do enet.c pass bad frames to netif_rx? Only late collisions
> are disregarded, the rest is passed on.
IMHO, this is another bug...

>

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] arch/ppc/8xx_io/enet.c

[Ricardo Scop]

On Wednesday 23 October 2002 13:32, Ricardo Scop wrote:
> On Wednesday 23 October 2002 05:51, Joakim Tjernlund wrote:
[snip]

> > No comments so far, no one interested in this?
>
> I'm interested! Indeed, I adapted and tested your patch in a 8260 FCC fast
> ethernet driver and it worked fine  I had a 20% increase in routing
> throughput with the  patch installed!
>
I forgot to mention the bad news. The increase mentioned above was obtained
with large packets (1500 bytes) traversing our board. With very small packets
(64 bytes), we  experimented a 10% DEcrease in PPS (packets per second).
We're still testing, but I guess the overall performance will be better with
the patch installed.

-Scop

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

Hi again

I trying to understand how the different mem*() and *_page() functions in arch/ppc/kernel/misc.S
and arch/ppc/lib/string.S works wrt data cache.

I found that on 8xx these are not used. Why? Are there 8xx variants that can't handle them?
Do 860 work with these?

Also, why is not dcbtst(write a cache line) used
in the different copy functions, copy_page()for example, instead of the COPY_16_BYTES macro?

  Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

It's me again

No response so far. Trying again.

     Jocke
>
> Hi again
>
> I trying to understand how the different mem*() and *_page() functions in arch/ppc/kernel/misc.S
> and arch/ppc/lib/string.S works wrt data cache.
>
> I found that on 8xx these are not used. Why? Are there 8xx variants that can't handle them?
> Do 860 work with these?
>
> Also, why is not dcbtst(write a cache line) used
> in the different copy functions, copy_page()for example, instead of the COPY_16_BYTES macro?
>
>   Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Dan Malek]

Joakim Tjernlund wrote:

>>I found that on 8xx these are not used. Why? Are there 8xx variants that can't handle them?
>>Do 860 work with these?

The TLB/Exception side effects of using these instructions in the 8xx varied
with different silicon versions.  Rather than have a bunch of unique code for
different silicon revisions (and in some cases the side effects didn't occur
at all so the instructions couldn't be used) it is just easier to not use the
instructions.  Any performance gained by using these instructions was lost
in the special exception handling.

>>Also, why is not dcbtst(write a cache line) used

Because dcbtst performs a load operation on a line you know is going to
replaced.  That generates unnecessary bus traffic and is why the dcbz is
used instead.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> >>I found that on 8xx these are not used. Why? Are there 8xx variants that can't handle them?
> >>Do 860 work with these?
>
> The TLB/Exception side effects of using these instructions in the 8xx varied
> with different silicon versions.  Rather than have a bunch of unique code for
> different silicon revisions (and in some cases the side effects didn't occur
> at all so the instructions couldn't be used) it is just easier to not use the
> instructions.  Any performance gained by using these instructions was lost
> in the special exception handling.

That figures, just out of curiosity I removed those restrictions in misc.S and
string.S on my 860, rev D4 to see what happened.
Result: It hung in init.
Now I exclude 8xx in __copy_tofrom_user in string.S.
Result: Now it works just fine.

It seems that even the newest 8xx chips still has problems. I did not expect that.

 Jocke

>
> >>Also, why is not dcbtst(write a cache line) used
>
> Because dcbtst performs a load operation on a line you know is going to
> replaced.  That generates unnecessary bus traffic and is why the dcbz is
> used instead.

OK, thanks.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Dan Malek]

Joakim Tjernlund wrote:

> It seems that even the newest 8xx chips still has problems. I did not expect that.

It may be the parts are working fine.  I long ago removed the code necessary
to detect the exception conditions when the cache instructions faulted, so
I wouldn't expect them to work properly.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > It seems that even the newest 8xx chips still has problems. I did not expect that.
>
> It may be the parts are working fine.  I long ago removed the code necessary
> to detect the exception conditions when the cache instructions faulted, so
> I wouldn't expect them to work properly.

Aah, that would explain why __copy_tofrom_user doesn't work I guess. But the
rest seems to be fine. Maybe there should be a new config option that
would one could enable for modern 8xx CPUs? With a big warning sign of cource
and default to off. Maybe even add the missing exception handler?

   Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

> > Joakim Tjernlund wrote:
> >
> > > It seems that even the newest 8xx chips still has problems. I did not expect that.
> >
> > It may be the parts are working fine.  I long ago removed the code necessary
> > to detect the exception conditions when the cache instructions faulted, so
> > I wouldn't expect them to work properly.
>
> Aah, that would explain why __copy_tofrom_user doesn't work I guess. But the
> rest seems to be fine. Maybe there should be a new config option that
> would one could enable for modern 8xx CPUs? With a big warning sign of cource
> and default to off. Maybe even add the missing exception handler?
>
>    Jocke

Dan, could you add the missing code for 8xx to detect the exception conditions when
the cache instructions faulted? I would like to give it a try.

Maybe add a new CONFIG option to allow 8xx to use these optimizations when
explicitly enabled. Perhaps it's enough to let it depend on CONFIG_8xx_CPU6, that is,
if CONFIG_8xx_CPU6 is set do NOT enable dcbz et. all, otherwise enable them?

        Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Dan Malek]

Joakim Tjernlund wrote:

> Dan, could you add the missing code for 8xx to detect the exception conditions when
> the cache instructions faulted? I would like to give it a try.

It's not very high on my list of things to do.  As I recall, the VM fault
handler may need knowledge of a dcbz fault and use a different method to
determine the address.  I also have this funny feeling that it isn't a
restartable operation, so we may have to emulate the instruction when
the fault occurs.

I know you can write a test that would show this to be a performance
benefit, which would just copy/zero lots of data, but that's not a real
world application.

IMHO, it's lots of work for little, if any, gain.  It has some specific
uses in specialized applications, but I don't think it's a general speed up
solution.

 > Maybe add a new CONFIG option to allow 8xx to use these optimizations when
> explicitly enabled.

How would you know when to do this?  It seldom appeared in any errata
so you couldn't know for certain if a particular chip version would
allow this.  I suspect people (like I have chosen to do) will just never
use the optimization because the software will always work in this case.
I don't like solutions that depend on luck or trial and error.

I'll do some research into this again, it's been many years since I
thought about it.

	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

----- Original Message -----
From: "Dan Malek" <dan@embeddededge.com>
To: <joakim.tjernlund@lumentis.se>
Cc: <linuxppc-embedded@lists.linuxppc.org>
Sent: den 1 november 2002 18:33
Subject: Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx


> Joakim Tjernlund wrote:
>
> > Dan, could you add the missing code for 8xx to detect the exception conditions when
> > the cache instructions faulted? I would like to give it a try.
>
> It's not very high on my list of things to do.  As I recall, the VM fault
> handler may need knowledge of a dcbz fault and use a different method to
> determine the address.  I also have this funny feeling that it isn't a
> restartable operation, so we may have to emulate the instruction when
> the fault occurs.

I don't follow you here, do you need to do all this for a modern(>=D4) 8xx CPU?
I hope not, but something needs to be done since copytofrom_user does not
work for me.

>
> I know you can write a test that would show this to be a performance
> benefit, which would just copy/zero lots of data, but that's not a real
> world application.
>
> IMHO, it's lots of work for little, if any, gain.  It has some specific
> uses in specialized applications, but I don't think it's a general speed up
> solution.
>
>  > Maybe add a new CONFIG option to allow 8xx to use these optimizations when
> > explicitly enabled.
>
> How would you know when to do this?  It seldom appeared in any errata
> so you couldn't know for certain if a particular chip version would
> allow this.  I suspect people (like I have chosen to do) will just never
> use the optimization because the software will always work in this case.
> I don't like solutions that depend on luck or trial and error.

Pretty much like the 8xx_CPU6 errata. If a user has a CPU with this bug,
he must enable it. If you have a fairly new CPU you don't have to.

The same resoning goes for the new option. If you have a new enough(>=D4) CPU,
then you can enable this optimization(I think?!). I am not talking about enabling
workarounds for non functioning CPUs in runtime. Take me for example,
our boards will never use anything older than D4 since it's a new design.

      Jocke
>
> I'll do some research into this again, it's been many years since I
> thought about it.
>
> -- Dan
>

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Dan Malek]

Joakim Tjernlund wrote:

> I don't follow you here, do you need to do all this for a modern(>=D4) 8xx CPU?

Yes.  On "working" silicon, the dcbz (among others) requires special
processing.....probably enough to ensure it costs more to use this
instruction in a general purpose case.

> I hope not, but something needs to be done since copytofrom_user does not
> work for me.

What do you mean it doesn't work?  It should work fine if you don't
try to use the dcbz.  That is, leave the code alone.  The bigger problem
is the newer glibc library has to be modified to conditionally not use
this as well.

> The same resoning goes for the new option. If you have a new enough(>=D4) CPU,
> then you can enable this optimization(I think?!).

I don't know.  Linux is the only OS that will cause VM faults on a dcbz,
maybe it will never work.  Until someone verifies it works on the newer
silicon we can't assume it will.  This certainly isn't a reason to not use
the processor or not ship products.......

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: dc* (Data Cache) instructions in mem*() and *_page() functions not used on 8xx

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > I don't follow you here, do you need to do all this for a modern(>=D4) 8xx CPU?
>
> Yes.  On "working" silicon, the dcbz (among others) requires special
> processing.....probably enough to ensure it costs more to use this
> instruction in a general purpose case.

hmm, then I asume that a "working" 8xx is different from any other PPC that
uses the dcbz? All other PPC arches are allowed to use the dcbz instruction.

>
> > I hope not, but something needs to be done since copytofrom_user does not
> > work for me.
>
> What do you mean it doesn't work?  It should work fine if you don't
> try to use the dcbz.  That is, leave the code alone.  The bigger problem
> is the newer glibc library has to be modified to conditionally not use
> this as well.

I mean that if I remove the #ifndef CONFIG_8xx restrictions in __copy_fromto_user(),
linux does not work anymore(init hangs). That was the experiment I did earlier in this thread.

clear_page() and copy_page() work fine if I remove #ifndef CONFIG_8xx.
I removed the #ifndef CONFIG_8xx in cacheble_memcpy() and cacheble_memzero() also but
these seems to unused.

>
> > The same resoning goes for the new option. If you have a new enough(>=D4) CPU,
> > then you can enable this optimization(I think?!).
>
> I don't know.  Linux is the only OS that will cause VM faults on a dcbz,
> maybe it will never work.  Until someone verifies it works on the newer
> silicon we can't assume it will.  This certainly isn't a reason to not use
> the processor or not ship products.......

Right, I quite like the 860 and even more the 862P(100MHz) I just got :-)

 Regards
         Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] arch/ppc/8xx_io/enet.c

[Joakim Tjernlund]

> > >
> > > Comments? Anyone care to do some benchmarking?
> >
> > No comments so far, no one interested in this?
> I'm interested! Indeed, I adapted and tested your patch in a 8260 FCC fast
> ethernet driver and it worked fine  I had a 20% increase in routing
> throughput with the  patch installed!

Cool!

>
> The bug fix in set_multicast_list worked too. But, since there's a continue
> clause inside the for command in this routine, I would suggest that the
> (dmi=dmi->next) command go inside the for increment clause, instead of the
> place you've put it; that is:
> 	for (...;...; i++, dmi = dmi->next) {...}

Yes thats better, thanks.
> >
> > A question, why do enet.c pass bad frames to netif_rx? Only late collisions
> > are disregarded, the rest is passed on.
> IMHO, this is another bug...
me too.

       Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] arch/ppc/8xx_io/enet.c

[Steven Scholz]

Ricardo Scop wrote:
>
> On Wednesday 23 October 2002 05:51, Joakim Tjernlund wrote:
> > On Monday 21 October 2002 15:13, Joakim Tjernlund wrote:
> > > Hi
> > >
> > > Here is a patch to drop the expensive memcpy of received ethernet frames
> > > in interrupt context. I have not done any bench marking, but mounting a
> > > NFS rootfs feels faster.
> > >
> > > I am using a heavily modified enet.c in my system, but I think I got the
> > > patch correct.
> > >
> > > Also fixed a bug in set_multicast_list(), move the dmi list forward when
> > > walking it(dmi = dmi->next;)
> > >
> > > Comments? Anyone care to do some benchmarking?
> >
> > No comments so far, no one interested in this?
> I'm interested! Indeed, I adapted and tested your patch in a 8260 FCC fast
> ethernet driver and it worked fine  I had a 20% increase in routing
> throughput with the  patch installed!
> ...

Did I understand that correctly that this patch would work with mith MPC8xx FEC
as well!?!?

Cheers,

Steven

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] arch/ppc/8xx_io/enet.c

[Joakim Tjernlund]

>
> Ricardo Scop wrote:
> >
> > On Wednesday 23 October 2002 05:51, Joakim Tjernlund wrote:
> > > On Monday 21 October 2002 15:13, Joakim Tjernlund wrote:
> > > > Hi
> > > >
> > > > Here is a patch to drop the expensive memcpy of received ethernet frames
> > > > in interrupt context. I have not done any bench marking, but mounting a
> > > > NFS rootfs feels faster.
> > > >
> > > > I am using a heavily modified enet.c in my system, but I think I got the
> > > > patch correct.
> > > >
> > > > Also fixed a bug in set_multicast_list(), move the dmi list forward when
> > > > walking it(dmi = dmi->next;)
> > > >
> > > > Comments? Anyone care to do some benchmarking?
> > >
> > > No comments so far, no one interested in this?
> > I'm interested! Indeed, I adapted and tested your patch in a 8260 FCC fast
> > ethernet driver and it worked fine  I had a 20% increase in routing
> > throughput with the  patch installed!
> > ...
>
> Did I understand that correctly that this patch would work with mith MPC8xx FEC
> as well!?!?

Yes, but you have to adapt it a little.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/