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 X-Spam-Level: X-Spam-Status: No, score=-5.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2FADC433E0 for ; Fri, 22 Jan 2021 19:29:16 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7EA2E23AF8 for ; Fri, 22 Jan 2021 19:29:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7EA2E23AF8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=/dhPTCK5uZqeDYutaE2DwxO3ic+sqSMy20lU0Dgj6rY=; b=U5yE3PntB1syZ7SUHU3DUc42m rSuqLX1cYiTqW5XbF/XuZ6w5LIfWLCoGr28JPfLpXWB80TtZHiOT5WXouS3a2fnRocAqEUPv89hon cixNn9L2GRlFJVAygi0UQ5fAX5N+GTIdMqgL37uUtX5aiRRsWyyHGopuN00PhmsFz8xgiDHhZ+Xjy VWEf+vpkHw4K1eYSXcgGW7kTtyymrwZRzTKXOjGhBEnh7tANK+wV+8sKgGbriLJhJ/S6lRCF2+YSJ wGBT8AiAj2dDvTEDFLiZKQ4XOO+/t8ud956WoXz6crfIpso0HmQDunQoxJnZYCMXOd+naqccBX75h 1oPCVD9sQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l3261-0001hV-59; Fri, 22 Jan 2021 19:27:49 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l325y-0001gh-1h for linux-arm-kernel@lists.infradead.org; Fri, 22 Jan 2021 19:27:46 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 96EFD23AF8; Fri, 22 Jan 2021 19:27:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1611343664; bh=kFNnYuWFfaff4kSnuH47UjTnPiizLDg/mNMwPFIhGTM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bV4hOK6u/cF7GPBpwKGoWBpojgOj+ogtuxSRA5pHaf2cgRN8w8vKOvjTt3L6cvqo6 8yIQjT+cygqZIHbw2S7Zr/c02xO7YanBfhzz9Ke0+QUIsqK/bjRPnon4LudQSN+cT4 rFjzZhYWoRev2TlzvCrJSYHePRzVas5IAfn9/2+aekQ47wVFPYB0VIRI1u+utHH+xZ +Z8cFKYh77oij/MZ4v0JRvoRcDfbISDOEmjX2HIZcN5TlLyKAF3N6uwhMaWBO185UZ J7dkOsthQmkya7oh7RUxilvhFhQ/t4jqeE3RtwO82CuJ7NrAq4OPVUM+N0NgcfW8YS CUImWuXg9S/TA== Date: Fri, 22 Jan 2021 19:27:39 +0000 From: Will Deacon To: Nick Desaulniers Subject: Re: [PATCH v4 8/8] mm: Mark anonymous struct field of 'struct vm_fault' as 'const' Message-ID: <20210122192739.GC25471@willie-the-truck> References: <20210120173612.20913-1-will@kernel.org> <20210120173612.20913-9-will@kernel.org> <20210121131101.GD22123@willie-the-truck> <20210121212832.GA23234@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210122_142746_209416_B3B8322D X-CRM114-Status: GOOD ( 25.93 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel-team , Jan Kara , "Kirill A . Shutemov" , Minchan Kim , Catalin Marinas , Hugh Dickins , LKML , Linux Memory Management List , Vinayak Menon , Luc Van Oostenryck , Andrew Morton , Linus Torvalds , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hey Nick, On Fri, Jan 22, 2021 at 11:10:50AM -0800, Nick Desaulniers wrote: > On Thu, Jan 21, 2021 at 1:28 PM Will Deacon wrote: > > On Thu, Jan 21, 2021 at 11:24:36AM -0800, Nick Desaulniers wrote: > > > On Thu, Jan 21, 2021 at 5:11 AM Will Deacon wrote: > > Sure, but here we are cleaning up this stuff, so I think review only gets > > you so far. To me: > > > > const struct { > > int foo; > > long bar; > > }; > > > > clearly says "don't modify fields of this struct", whereas: > > > > struct { > > const int foo; > > const long bar; > > }; > > > > says "don't modify foo or bar, but add whatever you like on the end" and > > that's the slippery slope. > > "but you could add additional non-const members on the end" for sure. > Though going back to > > >> What's to stop a new non-const field from getting added outside the > > > const qualified anonymous struct? > > my point with that is that the const anonymous struct is within a > non-const anonymous struct. > > struct vm_fault { > const { > struct vm_area_struct *vma; > gfp_t gfp_mask; > pgoff_t pgoff; > unsigned long address; > // Your point is about new member additions here, IIUC > }; > // My point: what's to stop someone from adding a new non-const member here? > unsigned int flags; > pmd_t *pmd; > pud_t *pud; > ... > // or here? > }; > > The const anonymous struct will help prevent additions of non-const > members to the anonymous struct, sure; but if someone really wanted a > new non-const member in a `struct vm_fault` instance they're just > going to go around the const anonymous struct. Or is there something > more I'm missing about the order of the members of struct vm_fault? All I was trying to say is that, given a struct with a mixture of const and non-const members, I would be less hesitant to remove 'const' from one of the members if it helped me get something else done. Having the 'const' on the struct itself, however, would deter me because at that point its clear that you're not supposed to be modifying this stuff. That's the difference between the "Your point" and "My point" lines above. But honestly, I can't say I particularly enjoy arguing about these idiosyncracies; I'd just rather wait for the dust to settle with clang before we add code to deal with that outcome. > > So then we end up with the eye-sore of: > > > > const struct { > > const int foo; > > const long bar; > > }; > > > > and maybe that's the right answer, but I'm just saying we should wait for > > clang to make up its mind first. It's not like this is a functional problem, > > and there are enough GCC users around that we're not exactly in a hurry. > > Yeah, I mean I'm happy to whip something up for Clang, even though I > suspect it will get shot down in code review until there's more > guidance from standards bodies. It doesn't hurt to try, and at least > have a patch "waiting in the wings" should we hear back from WG14 that > favors GCC's behavior. Who knows, maybe the guidance will be that > WG21 should revisit this behavior for C++ to avoid divergence with C > (as g++ and gcc currently do). I mean, a warning doesn't seem controversial to me, especially as opinions certainly seem to be divided about what the right behaviour should be here. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel