On Mon, Aug 5, 2013 at 10:58 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > [+cc Yinghai] > > After the loop we have this (added by Yinghai in fd5913411): > > if (min_align > io_align) > min_align = io_align; > > I don't understand this. There are three cases: > > 1) No downstream bridges: min_align <= 256 because I/O BARs are > limited to 256 bytes. > 2) All downstream bridges have 1K extension: min_align = 1024. > 3) At least one downstream bridge requires 4K alignment: min_align = 4096. > > "io_align" is the minimum alignment enforced by the bridge. This is > independent of any devices below the bridge, so "io_align >= 1024" in > all cases. > > Therefore, the "min_align = io_align" assignment only happens in case > 3, when a downstream bridge requires 4K alignment and *this* bridge > supports the 1K extension. But that seems wrong. The downstream > bridge's 4K requirement also applies to all bridges all the way > upstream. > > It looks to me like this test should instead be: > > if (min_align < io_align) > min_align = io_align; before my change, min_align always set to 4096. in the loop to get min_align, dev resource could have size bigger than 4k, those resource will have SIZEALIGN, so final min_align could be more than 4096. so at last we cap it to 4096 again. But according to your findings: "I/O BARs are limited to 256 bytes" we should not worry about that. So we should just drop io_align and checking like attached patch. (min_align is already set to 1k or 4k before the looping.) and that should solve Wei Yang's problem. Thanks Yinghai
Attachment:
fix_io_align_checking.patch
Description: Binary data
