Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Apr 2023 14:07:59 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc:     "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [PATCH 3/3] mm/mmap: Regression fix for unmapped_area{_topdown}
Message-ID: <20230414180759.zves3glmrptl6zog@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>
References: <20230414145728.4067069-1-Liam.Howlett@oracle.com>
 <20230414145728.4067069-3-Liam.Howlett@oracle.com>
 <3bf69efaeddae3599a6d7ea8a225f64066811473.camel@intel.com>
 <20230414172610.f35pngvz6cilxsdq@revolver>
 <20230414172919.ynxybbocc4o4tbtd@revolver>
 <3f30fe59dd6b368db82ae91b61152bb133c4831f.camel@intel.com>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <3f30fe59dd6b368db82ae91b61152bb133c4831f.camel@intel.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?6B+vfXhMTfaZnR/efax/pQCYj2AzMVP8CB1SoWu7o8pbrBv08lWXpaHezS?=
 =?iso-8859-1?Q?oXhfOXFb/D5sdQRy9T+eFQYQFPinqJp+Oa4W1G35Adby8zkoTyLbQXMIWa?=
 =?iso-8859-1?Q?ANtB+IxCzv10t+jVv7NjUpH9+Ows0ARdU2+b0Y5FYO2cTNjpnZHYyX9vPg?=
 =?iso-8859-1?Q?1PPuBZkQuwR8/89TMYaEUbJFootkeYQRVj6XdOxYFSMva69ijcucJa6hNA?=
 =?iso-8859-1?Q?mtkjcvzAkzqKJ/QGAfo9p4fFiGzcG8HQUbne6evqA30uweH+DPgLvRfGh6?=
 =?iso-8859-1?Q?qn22NDlkcUj/rIpaFxv1eD1R9qikcO6+8cJQ3NjMckUua2LfQAVw8eKGZM?=
 =?iso-8859-1?Q?7ca5VXjbomwpmZepkAAQ+XXTazjF/1VFL7VxK56vbcwQzzMQiBQkRp7/Lp?=
 =?iso-8859-1?Q?c4udAIykHGio5YZtRWRkr1MP/nlgeB4pxtlfSfrQ9gyyclKffgN90SiEJB?=
 =?iso-8859-1?Q?uosMvuakyBuI1jbtgNtyYuyHBKABqJxURw/C7YGl3c0uzkzVvLXwUZwqKW?=
 =?iso-8859-1?Q?oApffjfN+g71TqaaZEkYrWbtBMRWM43Fx4sO58zSuPUbUjXCVeG702kgu0?=
 =?iso-8859-1?Q?ipz9BuIbhNyuIQ0b5dxdquAnLNRrRxB/oZHl0sD02CsP1Adr+Iw1sFB8tF?=
 =?iso-8859-1?Q?Me9W9hXlR8AbLnqGgF7nNedv+WIX0ery2CkjatnTgNp/+vZtix1HNLqElo?=
 =?iso-8859-1?Q?aYH+4wh7RdVaB9Ydpa+U1Ds71GqbwdSyWrLn9PlVUSkG3VAXBCA3DGdN8z?=
 =?iso-8859-1?Q?k5mDsqdTSEn7ADFxWpwAvM4t7KoULxwdJC9EQwns0ccg+aVk122YNlfbUN?=
 =?iso-8859-1?Q?YPlCI9ud4ytalNMIalXKy+U7trxix5yXDwRzLm1BaQsQrHWYHpWKPOZrTX?=
 =?iso-8859-1?Q?wd2+OHd6uj6DGkhSXveV+xuhRTLeUZPo0uCD8eLikuUQ7yTIJtDJ9DW67S?=
 =?iso-8859-1?Q?cNME7Ef2Kc4uhMRnQ2XOvGUXocdcv2/bEzQlnkEOoLZ4nUdpZEzEk3LLbN?=
 =?iso-8859-1?Q?woGJdmStBs8KTrB3vFE9s7s+2Pr2L0b8/Eyomjx8hZTE/5wTI7z2PzRoC6?=
 =?iso-8859-1?Q?3eeMVF8yVPxZ6wAIbHVXaWEr60VqqrDRjFs/9dlsTGE6U8P8kNwTOhajNM?=
 =?iso-8859-1?Q?Xg9d/scOXr1/4WFl6oMoVjHW211KrdSVJtJxbqLN6l5utNX+N1MOwzMdxr?=
 =?iso-8859-1?Q?FXxBFvNPrntvTpGfDbVvIySF0gZXcw4SZMNiar2BgDz4iP1hYQxHz5ZlsW?=
 =?iso-8859-1?Q?smr/NtFQXk87F7gtfz6VbLrasYQVIqSsTXkEcRFpmD901U44J2+j6QDX5E?=
 =?iso-8859-1?Q?UFwPQDyw2bY76EzGqlfV76ruAsuW0o64+Xhl6XeyGzA4aGAG01P1AimD1P?=
 =?iso-8859-1?Q?0CE5D+sU61F2/YeLsDOraVWb9WjFJdUTtuThtCkwSa6zZ8iPc4fhSX0v8s?=
 =?iso-8859-1?Q?eqll3VRTfN5414tolDEgYYAVgNU81fgwdP/AOMHzOCCEFiTVSJfjhCOpfX?=
 =?iso-8859-1?Q?J0vsXMvHr+pOsWH9hhcxcpFBUHdv15cpf6LrQq6JC2/cKsiQ+faYI6+TFA?=
 =?iso-8859-1?Q?RP01ciZizn9EQNhBIsC2xpL4TyIFaB335/3OgKl8jFcV7gIZ9zxAdJdRRW?=
 =?iso-8859-1?Q?gS2sDVHs0jwlJQPbkDmx14yY4SEtH5iO9EFjsT5Z6Vqkf7jTznZ5TDlA?=
 =?iso-8859-1?Q?=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: KCE2wGIPTaz6qj03/jZ1oYnIlkoQvA2968e7YWFSvc9UzZtzhwV7VaOx58ZfX4zMeydiLAzdbcs01gj9XL5cXU+vqD16AyLYHQfhQUb42uQKlnw/x/2RsKLzAdu2giJpePvowHBWm1zBlYrTwxtYFc+iWX924jcmXs5Z+d3traYLA43ZTztxZX4m833+7TI0w0F4znuECTKK3CyYoQCXwErDtFK6nu+f1XOW35yxou/NVK4QWA7UbgqKFyV9r6ZYo3h857wruzGeucr1GmGpsOS0Bjcxj2pTwZ8W8TyzXZwoQaHdjnyFAPe6cs9/Kkd68e/7zP2frx1SD8Ior3C1+cXR8YMykAcqvvhcVwVFtvm0xPYwExdclZC14MhMrGUIAgt100GL/XxHvKcsZb+m/AB5t8nCE2pKwyoUx0gdCVM84tDR9pRuGfLv57Me466EyYIIu4zU5Ltr66YPPEAYYac/fC4gCHe0AXnY5CPki5CP4swE0LSa89DqU/8VBHssxLxDtmwxn82cGfS4QzBg1dU8+wE3SYc5lTEEBb0uwdYbtBFfmkIFb0ggurmKWcsQdl49XVMiQFBX1k6ZQ+Xr9rL7Lrm7VQjcgGEg4a5KpK3dPA80ylyqaTbMXBFgnpofJB1t2nyjQpb3xreY54gdh/GBPC0dkAXDWZQtUfHC11m5twrQrZK9NKRP90Dl6VHxoHFtdPcFCFmHocCfwnGYloWg4f6RXS+NaR0i3T3QZbgC5wXDrQCr1sZGnEyDFmiaMngSqOMrsymbLBKyxRdRHtHKUA0Wt860nWKqJUMPboOD75mCKwE9IiEnUL/7QrpOEVYTSSdk1oKiZlCPoCSrbExz/WqbTOALvX4Fji4tf03aU6KsFyorOFXd8nCo8/sx
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ea69e0e-3fd3-4c44-21b7-08db3d133009
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Apr 2023 18:08:02.3600
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PssiE105oaiOXAzJp/PfX9EN3YQAFRRR/AhYYXoYnzoN5kRhhiD8pgfTaWsca7ZNFrPy5ZR8DfMzOlLDVsRkZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5300
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-04-14_10,2023-04-14_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0
 malwarescore=0 suspectscore=0 bulkscore=0 spamscore=0 phishscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304140160
