[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] x86/mm: avoid phys_to_nid() calls for invalid addresses
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Fri, 16 Dec 2022 19:24:54 +0000
- Accept-language: en-GB, en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=2zZDXiGexGiO4rjouJfy7iVvJzriAj1YhvUadpBkcqQ=; b=nptynYfSA+qIu4ZEHC+QSVuVb4xa5ABirj5jr01cuX0J2UKeXhJVp+zl64RmlbktrVxUc/yJu7KSvZ1cvoQo+1KfwdD6DY0cspaBoGqvhNO3VKUSEHaoYmeCOAjgnFR+toBfNvgdIHTeafJRDYPiLX1xPZUMsJ19WXwP0IoMc10u6l0BBYeT+ykD3SZ+gessE/B5Oscicw2c3vO/iQbXQ1LeMs0CL1vV7UwxWaVQssRFmwC3VVsljpLu1lRI0UuTqNJKW30se5/sXnX5Jit6JKEKdoQaUS5r32yPXpXz6dnkOPHZkuADobyaqT6BdqYgQ2TfkzDgnfOqOSP/YCWLqQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VsraaAv2HimShC1n8L/KOcbiKtmwTwkH/5PDw+EYbAtjMaYLHaKv8LfuTi4t7fWCHO8zB4cnoUQfUdRZjFE70/ZXX964rx8HewXdEUsLzz+w98w4W8PTVxt9vjS7p3xeGoushYoXga1B0BvQrePyxG0faCBewjl9UbJPraBkjosfoTNeJfS1OxJumSFtele4CUGroB0wQCtrP6RHmzBhjbS56qjt4JFMrRfDqLtrUIoSaZWR0S1fOz3K6yEq3EOm/UAEiz6S5Pyu1q3kD34mQpai14Xz0YWM7WVqRzAeNZZ9kPloWwxxCc2PbHkTQhbKfWc3c3y/VeGakXhGBfxCnA==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: George Dunlap <George.Dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
- Delivery-date: Fri, 16 Dec 2022 19:25:07 +0000
- Ironport-data: A9a23:2zTxLauN8CgRzBTFc50ZARox3OfnVHRfMUV32f8akzHdYApBsoF/q tZmKWiOa/3eMGrxfth/Oo+y904D75/VnYJiQFNqqCs9ESob+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg0HVU/IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj5lv0gnRkPaoR5QWHySFOZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwdgg1Ug28g+yK3vG2ZeZdv5k9fPu2FdZK0p1g5Wmx4fcOZ7nmGvyPzvgBmTA6i4ZJAOrUY NcfZXx3dhPcbhZTO1ARTpUjgOOvgXq5eDpdwL6XjfNvvy6Pk0osjv60b4K9lt+iHK25mm6xo G7c8nu/KRYdLNGFkhKO8262h/+JliT+MG4XPO3orac73QTMroAVIFpNUgWDs8vjsBSVAc0AO n4/xSMep6dnoSRHSfG4BXVUukWsvBQRRt5RGO0S8xyWx+zf5APxLngJSHtNZcIrsOcyRCc2z RmZktXxHzttvbaJD3WH+d+8sjeaKSUTa2gYakc5oRAt5tDipMQ4iUvJR9M6Saqt1ISqQXf33 iyAqzU4i/MLl8kX2q6n/FfBxTWxupzOSQ1z7QLSNo640j5EiEeeT9TAwTDmATxodu51knHpU KA4pvWj
- Ironport-hdrordr: A9a23:nLjs9a3tupbswqCM5loN5gqjBRV3eYIsimQD101hICG9Lfb45q Xe4sjzhCWb+VRhJ03Ix+ruScrwPU80raQFq7X4Pd+ZLX3bURiTXcxfBOrZsmDd8kjFh59gPM hbAuBD4bHLfCpHZKXBkVWF+rQbsZK6GcmT7I+0owYIPGYaEtAZnnwJcHfnYz0GNzWqHaBJcq Z1UKd8zQZJgxwsDviTPXUeU+/f4/nGjojvbxJDJxNP0mSzZFiTmcnH+m2jr14jukR0sMEfGA b+4nnEz5TmntSD9wTd2WHe9P1t6encI+94dZyxYqh8EES+tu/kXvVNZ1VM1ApF+N1Grz0R4f DxiiZlG/42x2Laf2mzrxeo8w780Aw243un7VODm3PsreHwWTp/LdFAi4Jfeh6csiMbzYtB+Z MO+1jcm4tcDBvGkii4z9/UVytynk7xhXY5i+Ycg1FWTINbTqRQo4wZ9EYQOpYdGyDR7pwhDY BVfZrhzccTVWnfQ2HSv2FpztDpdnMvHi2eSkxHgcCR2yg+pgE/86O1rPZvtksoxdYYcd1p9u 7EOqNnmPVlVckNd5tnCOMAW8esTkTLXBLXKWqXZW7sHKYsPXXRp4/riY9F2d2CSdgt9t8fiZ 7BWFRXuSoZYET1E/SU0JlK6BzWBE2gQDXE0KhllqNEk4y5YICuHTyISVgoncflie4YGNfjQP q2OIhbGbvKMXbuI4BUxAfzMqMiEUU2YYkwgJIWSliOqsXEJsnBrerAas/JKL7sCzo/HkviH3 0tWiPsLN5M4k3DYA7DvDHhH1fWPmDv95N5F6bXu8IJzpIWD5ZBtggOhU78x8GQNDtYosUNDX VDCYKitpn+iXi9/G7O4WksEAFaFFxt+7nlU2lHv0stKEP7cbEKvv+beWxUwVu/DhJzVM/NCm dk1nNK0JPyC6bV6TEpCtqhPG7fpWAUvmiyVJsZmreO/4PdYZUzAow9VKE0PhWONBpoggFjrW dFZmY/N3DiPwKrrZ/goI0fBenZedU5qhysO9Rssn7atV+Rv4UUbF5zZU+TbeenxSIVAxZEjF x49KESxJCanyy0EHAyhOQjPEcJVX+eB6heCh+ZWZ5dlb/qcjttRlqbnDDysWALRkPas2Epwk DxJyydfv/GRnBHvGpD673n9FNven/YQll5bWpit5ZhKH/PtXly29Kaf6bb6Rq2VnIyhsUmdB 3VazobJQ1jg/qt0gSOpTqEHXI6gr0zI+30Ft0YApPu80LoDLfNubANHvdS8pogHsvpqPU3Xe WWfBLQBC/kCtkuxxeeqh8eSQpJQUEf4NbVMSDenUiFNT8EcNvvyW1dNp8my5f21Rmwex7OuK 8Joe7c8ICLQzfMg5C9uP6nSwKq7Hvo0DqLpqoT2NxpVJkJxfhO90Oya0qf6Jh25mRoEC69rj JQfE1a2sHrBmY9Rb05R8qulmBZzehm2yMQw0rL60UFDGAQZjngTvy0youNkKEoBEKZogv2JB 2wzw1xls21bxer5PoiEKQ3Ln1RaE8grFJY3M3HWbHxJWyRBtJ+FHzTCA7tAYtgoWy+aOtgmy qSTOv42tO/Zm79wkTdrDF7KqVB/yKsSca/Bw+FAqpT/8e7NU7Jgqyx/ca1hjuySTbTUTVYuW VvHXZgIPirSgNSyLEfw2y3UOj6s0glm1xR7XV6jFL33oCn5WzcDAVcNxHFgpJZVTdeNWXNlN /C++SDznjxiQI1m6UrPH0gPu1zJw==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHZDuc9jXw99mNebki+EqZyGC5j4q5w6lcA
- Thread-topic: [PATCH 1/2] x86/mm: avoid phys_to_nid() calls for invalid addresses
On 13/12/2022 11:36 am, Jan Beulich wrote:
> With phys_to_nid() now actively checking that a valid node ID is on
> record, the two uses in paging_init() can actually trigger at least the
> 2nd of the assertions there. They're used to calculate allocation flags,
> but the calculated flags wouldn't be used when dealing with an invalid
> (unpopulated) address range. Defer the calculations such that they can
> be done with a validated MFN in hands. This also does away with the
> artificial calculations of an address to pass to phys_to_nid().
>
> Note that while the variable is provably written before use, at least
> some compiler versions can't actually verify that. Hence the variable
> also needs to gain a (dead) initializer.
I'm not surprised in the slightest that GCC can't prove that it is
always initialised. I suspect a lot of humans would struggle too.
> Fixes: e9c72d524fbd ("xen/x86: Use ASSERT instead of VIRTUAL_BUG_ON for
> phys_to_nid")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
This does appear to fix things. (Testing hasn't finished yet, but all
systems have installed, and they didn't get that far previously).
> ---
> RFC: With small enough a NUMA hash shift it would still be possible to
> hit an SRAT hole, despite mfn_valid() passing. Hence, like was the
> original plan, it may still be necessary to relax the checking in
> phys_to_nid() (or its designated replacements). At which point the
> value of this change here would shrink to merely reducing the
> chance of unintentionally doing NUMA_NO_NODE allocations.
Why does the NUMA shift matter? Can't this occur for badly constructed
SRAT tables too?
Nevertheless, this is a clear improvement over what's currently in tree,
so I'm going to commit it to try and unblock OSSTest. The tree has been
blocked for too long. Further adjustments can come in due course.
~Andrew
|
