lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 19 Aug 2009 14:59:29 +0800 From: Shaohua Li <shaohua.li@...el.com> To: "Siddha, Suresh B" <suresh.b.siddha@...el.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, Nick Piggin <npiggin@...e.de>, "mingo@...e.hu" <mingo@...e.hu>, "hpa@...or.com" <hpa@...or.com>, "tglx@...utronix.de" <tglx@...utronix.de>, "Pallipadi, Venkatesh" <venkatesh.pallipadi@...el.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [patch] x86: Rendezvous all the cpu's for MTRR/PAT init On Wed, Aug 19, 2009 at 02:20:57PM +0800, Siddha, Suresh B wrote: > On Tue, 2009-08-18 at 18:01 -0700, Andrew Morton wrote: > > On Tue, 18 Aug 2009 17:30:35 -0700 Suresh Siddha <suresh.b.siddha@...el.com> wrote: > > > + /* > > > + * Ideally we should hold mtrr_mutex here to avoid mtrr entries changed, > > > + * but this routine will be called in cpu boot time, holding the lock > > > + * breaks it. This routine is called in two cases: 1.very earily time > > > + * of software resume, when there absolutely isn't mtrr entry changes; > > > + * 2.cpu hotadd time. We let mtrr_add/del_page hold cpuhotplug lock to > > > + * prevent mtrr entry changes > > > + */ > > > > That's a tantalising little comment. What does "breaks it" mean? How > > can reviewers and later code-readers possibly suggest alternative fixes > > to this breakage if they aren't told what it is? > > This is a cut and paste comment coming from the previous code. Shaohua > added this comment originally and I think this is the case he is trying > to avoid. > > cpu - A modifying/adding a MTRR register > > cpu - B is coming online > > if cpu - A doesn't take the cpu hotplug lock, then potentially what can > happen is that cpu B will update its mtrr's with old state and now A can > change the state and before B comes completely online, A can do send the > MTRR update to all cpu's except B. > > So Shaohua's code is taking cpu hotplug lock before A updates MTRR's so > that B's MTRRs are always is in sync with rest of the cpu's in the > system. Only the mtrr_mutex is not enough. > > Nevertheless as far as this patch is concerned mtrr_aps_init() gets > called during early boot/resume time and as such we never hit this > condition. So I removed this comment in the new patch appended. > > Shaohua if you agree with my explanation we can have a separate patch to > make the original comment more meaningful. Yes, your explanation is correct. Thanks, Shaohua -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists