Date: Wed, 24 Dec 2014 18:31:24 -0700 From: Ian Lepore <ian@freebsd.org> To: Rui Paulo <rpaulo@me.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andrew Turner <andrew@fubar.geek.nz> Subject: Re: svn commit: r276187 - head/sys/arm/arm Message-ID: <1419471084.1018.160.camel@freebsd.org> In-Reply-To: <8E8B7FE3-0C97-4A84-BC1D-1C5A0E732D0C@me.com> References: <201412241712.sBOHCqvW039381@svn.freebsd.org> <20141224222637.03a19e57@bender> <1419460812.1018.157.camel@freebsd.org> <8E8B7FE3-0C97-4A84-BC1D-1C5A0E732D0C@me.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 2014-12-24 at 17:05 -0800, Rui Paulo wrote: > On Dec 24, 2014, at 14:40, Ian Lepore <ian@FreeBSD.org> wrote: > > > > On Wed, 2014-12-24 at 22:26 +0000, Andrew Turner wrote: > >> On Wed, 24 Dec 2014 17:12:52 +0000 (UTC) > >> Ian Lepore <ian@FreeBSD.org> wrote: > >> > >>> Author: ian > >>> Date: Wed Dec 24 17:12:51 2014 > >>> New Revision: 276187 > >>> URL: https://svnweb.freebsd.org/changeset/base/276187 > >>> > >>> Log: > >>> Eliminate unnecessary references to pte.h internals by using the > >>> standard pmap_kenter_temporary() to map pages while dumping. > >>> > >>> Submitted by: Svatopluk Kraus <onwahe@gmail.com>, > >>> Michal Meloun <meloun@miracle.cz> > >>> > >>> Modified: > >>> head/sys/arm/arm/dump_machdep.c > >>> > >>> Modified: head/sys/arm/arm/dump_machdep.c > >>> ============================================================================== > >>> --- head/sys/arm/arm/dump_machdep.c Wed Dec 24 16:11:15 > >>> 2014 (r276186) +++ head/sys/arm/arm/dump_machdep.c Wed > >>> Dec 24 17:12:51 2014 (r276187) @@ -160,11 +160,13 @@ static int > >>> cb_dumpdata(struct md_pa *mdp, int seqnr, void *arg) > >>> { > >>> struct dumperinfo *di = (struct dumperinfo*)arg; > >>> - vm_paddr_t pa; > >>> + vm_paddr_t a, pa; > >>> + void *va; > >>> uint32_t pgs; > >>> size_t counter, sz, chunk; > >>> - int c, error; > >>> + int i, c, error; > >>> > >>> + va = 0; > >>> error = 0; /* catch case in which chunk size is 0 */ > >>> counter = 0; > >>> pgs = mdp->md_size / PAGE_SIZE; > >>> @@ -194,16 +196,14 @@ cb_dumpdata(struct md_pa *mdp, int seqnr > >>> printf(" %d", pgs * PAGE_SIZE); > >>> counter &= (1<<24) - 1; > >>> } > >>> - if (pa == (pa & L1_ADDR_BITS)) { > >>> - pmap_kenter_section(0, pa & L1_ADDR_BITS, 0); > >>> - cpu_tlb_flushID_SE(0); > >>> - cpu_cpwait(); > >>> + for (i = 0; i < chunk; i++) { > >>> + a = pa + i * PAGE_SIZE; > >>> + va = pmap_kenter_temporary(trunc_page(a), i); > >> > >> Is this correct? It may map multiple chunks here. > >>> } > >>> #ifdef SW_WATCHDOG > >>> wdog_kern_pat(WD_LASTVAL); > >>> #endif > >>> - error = dump_write(di, > >>> - (void *)(pa - (pa & L1_ADDR_BITS)),0, dumplo, > >>> sz); > >>> + error = dump_write(di, va, 0, dumplo, sz); > >> > >> Then uses the last virtual address to dump the data here. Wouldn't this > >> miss the chunks mapped other than the last one? > > > > Yeah, but a quirk of pmap_kenter_temporary() is that its return value is > > constant, so this wrong-looking code is actually right. > > I'm a bit surprised about this. This most likely warrants a comment. In every architecture and every place it's used, or just arm just here? This appears to be an idiom, or at least something that has been pasted in identical form in every arch so far. -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1419471084.1018.160.camel>