From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: PATastic fun Date: Tue, 26 Feb 2013 12:43:13 -0500 Message-ID: <20130226174313.GC23731@phenom.dumpdata.com> References: <51277888.50908@canonical.com> <20130222145316.GB8017@phenom.dumpdata.com> <512B2A7C.1050906@canonical.com> <512CD595.2000502@canonical.com> <20130226170330.GB22422@phenom.dumpdata.com> <512CECCE.3070305@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <512CECCE.3070305@canonical.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefan Bader Cc: "Liu, Jinsong" , Colin Ian King , "xen-devel@lists.xensource.com" , Sander Eikelenboom List-Id: xen-devel@lists.xenproject.org On Tue, Feb 26, 2013 at 06:11:42PM +0100, Stefan Bader wrote: > On 26.02.2013 18:03, Konrad Rzeszutek Wilk wrote: > > On Tue, Feb 26, 2013 at 04:32:37PM +0100, Stefan Bader wrote: > >> On 25.02.2013 10:10, Stefan Bader wrote: > >>> On 25.02.2013 04:15, Liu, Jinsong wrote: > >>>> Konrad Rzeszutek Wilk wrote: > >>>>> On Fri, Feb 22, 2013 at 02:54:16PM +0100, Stefan Bader wrote: > >>>>>> Hi Konrad, > >>>>> > >>>>> Hey Stefan, > >>>>>> > >>>>>> here is another one from the hm-what? department: > >>>>> > >>>>> Heh - the really good-bug-hunting one. Lets also include Jinsong as > >>>>> he has been tracking a similar one with mcelog. > >>>>>> > >>>>>> Colin discovered that running the attached program with the fork > >>>>>> active (e.g. "./mmap-example -f 0x10000", the address can be that or > >>>>>> iomem) this triggers the following weird messages: > >>>>>> > >>>>>> [ 6824.453724] mmap-example:3481 map pfn expected mapping type > >>>>>> write-back for [mem 0x00010000-0x00010fff], got uncached-minus > >>>>>> [ 6824.453776] ------------[ cut here ]------------ > >>>>>> [ 6824.453796] WARNING: at > >>>>>> /build/buildd/linux-3.8.0/arch/x86/mm/pat.c:774 > >>>>>> untrack_pfn+0xb8/0xd0() ... [ 6824.453920] Pid: 3481, comm: > >>>>>> mmap-example Tainted: GF > >>>>>> 3.8.0-6-generic #13-Ubuntu > >>>>>> [ 6824.453926] Call Trace: > >>>>>> [ 6824.453944] [] warn_slowpath_common+0x7f/0xc0 > >>>>>> [ 6824.453954] [] warn_slowpath_null+0x1a/0x20 > >>>>>> [ 6824.453963] [] untrack_pfn+0xb8/0xd0 > >>>>>> [ 6824.453975] [] unmap_single_vma+0xac/0x100 > >>>>>> [ 6824.453985] [] unmap_vmas+0x49/0x90 > >>>>>> [ 6824.453995] [] exit_mmap+0x98/0x170 > >>>>>> [ 6824.454007] [] mmput+0x64/0x100 > >>>>>> [ 6824.454017] [] dup_mm+0x445/0x660 > >>>>>> [ 6824.454027] [] > >>>>>> copy_process.part.22+0xa5f/0x1510 [ 6824.454038] > >>>>>> [] do_fork+0x91/0x350 [ 6824.454048] > >>>>>> [] sys_clone+0x16/0x20 [ 6824.454060] > >>>>>> [] stub_clone+0x69/0x90 [ 6824.454069] > >>>>>> [] ? system_call_fastpath+0x1a/0x1f [ 6824.454076] > >>>>>> ---[ end trace 4918cdd0a4c9fea4 ]--- > >>>>>> > >>>>>> I found that this is related to your bandaid patch > >>>>>> > >>>>>> commit 8eaffa67b43e99ae581622c5133e20b0f48bcef1 > >>>>>> Author: Konrad Rzeszutek Wilk > >>>>>> Date: Fri Feb 10 09:16:27 2012 -0500 > >>>>>> > >>>>>> xen/pat: Disable PAT support for now. > >>>>>> > >>>>>> I just do not understand how this happens. From the trace it seems > >>>>>> the fork > >>>>>> fails when duplicating the VMAs (dup_mm calls mmput on failure). So > >>>>>> maybe the > >>>>>> warning is just related to this. So primarily the question is how on > >>>>>> fork the _PAGE_PCD bit can become set? That and _PAGE_PWT are > >>>>>> cleared from the supported > >>>>>> mask by the patch, so somehow I would think nothing should be able > >>>>>> to set it... > >>>>>> But apparently not so. > >>>>>> Not sure it is a big deal since I never saw this in normal operation > >>>>>> and it > >>>>>> seems to be ok when unapping before doing the fork. It is just plain > >>>>>> odd. > >>>>> > >>>>> Jinsong mentioned that there is some oddity with the MTRR. Somehow the > >>>>> ranges are swapped or not correct. Jinsong, could you shed some light > >>>>> on what you have found so far? > >>>>> > >>>> > >>>> Yes, Sander once also reported a similar weird warning when start mcelog daemon, as attached. > >>>> > >>>> Basically, it occurs when mcelog user daemon start, > >>>> do_fork > >>>> --> copy_process > >>>> --> dup_mm > >>>> --> dup_mmap > >>>> --> copy_page_range > >>>> --> track_pfn_copy > >>>> --> reserve_pfn_range > >> > >> So that makes it clearer as this will do > >> > >> reserve_memtype(...) > >> --> pat_x_mtrr_type > >> --> mtrr_type_lookup > >> --> __mtrr_type_lookup > >> > >> And that can return -1/0xff in case of mtrr not being enabled/initialized. Which > >> is not the case (given there are no messages for it in dmesg). This is not equal > >> to MTRR_TYPE_WRBACK and thus becomes _PAGE_CACHE_UC_MINUS. > >> > >> It looks like the problem starts early in reserve_memtype: > >> > >> if (!pat_enabled) { > >> /* This is identical to page table setting without PAT */ > >> if (new_type) { > >> if (req_type == _PAGE_CACHE_WC) > >> *new_type = _PAGE_CACHE_UC_MINUS; > >> else > >> *new_type = req_type & _PAGE_CACHE_MASK; > >> } > >> return 0; > >> } > >> > >> This would be what we want, but only clearing the PWT and PCD flags from the > >> supported flags is not changing pat_enabled (which is 1 when PAT support is > >> compiled into the kernel). Unfortunately the variable is local and since there > >> are not any messages about PAT in dmesg I would say pat_init() is not called > >> either. Which might be used to disable PAT support by clearing the CPU feature > >> flag. > > > > Hm, so: > > > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > > index 39928d1..9ac70c5 100644 > > --- a/arch/x86/xen/enlighten.c > > +++ b/arch/x86/xen/enlighten.c > > @@ -379,6 +379,7 @@ static void __init xen_init_cpuid_mask(void) > > > > cpuid_leaf1_edx_mask = > > ~((1 << X86_FEATURE_MTRR) | /* disable MTRR */ > > + (1 << X86_FEATURE_PAT) | /* disable PAT */ > > (1 << X86_FEATURE_ACC)); /* thermal monitoring */ > > > > if (!xen_initial_domain()) > > > > > > > > should do it right? Let me check on the troublesome machine I saw > > it. > > > I could try it as well but somehow my reading of pat_init() is that if that > would have been called at all, there should be some message about PAT in dmesg. > And normal dom0 boots do not show anything. > > It looks like pat_init only gets called from two places in mtrr code, and that > probably is not done in dom0 either. So clearing the feature is one step, but I > would think that also there needs to be a call to pat_init... So how about this super-complex patch: diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index 39928d1..c8e1c7b 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -67,6 +67,7 @@ #include #include #include +#include #ifdef CONFIG_ACPI #include @@ -1417,7 +1418,14 @@ asmlinkage void __init xen_start_kernel(void) */ acpi_numa = -1; #endif - +#ifdef CONFIG_X86_PAT + /* + * For right now disable the PAT. We should remove this once + * git commit 8eaffa67b43e99ae581622c5133e20b0f48bcef1 + * (xen/pat: Disable PAT support for now) is reverted. + */ + pat_enabled = 0; +#endif /* Don't do the full vcpu_info placement stuff until we have a possible map and a non-dummy shared_info. */ per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0]; trying it out now. > > -Stefan >