From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F1E7C25B4E for ; Fri, 20 Jan 2023 14:58:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1C5716B0078; Fri, 20 Jan 2023 09:58:11 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 176596B007B; Fri, 20 Jan 2023 09:58:11 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 016566B007D; Fri, 20 Jan 2023 09:58:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id E9A006B0078 for ; Fri, 20 Jan 2023 09:58:10 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B3110C07DD for ; Fri, 20 Jan 2023 14:58:10 +0000 (UTC) X-FDA: 80375482740.25.5B1E52B Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2041.outbound.protection.outlook.com [40.107.236.41]) by imf27.hostedemail.com (Postfix) with ESMTP id 0A3A840018 for ; Fri, 20 Jan 2023 14:58:06 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=BLav+z+3; spf=pass (imf27.hostedemail.com: domain of jgg@nvidia.com designates 40.107.236.41 as permitted sender) smtp.mailfrom=jgg@nvidia.com; arc=pass ("microsoft.com:s=arcselector9901:i=1"); dmarc=pass (policy=reject) header.from=nvidia.com ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1674226687; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DShjQL3Ydzx33H0uFTLAeU9F6sIW2GRl2HEaUbCasPg=; b=kX68KrZuNA2mmP4dlru6DVCmZZ+gilvdHBb8QSqZd1IrsAUifqYH0FYiojPzd5UcT4hjWz wWqaHIG48+Dsd6RlzGmVHJxi9kNKR+CLi8dYhPd31yXSr/QBNceCTWUPM3qEI/je6BZ4Du 1VAeSmyG5Wp1TuiggZYbYAUqWiDkpiU= ARC-Authentication-Results: i=2; imf27.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=BLav+z+3; spf=pass (imf27.hostedemail.com: domain of jgg@nvidia.com designates 40.107.236.41 as permitted sender) smtp.mailfrom=jgg@nvidia.com; arc=pass ("microsoft.com:s=arcselector9901:i=1"); dmarc=pass (policy=reject) header.from=nvidia.com ARC-Seal: i=2; s=arc-20220608; d=hostedemail.com; t=1674226687; a=rsa-sha256; cv=pass; b=Csm/F7tlHNnAK2FX+mGh1OShhxlSZQvD3VMxXRZHv6N+jvlF+BCuvWKa4h8EIbFvUgzHwz sGKeSHu6f7yVcVnHOudob/C5BmEMDsX1UxYUZiBwpt+z/hsdjIdbT1JWuoXZn5joZU19DL YmQcqYQpLDlOTDPhQNjxWtYQcD9tzas= ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kDOa09oGWVDz+Qlb3dzDW1xKaHj1qUhIWEDwb1+9+SM7SJEvvCl1ivuue/fAP2cWlxafREgk9Yoq80kmSHNc8NzCsBk33O/juRkIOTGLhKjM6uh9UN9X4gPuLHJBYwAxe03/VXN8g2KFX7Hd0MZBPwRPcjxogdHyqBDkyJcgdGh0SPx9pyUJNuiTan0SNerGO6QHMHTDlk3XY4EV4liRuU87Mp2d9tLIj17CEizgs3i3f3iyPbEVwkREgR5HfVzP6ZVLlE9Q2tK8vm1LrLfX1AoLCJse8akWNSICSVFfB/oSGvvXSisDtUf/yj1tQFeh8oUnyzR0uZDhR22P9yLT3Q== 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=DShjQL3Ydzx33H0uFTLAeU9F6sIW2GRl2HEaUbCasPg=; b=TtjmfDvERMYb/fROh7zykM/OpMx/ZcJVMWFwJMRXmL75SLHt2wUop3vqCTIXnB0bSJPqY24F1UghNb7OLJzoO0kH3QyKHrbiYSPBYGEGqBkNc2tq4nwKXlpqJMkbrVIMUtI8qwu8njrbSggutla57PNBQ99qMp6HMV86naVAFHpUShAOOgjlcxRUlOtdT/YcgOWxjC7PLwWapYVrUBlB0dHvcrtWMbeko4dr6moZ91gGXMq90UTdjOMUAZKrQDBD9/4wG7x6iM+h5rw23XgRK82czy5eSczRlMBkI3TcLCq+y2Ihl+Gdp+l5yk7GG7oRaH/puXcSx/XGweNWX9LvpQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=DShjQL3Ydzx33H0uFTLAeU9F6sIW2GRl2HEaUbCasPg=; b=BLav+z+3soHBhCTyPCCWWIewo+BOeesF+QEtVQCULdsgCCD4RKEUVUZxfOiCGdIPLu9GS1xeN90hzRxiQDPJo9eA21UtAsO/Y8ThyZenIq8mMuRbuLxersUfRnt4kDGTbGns9XrcV3pWfEiPrtvm4MmLNsYe9XXgMNhh6FtwicDyZjgct/+qVtxmGveF9wRsjQAGY3Mfb/z5j1Lw1kp6kT/fcBVDok0NABMg/8gwp4yhNwgIPAb9Fv5WEhdHHSd7/ZWysW0d+6tG87vbxFU75p+E9UCMANABDGizSSJg65zUmqkof1vjjlIGvlb9eExPO9a1VFGustfMd/ULDQd0gA== Received: from LV2PR12MB5869.namprd12.prod.outlook.com (2603:10b6:408:176::16) by CY8PR12MB7291.namprd12.prod.outlook.com (2603:10b6:930:54::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6002.25; Fri, 20 Jan 2023 14:58:04 +0000 Received: from LV2PR12MB5869.namprd12.prod.outlook.com ([fe80::f8b0:df13:5f8d:12a]) by LV2PR12MB5869.namprd12.prod.outlook.com ([fe80::f8b0:df13:5f8d:12a%9]) with mapi id 15.20.6002.013; Fri, 20 Jan 2023 14:58:04 +0000 Date: Fri, 20 Jan 2023 10:58:03 -0400 From: Jason Gunthorpe To: John Hubbard Cc: Alistair Popple , linux-mm@kvack.org Subject: Re: [PATCH 3/8] mm/gup: simplify the external interface functions and consolidate invariants Message-ID: References: <3-v1-dd94f8f0d5ad+716-gup_tidy_jgg@nvidia.com> <845c3d54-efe5-6e3a-05ca-5f419ac7d145@nvidia.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <845c3d54-efe5-6e3a-05ca-5f419ac7d145@nvidia.com> X-ClientProxiedBy: BL1PR13CA0308.namprd13.prod.outlook.com (2603:10b6:208:2c1::13) To LV2PR12MB5869.namprd12.prod.outlook.com (2603:10b6:408:176::16) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: LV2PR12MB5869:EE_|CY8PR12MB7291:EE_ X-MS-Office365-Filtering-Correlation-Id: e9a948fa-5e16-42b7-e9ee-08dafaf6bbca X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ZEg0mJIcDgRzmLANHSagejfw7LT2jsHPka//CEx+HCzpw6RVG7pHmWG/EurmkolP72InUxUo8IaPtpMSjI3omaIF8rkcNHw6Fv4zi3wC5mBc4cvm+hGn9TVIzTc1ml7NZK6OKt2a0AUdDnJOnlQDGqKucK8eSDskftfYB5UdgZI2mjG8EShPXWOwlGrED3Sh61J7nh+XlgirL/vgyVtvfnaxLA+CxS//becsTwlVuHe6PyS742cTsSgrdzDg6XsHIOkoyxUBzh0u1aGJ5LS1fu2tO+Ojw4Ge92GYKn97XBQ3It9GYjQtYDNKLfso7UlutEPB2MxsBlA5B3y+rx5el01ur+MHtzXEq1NEuUhY4faAEHv0MEpxpYhORzpP51GlBdxDGnjArUMT3F2Dos6Poyn9Lnq8FsxdWO94VhVfj72ofqFWHLxPwn4UsUzQ+3e6ctFCS8UTRZcMiFv5FAKXBRE1SbV9W2ZTzB4Bqx6Hi0RqLWIbccd5NhuH8+M/q6f/4wI9YqutiqyXOMrn936RZAHCbXw4iNusqnqdQw9DTezdgnHaj/IBAwYpfY0b9gGBEz2qXArmpWMwzxTJrvqujBx0mGGBoN2k2uUKAcKfFQEX/UebGfLZjx7rofTIhXtYefktHQkdcVrdBJZ9x93kbRfPM6Oec2L/LMzEgXF7NnUpydT0bulpWoeIyVYbSPnj X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:LV2PR12MB5869.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(39860400002)(376002)(366004)(396003)(346002)(136003)(451199015)(38100700002)(36756003)(86362001)(8936002)(41300700001)(5660300002)(478600001)(37006003)(8676002)(66556008)(4326008)(66946007)(66476007)(2906002)(2616005)(83380400001)(53546011)(6506007)(6636002)(6862004)(186003)(6512007)(316002)(26005)(6486002)(67856001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?a9ozXfWYlmmSMcLJgXGeUHSgcWsvmmyQBGtrbcC66x8zyTLMp5DI9OhptASD?= =?us-ascii?Q?daf6y5Z7Wd0sJoDiJFyknVa83gNAMZa2hQ78+pkmW2pt4gjrBHH8tKd6iXnE?= =?us-ascii?Q?CEBQ24ouFDcEbAn4uFHAvJSIp9es3II5Du0C0Au/R6JCvO9Axpzx9D7Ovn6m?= =?us-ascii?Q?0AlXq+3awDTSBmdiEGAEr+2bGOCfUjClAgIRz6fC0iiY9jpXzax8EFwxLLRN?= =?us-ascii?Q?bXfeCIwcd6oDPl+1VBidiYGbM6TQ+LF/3A3IPZkDtxEx8pib6ebqr1VUuOg1?= =?us-ascii?Q?CltRPw/cezTeP0AClBGKmWkWWsD+YGsBmgSzorhgdkATBDSAVMTZFdAzkL6W?= =?us-ascii?Q?CWolhCX/k5gVTC4PkCxoBP0o8SE8DSGdMRaTdktr9qQKHi74eHq44/MFS58a?= =?us-ascii?Q?7ub9E7Vy1pVKTKo46B15zKr8V4t55ROCaMsn541FM5H9S2tP85hUqlNzCzCc?= =?us-ascii?Q?TSk/4OfMfsV/vJ9+z09+y05ay7dFwEboeIHByhgPPht3eOJ77ieiRKPbbd1P?= =?us-ascii?Q?+eU4m9qTTcj7EJ/W/qTD5P0V17VUeKKo5Lih1SNUHAzLIhPNDKAPbmgQFA+l?= =?us-ascii?Q?HRxf9iJDy3NqF1zPVJ+Vi/xvPBqUKa0DU7lIBpWmLpp0F2V8vOUaKO48QVu3?= =?us-ascii?Q?ZFluIVwVAHx1UF/1WyVK/YIcjVjpoPF8PCAYg5Nx1GfvgI4Ogcwj6wVxmbWo?= =?us-ascii?Q?SlETbV+uVNDbsMrLHU5sI4FG+r5Q4lI+J7GtWAAB32wR34D65KFT4bNB7Mh9?= =?us-ascii?Q?PlTB/c9Io4EeZwetuBTP2ClHMFJdNIifU1lsDabjgshEFIAmX1sfvmz3yXku?= =?us-ascii?Q?3XzGvQ5GsO0L9NBXoT/RqpEIRGimati5MzL+G42oSMRAqPf9Q71VyBkNfj1X?= =?us-ascii?Q?kSa540VyA8i+XmDQUsA4V4koc/lblV4XO8V+WvK+L0g5vlSUa6R9pxKTlqeF?= =?us-ascii?Q?2z9/nsjDmGNfKjfiHCB8dLmdZlgQE8J0sfOAdNNfh8itYOuFxhssFF1nDyeZ?= =?us-ascii?Q?MKhvLGvSYz1UGcwExd6pKv8RsGaPLFh5axnFiVyArw6QszMBUFIj3MozYDuQ?= =?us-ascii?Q?q+XRMRfQ+SooL4hU4l6bszjFBDgyt3LOPrzCnb6FS4qxZUg8wufGMrUTrYZF?= =?us-ascii?Q?L6MGAqocsXHl/kx85zXSgtdowxY6v6BFEAILfOW871XStLfhWPvvqsHjWo+X?= =?us-ascii?Q?5FBBHIg6EK57cqzAE9i4ULrkW8uVuTYnsV1osLZliqkGMWOFO145ZMCT1gXC?= =?us-ascii?Q?D0GCPZt3ayRZx/wOZS3Egw6tHaMRQIpNvkiP76TM7CXITYwE1bdZX0mcLIU9?= =?us-ascii?Q?ZMgxlrI+B5t2mSbuFUAJTGR704dPRY7P47YIe5y3rxMyZV0yL2skGvQRHOS7?= =?us-ascii?Q?rmgm6ZwIuavgatNsiuHyJtpq12/U8GAefHwZI4sCDhdGKjXdIHHfU0nalrbF?= =?us-ascii?Q?uP90MiKiMJwde2XceKxgls2qJvEYQpfe88kCIHLbH7O/fw+U3Hpj+Zlg16Nv?= =?us-ascii?Q?TkUNN99WIZqIsW1Q84D7xjuREeuaIycWIZwQudZnfaHwsqAFantxpMb04emt?= =?us-ascii?Q?RkDkYEBbod7nomRnDZ9bpX+2hrnbVQRmEqjDw6wT?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: e9a948fa-5e16-42b7-e9ee-08dafaf6bbca X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jan 2023 14:58:04.7132 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: vJYDxPjYhzqjG859UaeScUmwCeXRm/wdxwXVOC2pIm2oY0EQBWF+r0t4+H4F++NX X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7291 X-Rspamd-Queue-Id: 0A3A840018 X-Stat-Signature: fpgbexr9p7sycjb96p98p9bqk566mnot X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1674226686-369705 X-HE-Meta: U2FsdGVkX19cxl9pxcYso+aSa8wlHHatv2qT14nqdELaQTNrZkX0kyd28dK157OB22Q6Rxf4ifZXDemmOYBcyodsiW3315rMJSIxx/gJ2A9Usc7lghieOC0cHaffjkfyF2n6eCJwxkDIzppt+GMOVajGYIjYDHQvZziV0l8sjqhbKDZIAkMp/E7fSvyk4nfYksnW/QFMi8BMZLRmCD91P9SELbhb5GxCo3uIedu5F0ld5llc89yuWQOfw5tbz7PUa8k2SWfQzzPz86K3iQNMb+wL1DGt+K3jY6wgPfNt7oPl0DTvvrkyIV/ucIluECEgegrAXvYS7YkIJ92HBiftZX1bzVKB6NEO983Zp86G/ZS5wUNantQVLr1AxYBW6F8fd7ecFhSejwTgPzvkFRJVGS+dN4l5Vv1p7OlRsMj4eejebKr3dnLskSqgbNxlCUb8cuUWsCECAcwLtaRBlPG+5SWHY8igiUQwtPRob6MMDRUkI/hu5yZETRCzQBfZ5q6C0Tbn9bcH4QdEjy6QVKMFKEYdtAP1EqiKd2F3sLos6yZIZx02oWyE0eU2WaWaUsn4T5gr/OFq99DxYe7eYK/KcpfRrKNYRMgh9Y7kvNQy5RJ7lHPh18BgnTBbKRixGS59Zb/MLv5qnVmAYS+1AOZ/BVOKiKp/5K6vbkiovwIhlkbk7hxcT2hWFw4+FBluMeD8shTdJAbNTURucD5ASiwDVC0oPcJy3xfQmrYzmu3LnJbTfRBeQb0E6ip6CrVFI/KYdbxNbAtPv0IWQgeRT1BtY7AxfiFghOlEkkNXMflmByWlMRR0XvxzMuxyd6LyrEU+BBTH2Um9wNBFlZZ7ZgOb0Q6I7FPr259OKNtnW9Wa5QhZUi45fiEmyBu+W9BV40o3LZ7mT77RTTQ6WwJxJO5mykiBemhBOli+dJVm1gaUo6TemXF9F5LIzbCz52oyevzlCuGB04/+8xFR7aZ1F6q i5up+tQ2 cbWOY9vNUQZAXAjc0kiY3CNONEJ4YPoJRBL2bVxjHlBqSYJ0qKuUTk0sVaz9CakRy9HuL28KlpTvZ2ehKcoWyTBhKoZf7Id9h1qcZhjlUSmNOQrR72OYtktzBnwU/I2LZhi7hqecP2QuKOdkAKFLSc8eni5LTnM7xySzwaJ3e/nRWVaC5i21C/TFpVcyKORtc+mecuDnX//iPXFeXvRrg9wyajKdkKOMY+juDqJffXeqxnLOMaQF1gjAPIA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Jan 19, 2023 at 06:51:18PM -0800, John Hubbard wrote: > On 1/17/23 07:58, Jason Gunthorpe wrote: > > The GUP family of functions have a complex, but fairly well defined, set > > of invariants for their arguments. Currently these are sprinkled about, > > sometimes in duplicate through many functions. > > > > Internally we don't follow all the invariants that the external interface > > has to follow, so place these checks directly at the exported > > interface. This ensures the internal functions never reach a violated > > invariant. > > > > Remove the duplicated invariant checks. > > > > The end result is to make these functions fully internal: > > __get_user_pages_locked() > > internal_get_user_pages_fast() > > __gup_longterm_locked() > > > > And all the other functions call directly into one of these. > > > > Suggested-by: John Hubbard > > Signed-off-by: Jason Gunthorpe > > --- > > mm/gup.c | 150 +++++++++++++++++++++++------------------------ > > mm/huge_memory.c | 10 ---- > > 2 files changed, 75 insertions(+), 85 deletions(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 2c833f862d0354..9e332e3f6ea8e2 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -215,7 +215,6 @@ int __must_check try_grab_page(struct page *page, unsigned int flags) > > { > > struct folio *folio = page_folio(page); > > - WARN_ON_ONCE((flags & (FOLL_GET | FOLL_PIN)) == (FOLL_GET | FOLL_PIN)); > > try_grab_page() is declared in mm.h and is therefore potentially > something that other subsystems could call--although they really > shouldn't! And here, we are simply assuming that that is enough. But in > order to be really comfortable removing this check on the basis of > "try_grab_page() is internal to mm", I think it would help to move its > declaration from mm.h, to mm/internal.h. Perhaps as a separate > patch. Yes, lets do that > > @@ -818,7 +817,7 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address, > > if (vma_is_secretmem(vma)) > > return NULL; > > - if (foll_flags & FOLL_PIN) > > + if (WARN_ON_ONCE(foll_flags & FOLL_PIN)) > > OK, so we're slightly fortifying follow_page() checking, but > not at the level of is_valid_gup_args(). Should this be mentioned > in the commit description? And should the checks be more extensive? I'd leave it, there is no reason to be too nannyish - follow_page() isn't an exported symbol. > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index abe6cfd92ffa0e..eaf879c835de44 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -1039,11 +1039,6 @@ struct page *follow_devmap_pmd(struct vm_area_struct *vma, unsigned long addr, > > assert_spin_locked(pmd_lockptr(mm, pmd)); > > - /* FOLL_GET and FOLL_PIN are mutually exclusive. */ > > - if (WARN_ON_ONCE((flags & (FOLL_PIN | FOLL_GET)) == > > - (FOLL_PIN | FOLL_GET))) > > - return NULL; > > - > > For both follow_devmap_pmd() and follow_devmap_pud(), below, it looks like > the following external API path is left exposed (with respect to checking > gup flags): > > do_mlock() > __mm_populate() > populate_vma_page_range() This is in gup.c and sets the flags directly, so it is not part of the "external interface" we should just leave it. IMHO, the point of the checks is primarily prevent bad gup_flags from entering gup.c, primarily from creative driver authors, not to prevent bugs in gup.c. Jason