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 585E5C77B7D for ; Sat, 13 May 2023 13:14:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81D846B0071; Sat, 13 May 2023 09:14:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CD836B0072; Sat, 13 May 2023 09:14:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6BC196B0074; Sat, 13 May 2023 09:14:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 5DB046B0071 for ; Sat, 13 May 2023 09:14:25 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 224CBA10E1 for ; Sat, 13 May 2023 13:14:25 +0000 (UTC) X-FDA: 80785275690.13.E90DD2F Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf20.hostedemail.com (Postfix) with ESMTP id 415EE1C0014 for ; Sat, 13 May 2023 13:14:22 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=fru4WBJF; spf=pass (imf20.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1683983663; 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=KScg4ydCaUpShKuw7MCg69AXYFIohUjFbKBvL+ntZ4c=; b=62jirT09radDi8+f1XOl6SlEjC1LPvBlaaF4txcHxRsh5xcjt1Fpq1Y4HxMuPNiDf6t3zt eonS5r2XY5n1aepKo4aNjB/x+5Z84ZpobRMWw5b6z6lrzRiiYEJotASDNRdZUBR8JrQ2r/ aC3C1NZAvHy3IEIfdPxnya01XORN9ww= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1683983663; a=rsa-sha256; cv=none; b=P7VyjxVUohAGqE2AY0n1WlpHwffqtWqGl3scHQvIYxKCV3SrdeIdqyn37qOKXhZW5c7hg5 GpUaRXfCbx8UJ76M6CmyiHk3bgJfSUlYeKMThaZ05PDp4h+JYfrLC8EiURg8aZefEywufx VaN6zI1JXpHiZtbtHSheB8QrQf/lrho= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=fru4WBJF; spf=pass (imf20.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-3f475366514so30990395e9.2 for ; Sat, 13 May 2023 06:14:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683983661; x=1686575661; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=KScg4ydCaUpShKuw7MCg69AXYFIohUjFbKBvL+ntZ4c=; b=fru4WBJFozuJiW7li8qmMR6SqWTExWH9KYH9ot9EMkzBgywJGHGPAYTXhcVjfJ0wn/ 8iDLYVxJf0YME4AHr9WfrySF9zZyuNYVOKJczQiqZ/KnzzJTBvWUKzOzmzY3pbRhcyM2 5BJpoIeWehf1zOH8M26AjCMq1HpJe1cVuhmHvYXxDaNue4GIg4/RKjV53xMPd7P5BVKb 6RisXKsCRPnd98WlzEq1U6/FAmzagUxVvF8JKsOPX5ky++/5Psgoeotx4gLU7xCD0SCf beAhH7mHuFmr3asVygACGMJz9yIrp4g+H4btpTXqNh7mHr5ns2F/FcoaMyaWUhO6pFm+ yt5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683983661; x=1686575661; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=KScg4ydCaUpShKuw7MCg69AXYFIohUjFbKBvL+ntZ4c=; b=WXePMfht1Tf85TwRcAqYo21F9RXzrol6vfpofPhVQv+q6oycBHCvDNnV/yuvvf/hPW Ujnxrsc1WvtamhGHEDixQM/KSizMiNfEky8h1VL1u/YxZw06fTuV92J7YNxMMr4gWL0K BV+R+u6MRY21Tbaz/k37D3XuojQMGEXtxPUIbamfxezcqEkDyatjTwBg/K31UIZvwx5J KW6HQYYFQfqhpP9+PCY9cnoJuCK62zy2nug9zRn4DNZ5Dkt0IqTpACBFnmnyGDhk1Wf2 leRy3YK3v5gL3RfQSk/kb5/VwIY/WdmNXbSPzNjysB0yMCawESLWYkm0I/MLFN01teAH AC2Q== X-Gm-Message-State: AC+VfDwAT3b99yzAnS6R63zYxP+HEzc29WnMiaffLBEyxBX5VDFGzOel kg0U3saz6kNSgxUh7BeTKb0= X-Google-Smtp-Source: ACHHUZ6GzE4EtDYDWvxFIutm/k122S95nAwJ4sg8r+G2b7K9rajmoRg+0Exa9qsZwr3ZRfT2MnPrCQ== X-Received: by 2002:a7b:c4c3:0:b0:3f1:75ae:1cfe with SMTP id g3-20020a7bc4c3000000b003f175ae1cfemr19040929wmk.7.1683983661254; Sat, 13 May 2023 06:14:21 -0700 (PDT) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id f8-20020a7bcd08000000b003f42894ebe2sm15878056wmj.23.2023.05.13.06.14.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 13 May 2023 06:14:19 -0700 (PDT) Date: Sat, 13 May 2023 14:14:18 +0100 From: Lorenzo Stoakes To: Ryan Roberts Cc: Andrew Morton , "Matthew Wilcox (Oracle)" , "Kirill A. Shutemov" , SeongJae Park , linux-kernel@vger.kernel.org, linux-mm@kvack.org, damon@lists.linux.dev, Christoph Hellwig , Uladzislau Rezki Subject: Re: [RESEND PATCH v1 1/5] mm: vmalloc must set pte via arch code Message-ID: References: <20230511132113.80196-1-ryan.roberts@arm.com> <20230511132113.80196-2-ryan.roberts@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230511132113.80196-2-ryan.roberts@arm.com> X-Stat-Signature: xyskqhoi8rpnq51x8okcdubtrg6xybrc X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 415EE1C0014 X-HE-Tag: 1683983662-738011 X-HE-Meta: U2FsdGVkX19HQZwgp+EQRE5SaNcqL3TDHwESO0Jh6H8bJT18WJk+K+BnTYwAcvYc/naI1ufGlKKrCzcWOipZFwN5nRjhID1BBULUzdqwNiFfMTLcbQNQ7NQYQKWYnSLbye87ssM+8mJOmmbzbYp60FyJVNq/Ik9PHR6sNqBF/nrIhOTMlgcSAjJ2LwC8cGEZeKgLy2ecyBSyJwsD2/6IcJL8bDNdhrxJ0k/hW/N4VDceKG/s0GCcmhEWyTKCgxjBaNzZ/1BEyOc+lm1EP5BBNdc6s3iHNLN7ukd59EHuSyZf0R6meSx1GHza95D/KWrO3cMXaBWYLqj4oVInF6rdIKtxxzZmoZ3yByYNTtKG6ArYBeL6yrZa12MXz8tJpYb2ScHFjBgIYjGmHOOHfmp6sS1nHRHNE/LdLvSmtCSCt10anaXnJa7T86euDEsp9So9dHdzVk2XI8cP+UyRBbljnT2fTWSHZOYGcz+qhvkGDbNAduZFJXMsGa9k3Ev6jqyaukcwdX2E5fLgL9BCF4gjkcVb0XcrwbgagPafzYcWMaKuD/sq9tBWWr+VeR2xZgaS37HuGGlDnEX0004bZEYR73uQ3FG2rIv+GH3OzshgHGF/TvJrxS5euk0BZ/fqzvk/w3rMEMWfmPuoCvvPrj6gSipzSLq8uLOAMvR8XpwyD2JaX0m+BplDhCsgOpIErLuvuU3xbAoHzIR9LwmI1DmRgV94mHSOI5nqU5zkyOevkfsZAqGbd6MON9wV0DfV6WfKpOdKnHaJJjeG6MNdbqKd2CaAbO8Zd5OoywA6nGBt763WBkCCG70+HYZZMg+K8H/4fD0Sg6kRdlNRebW/lOPseTSuDy3O6G5pMKpwcU5Ybvw5dAlQwRWnbecIisVFTkX8UBCc4g5y//iIitDQKSp2fJKQOqpTE6If2b7cLhHM6YNTwWga5uOeM6BnerkU0HSoDbUh2Ym+E5bgsjUQQAW InFbFoa1 4hwlqZ+L7qo5KhvWHbHnBYedCf1XEC8dLGXV6J0qAmxpFwXV2s2gDhaVR9TsM/lnCe4g+I6R+G1JTDo7O11Y/bwK3O4VNzK3hQQYyvhywEOwNx53Taav8ic0A+fVwzoX15EsQjnc64V/kGx18UwN0xID4d2Ebf3fFHTiP9nWV8ClqsIrJ40a+iYatSyLkyPdJcjjS1lfgetzr0//4WPi5Bngjs2+X4Fv33j+V8lySXFCMMBhT7toeKt9e6bD3uti0Vop6VmFwANma7e7a12G7lGd1jBj3EWH69zubNKyCDIOjn9WpG+Eki3YoJIVTEMM2lH8Dxzn74UOyk3FCK4ZEJWxCl9xGiel85ehQToZmsZ59uChi/pe3aIOx7C77ihsDO6YTisAvXksqmF1qHKPL1/F/VBCRL4yRlXa0W8IMhOYKuXVT6waEKvaPQbNrqxSM7RdZ7jiYMP+W6cc= 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: You've not cc'd the vmalloc reviewers, including the author of 3e9a9e256b1e whose patch you purport to fix. Please remember to run get_maintainers.pl on all files you patch and cc them at least on relevant patches. Have added Christoph + Uladzislau as cc. You'll definitely want an ack from Christoph on this! On Thu, May 11, 2023 at 02:21:09PM +0100, Ryan Roberts wrote: > It is bad practice to directly set pte entries within a pte table. > Instead all modifications must go through arch-provided helpers such as > set_pte_at() to give the arch code visibility and allow it to validate > (and potentially modify) the operation. This does make sense, and I see for example in xtensa that an arch-specific instruction is issued under certain circumstances so I do suspect we should do this. As for validation, the function never indicates an error, so only in the sense that a WARN_ON() could _in theory_ trigger is it being validated. This might be quite a nitty point :) as set_pte_at() has no means of indicating an error. But maybe to be pedantic 'check' rather than 'validate'? > > Fixes: 3e9a9e256b1e ("mm: add a vmap_pfn function") Not sure if this is really 'fixing' anything, I mean ostensibly, but not sure if the tag is relevant here, that is more so for a bug being introduced, and unless an issue has arisen not sure if it's appropriate. But this might be a nit, again! > Signed-off-by: Ryan Roberts > --- > mm/vmalloc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 9683573f1225..d8d2fe797c55 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2899,10 +2899,13 @@ struct vmap_pfn_data { > static int vmap_pfn_apply(pte_t *pte, unsigned long addr, void *private) > { > struct vmap_pfn_data *data = private; > + pte_t ptent; > > if (WARN_ON_ONCE(pfn_valid(data->pfns[data->idx]))) > return -EINVAL; > - *pte = pte_mkspecial(pfn_pte(data->pfns[data->idx++], data->prot)); > + > + ptent = pte_mkspecial(pfn_pte(data->pfns[data->idx++], data->prot)); > + set_pte_at(&init_mm, addr, pte, ptent); While we're refactoring, it'd be nice to stash data->pfns[data->idx] into a local pfn variable. > return 0; > } > > -- > 2.25.1 >