From owner-svn-src-all@freebsd.org Thu Nov 5 16:08:39 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8E1B9A26007; Thu, 5 Nov 2015 16:08:39 +0000 (UTC) (envelope-from jason.harmening@gmail.com) Received: from mail-lf0-x22f.google.com (mail-lf0-x22f.google.com [IPv6:2a00:1450:4010:c07::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F30B51E09; Thu, 5 Nov 2015 16:08:38 +0000 (UTC) (envelope-from jason.harmening@gmail.com) Received: by lfgh9 with SMTP id h9so59697124lfg.1; Thu, 05 Nov 2015 08:08:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=FKEvlqPQ/PoYUBdN5MMFGZY+mjM1qQjs+i/Kz0126WE=; b=tDaAcT72qQ8rlUCNZk3RoKAg/Zf1j8Ba9h6rC5s64NrKrCAQPnpiZV7Xk8PCSh1aUM wQ/1dr7uphroL9gwA9v7Fo+7P99s7lBJQPP4asztabXLnJ+y1170Wu3byHCCMS8a8yf7 d6JSfNHhxSnYBBh/MUN6nFTBh1Gf/eOk9uloPGPvuJM81M8/8ExvaEO525+cTS1Eg/v6 oFlVJYxIaCx0F9Ri9ObnZuQDAON8s8ZuwftBukJECxWbx0L3qZgQxJLJfE0M1tujSibz Kpyf6jtCEddD9n+5bRmT/VLkY01sGHxrXcF5J1DJ3/1XULcwT2pu5gHPTdZYiZFDb73I 28xg== MIME-Version: 1.0 X-Received: by 10.25.161.16 with SMTP id k16mr2403833lfe.31.1446739716940; Thu, 05 Nov 2015 08:08:36 -0800 (PST) Received: by 10.112.126.104 with HTTP; Thu, 5 Nov 2015 08:08:36 -0800 (PST) In-Reply-To: References: <201510221638.t9MGc1cc053885@repo.freebsd.org> <56348FF8.3010602@gmail.com> <1446394311.91534.236.camel@freebsd.org> Date: Thu, 5 Nov 2015 08:08:36 -0800 Message-ID: Subject: Re: svn commit: r289759 - in head/sys/arm: arm include From: Jason Harmening To: Svatopluk Kraus Cc: Ian Lepore , Ganbold Tsagaankhuu , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Nov 2015 16:08:39 -0000 Userspace buffers in load_buffer() also need temporary mappings, so it might be nice to keep the panic/KASSERT there for completeness. I don't see how anything coming from userspace would be outside vm_page_array though, so that is up to you and Michal. Since Michal's already made the initial patch, I think he should make the commit. That seems only right, plus I'm still busy trying to get everything set up at my new place. I'd like to be on the phabricator review, though. On Thu, Nov 5, 2015 at 3:58 AM, Svatopluk Kraus wrote: > On Thu, Nov 5, 2015 at 1:11 AM, Jason Harmening > wrote: > > > > On Wed, Nov 4, 2015 at 2:17 PM, Svatopluk Kraus > wrote: > >> > >> On Tue, Nov 3, 2015 at 8:45 AM, Jason Harmening > >> wrote: > >> > > >> > On Sun, Nov 1, 2015 at 8:11 AM, Ian Lepore wrote: > >> >> > >> >> > >> >> It's almost certainly not related to sysinit ordering. This > exception > >> >> is happening during mmc probing after interrupts are enabled. > >> >> > >> >> It appears that the problem is the faulting code is running on one of > >> >> the very early pre-allocated kernel stacks (perhaps in an interrupt > >> >> handler on an idle thread stack), and these stacks are not in memory > >> >> represented in the vm page arrays. The mmc code is using a 64-byte > >> >> buffer on the stack and mapping it for DMA. Normally that causes a > >> >> bounce for cacheline alignment, but unluckily in this case that > buffer > >> >> on the stack just happened to be aligned to a cacheline boundary and > a > >> >> multiple of the cacheline size, so no bounce. That causes the new > sync > >> >> logic that is based on keeping vm_page_t pointers and offsets to get > a > >> >> NULL pointer back from PHYS_TO_VM_PAGE when mapping, then it dies at > >> >> sync time trying to dereference that. It used to work because the > sync > >> >> logic used to use the vaddr, not a page pointer. > >> >> > >> >> Michal was working on a patch yesterday. > >> >> > >> > > >> > Ah, thanks for pointing that out Ian. I was left scratching my head > >> > (admittedly on the road and w/o easy access to the code) wondering > what > >> > on > >> > earth would be trying to do DMA during SI_SUB_CPU. > >> > > >> > >> > >> Using of fictitious pages is not so easy here as in case pointed by > >> kib@ where they are allocated and freed inside one function. For sync > >> list sake, they must be allocated when a buffer is loaded and freed > >> when is unloaded. > >> > >> Michal uses pmap_kextract() in case when KVA of buffer is not zero in > >> his proof-of-concept patch: > >> https://gist.github.com/strejda/d5ca3ed202427a2e0557 > >> When KVA of buffer is not zero, temporary mapping is not used at all, > >> so vm_page_t array is not needed too. > >> > >> IMO, buffer's physical address can be saved in sync list to be used > >> when its KVA is not zero. Thus pmap_kextract() won't be called in > >> dma_dcache_sync(). > > > > > > I like the idea of storing off the physical address. If you want to save > > space in the sync list, I think you can place busaddr and pages in a > union, > > using vaddr == 0 to select which field to use. Some people frown upon > use > > of unions, though. > > > > Any reason the panics on PHYS_TO_VM_PAGE in load_buffer() and load_phys() > > shouldn't be KASSERTs instead? > > > KASSERTs are fine. I have looked at Michal's patch again and only > KASSERT should be in load_phys() as such buffers are unmapped, > temporary mapping must be used and so, they must be in vm_page_array. > And maybe some inline function for getting PA from sync list would be > nice too. I would like to review final patch if you are going to make > it. And it would be nice if Ganbold will test it. Phabricator? >