lkml.org 
[lkml]   [2013]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v7u1 26/31] x86: Don't enable swiotlb if there is not enough ram for it
From
On Fri, Jan 4, 2013 at 3:47 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Shuah Khan <shuahkhan@gmail.com> writes:
>
>> On Fri, Jan 4, 2013 at 3:10 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Jan 4, 2013 at 1:02 PM, Shuah Khan <shuahkhan@gmail.com> wrote:
>>>> Pani'cing the system doesn't sound like a good option to me in this
>>>> case. This change to disable swiotlb is made for kdump. However, with
>>>> this change several system fail to boot, unless crashkernel_low=72M is
>>>> specified.
>>>
>>> this patchset is new feature to put second kdump kernel above 4G.
>>>
>> I understand this is just one of the patches to implement the new
>> kdump feature. However, I think regression on existing behavior with a
>> panic is a bit of a big hammer. Thie change causes panic on systems
>> even when kdump is not enabled, if I understand it correctly.
>>
>> Granted kdump gets enabled by several distros, but it is not a
>> required feature. However, expecting system to boot with devices that
>> require swiotlb fully functioning is a basic feature. So I would argue
>> that not breaking the basic functionality is a higher priority over
>> enabling kdump in this case.
>
> Yinghai Lu it looks like your autodetection of the problem case in this
> patch is problematic and needs a rethink. My quick skim says you are
> trying to detect failure too early in the code. Furthermore having
> kexec on panic sized magic comments without explanation is wrong.
>
> Shuah Khan this is motivated by kdump. However a correct implementation
> should be about dealing with the case when there is simply not enough
> memory available below 4G for bounce buffers.
>
> If a device needs an iommu, and swiotlb is the only iommu option, and
> there is not enough memory below 4G panic'ing is entirely reasonable.
>
> Do I read this discussion right that we are waisting 64M on systems
> that have the swiotlb code but don't use the swiotlb?
>

No. pci_swiotlb_late_init() does free reserved swiolb buffers on
systems that don't need swiolb. IOMMU drivers turn off swiotlb after
iommu is initialized correctly. It is possible on some systems when
BIOS is incorrect, iommu initialization could fail and swiotlb is left
enabled.

AMD IOMMU driver is using this lever to leave swiotlb enabled when it
detects devices that can't be supported by iommu. My concern is that
this change for kdump removes that handshake ability between iommu and
swiolb.

-- Shuah


\
 
 \ /
  Last update: 2013-01-05 00:41    [W:0.189 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site