Date: Mon, 18 Feb 2013 17:08:09 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Svatopluk Kraus <onwahe@gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: [patch] i386 pmap sysmaps_pcpu[] atomic access Message-ID: <20130218150809.GG2598@kib.kiev.ua> In-Reply-To: <CAFHCsPUVTM9jfrnzY72YsPszLWkg-UaJcycTR4xXcS%2BfPzS1Vg@mail.gmail.com> References: <CAFHCsPUVTM9jfrnzY72YsPszLWkg-UaJcycTR4xXcS%2BfPzS1Vg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Mon, Feb 18, 2013 at 01:44:35PM +0100, Svatopluk Kraus wrote:
> Hi,
>
> the access to sysmaps_pcpu[] should be atomic with respect to
> thread migration. Otherwise, a sysmaps for one CPU can be stolen by
> another CPU and the purpose of per CPU sysmaps is broken. A patch is
> enclosed.
And, what are the problem caused by the 'otherwise' ?
I do not see any.
Really, taking the mutex while bind to a CPU could be deadlock-prone
under some situations.
This was discussed at least one more time. Might be, a comment saying that
there is no issue should be added.
>
> Svata
>
> Index: sys/i386/i386/pmap.c
> ===================================================================
> --- sys/i386/i386/pmap.c (revision 246831)
> +++ sys/i386/i386/pmap.c (working copy)
> @@ -4146,11 +4146,11 @@
> {
> struct sysmaps *sysmaps;
>
> + sched_pin();
> sysmaps = &sysmaps_pcpu[PCPU_GET(cpuid)];
> mtx_lock(&sysmaps->lock);
> if (*sysmaps->CMAP2)
> panic("pmap_zero_page: CMAP2 busy");
> - sched_pin();
> *sysmaps->CMAP2 = PG_V | PG_RW | VM_PAGE_TO_PHYS(m) | PG_A | PG_M |
> pmap_cache_bits(m->md.pat_mode, 0);
> invlcaddr(sysmaps->CADDR2);
> @@ -4171,11 +4171,11 @@
> {
> struct sysmaps *sysmaps;
>
> + sched_pin();
> sysmaps = &sysmaps_pcpu[PCPU_GET(cpuid)];
> mtx_lock(&sysmaps->lock);
> if (*sysmaps->CMAP2)
> panic("pmap_zero_page_area: CMAP2 busy");
> - sched_pin();
> *sysmaps->CMAP2 = PG_V | PG_RW | VM_PAGE_TO_PHYS(m) | PG_A | PG_M |
> pmap_cache_bits(m->md.pat_mode, 0);
> invlcaddr(sysmaps->CADDR2);
> @@ -4220,13 +4220,13 @@
> {
> struct sysmaps *sysmaps;
>
> + sched_pin();
> sysmaps = &sysmaps_pcpu[PCPU_GET(cpuid)];
> mtx_lock(&sysmaps->lock);
> if (*sysmaps->CMAP1)
> panic("pmap_copy_page: CMAP1 busy");
> if (*sysmaps->CMAP2)
> panic("pmap_copy_page: CMAP2 busy");
> - sched_pin();
> invlpg((u_int)sysmaps->CADDR1);
> invlpg((u_int)sysmaps->CADDR2);
> *sysmaps->CMAP1 = PG_V | VM_PAGE_TO_PHYS(src) | PG_A |
> @@ -5072,11 +5072,11 @@
> vm_offset_t sva, eva;
>
> if ((cpu_feature & CPUID_CLFSH) != 0) {
> + sched_pin();
> sysmaps = &sysmaps_pcpu[PCPU_GET(cpuid)];
> mtx_lock(&sysmaps->lock);
> if (*sysmaps->CMAP2)
> panic("pmap_flush_page: CMAP2 busy");
> - sched_pin();
> *sysmaps->CMAP2 = PG_V | PG_RW | VM_PAGE_TO_PHYS(m) |
> PG_A | PG_M | pmap_cache_bits(m->md.pat_mode, 0);
> invlcaddr(sysmaps->CADDR2);
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iQIcBAEBAgAGBQJRIkPZAAoJEJDCuSvBvK1B5JcP/RIJxvumbc5eMO4xE8z19rT/
jDjkcKCRUQMKkc0D2NrtI3La1jyj6Hgen7hL5HE4rZBZPm57pNXbF/L4rf7hCQtt
yg0AJddHI+ZpBIhoDGpUTZA7P476hFckwcDe++3+eOe104JFVs4QbhnfjI3Duc2K
fa+epN4gWeeKXI7GSm64T9XNfsSJXzhRq+fgvN6U5dTTY45oiz/LXqO7/Jz7yZVR
gaGoIgSBWxaproCm0hvXCI5dQ0BHfimgvHQgtUGI9t9JyTVPVymjKMdcDUk55ksc
e1v977j8nECWjlR0Jhq9WPeQaSVy/M0J12BJtdoRZL5Z4IEst8vd/a8s8XA98dV/
77LDD6E6KJXhCrMG0TphEm1c2Kh1ZWn3YyjCFEUVpjJ4K7IUfK7J3sRKvCeilTzk
bm2mQnkCvEMw7vsri/AZApBnNkCsYBDMj/B4PezfDIeSqEN0547MxpITLp43QwiU
viLriUs9hPOrIdOQ7jgKaSL7Gr+G+/zHW4hBdOwhHi7TWcpov0NZbdJiAQTLeFJr
v9NLglj2czAqAcspBf28UNm03y8M0ClItCC3MOTR936RLzgntKXcliFowcWKnW0b
YoiODZECGTTA9DFB0GQtb28nrTQC3cXcQSVxAfz1iKD0ThqA5/r3CNV48+diOa+t
I8JhVjY4CGJCexr46TCX
=xOUl
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130218150809.GG2598>