X-Proofpoint-GUID: J1L5pkc6kzH7a6Wx6wzToppgYiFQA5II
X-Proofpoint-ORIG-GUID: J1L5pkc6kzH7a6Wx6wzToppgYiFQA5II
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Edgecombe, Rick P <rick.p.edgecombe@intel.com> [230414 13:53]:
> On Fri, 2023-04-14 at 13:29 -0400, Liam R. Howlett wrote:
> > * Liam R. Howlett <Liam.Howlett@Oracle.com> [230414 13:26]:
> > > * Edgecombe, Rick P <rick.p.edgecombe@intel.com> [230414 12:27]:
> > > > On Fri, 2023-04-14 at 10:57 -0400, Liam R. Howlett wrote:<br>
> > > > > +=A0=A0=A0=A0=A0=A0=A0tmp =3D mas_next(&mas, ULONG_MAX);
> > > > > +=A0=A0=A0=A0=A0=A0=A0if (tmp && (tmp->vm_flags & VM_GROWSDOWN)) =
{
> > > >=20
> > > > Why also check VM_GROWSDOWN here (and VM_GROWSUP below)?
> > > > vm_start/end_gap() already have checks inside.
> > >=20
> > > An artifact of a plan that was later abandoned.
> > >=20
> > > >=20
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (vm_start_gap(tm=
p) < gap + length - 1) {
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0low_limit =3D tmp->vm_end;
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0mas_reset(&mas);
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0goto retry;
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0}
> > > > > +=A0=A0=A0=A0=A0=A0=A0} else {
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0tmp =3D mas_prev(&m=
as, 0);
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (tmp && (tmp->vm=
_flags & VM_GROWSUP) &&
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 vm_end_ga=
p(tmp) > gap) {
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0low_limit =3D vm_end_gap(tmp);=20
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0mas_reset(&mas);
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0goto retry;=20
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0}
> > > > > +=A0=A0=A0=A0=A0=A0=A0}=20
> > > > > +
> > > >=20
> > > > Could it be like this?
> > >=20
> > > Yes, I'll make this change.=A0 Thanks for the suggestion.
> >=20
> >=20
> > Wait, I like how it is.
> >=20
> > In my version, if there is a stack that is VM_GROWSDOWN there, but
> > does
> > not intercept the gap, then I won't check the prev.. in yours, we
> > will
> > never avoid checking prev.
>=20
> Hmm, I see. I guess I'm thinking ahead a bit to adding the shadow stack
> guard gap, but I can always add to these vm_flags checks.
>=20
> But are you sure this optimization is even possible? The old
> vma_compute_gap() had this comment:
> /*
>  * Note: in the rare case of a VM_GROWSDOWN above a VM_GROWSUP, we
>  * allow two stack_guard_gaps between them here, and when choosing
>  * an unmapped area; whereas when expanding we only require one.
>  * That's a little inconsistent, but keeps the code here simpler.
>  */

I didn't think this was possible.  ia64 (orphaned in 96ec72a3425d) did
this.

>=20
> Assuming this is a real scenario, if you have VM_GROWSDOWN above and
> VM_GROWSUP below, don't you need to check the gaps for above and below?
> Again thinking about adding shadow stack guard pages, something like
> that could be a more common scenario. Not that you need to fix my out
> of tree issues, but I would probably need to adjust it to check both
> directions.
>=20
> I guess there is no way to embed this inside maple tree search so we
> don't need to retry? (sorry if this is a dumb question, it's an opaque
> box to me).

Absolutely, and I'm working on this as well, but right now I'm trying
to fix my regression for this and past releases.  Handling this in the
maple tree is more involved and so there's more risk.