Re: pci_set_mwi() ... why isn't it used more?

[David Brownell <david-b@pacbell.net>]

Ivan Kokshaysky wrote:
> On Thu, Jan 30, 2003 at 10:35:25AM -0800, David Brownell wrote:
> 
>>I think the first answer is better, but it looks like 2.5.59 will
>>set the pci cache line size to 16 bytes not 128 bytes in that case.
> 
> 
> Yes, and it looks dangerous as the device would transfer incomplete
> cache lines with MWI...

... while invalidating complete lines, yes that's what I meant about
it causing breakage.


>>Another option would be to do like SPARC64 and set the cacheline
>>sizes as part of DMA enable (which is what I'd first thought of).
>>And have the breakage test in the ARCH_PCI_MWI code -- something
>>that sparc64 doesn't do, fwiw.

It also sets latencies ... one could get the impression that most
PCI code on Linux hasn't been tuned for bus utilization yet!


> Actually I think there is nothing wrong if we'll try to be a bit
> more aggressive with MWI and move all of this into generic
> pci_set_master().
> To do it safely, we need
> - kind of "broken_mwi" field in the struct pci_dev for buggy devices,
>   it can be set either by PCI quirks or by driver before pci_set_master()
>   call;
> - arch-specific pci_cache_line_size() function/macro (instead of
>   SMP_CACHE_BYTES) that returns either actual CPU cache line size
>   or other safe value (including 0, which means "don't enable MWI");
> - check that the device does support desired cache line size, i.e.
>   read back the value that we've written into the PCI_CACHE_LINE_SIZE
>   register and if it's zero (or dev->broken_mwi == 1) don't enable MWI.
> 
> Thoughts?

Sounds plausible to me, but then I was asking about this because
it was evident there were a few more things going on here than I
was aware of.  You imply removing pci_{set,clear}_mwi() too.

I certainly agree that setting the cache line size automatically
should happen; and having that be correct means there's got to
be some arch dependent code.  That alone might help improve
throughput on PCI_DMA_TODEVICE, even on MWI-incapable devices.

It might be appropriate to make aggressively setting MWI be a
Kconfig (or boot) option.  That it "should not" be problematic
doesn't mean that some 2.6 users might not find trouble.

Also, benchmarks from folk with really busy PCI busses would
be interesting.  All this stuff can be tweaked with "setpci",
so they could be generated without needing kernel patches.

- Dave