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=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 B7BC0C433DB for ; Fri, 22 Jan 2021 19:11:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5293923AC1 for ; Fri, 22 Jan 2021 19:11:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5293923AC1 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B13BF6B0005; Fri, 22 Jan 2021 14:11:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AA4F46B0007; Fri, 22 Jan 2021 14:11:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 98A726B0008; Fri, 22 Jan 2021 14:11:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0142.hostedemail.com [216.40.44.142]) by kanga.kvack.org (Postfix) with ESMTP id 7FE386B0005 for ; Fri, 22 Jan 2021 14:11:03 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 3AD31181AF5F4 for ; Fri, 22 Jan 2021 19:11:03 +0000 (UTC) X-FDA: 77734353606.17.bit12_010ecee2756e Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin17.hostedemail.com (Postfix) with ESMTP id 1EF3A181355F8 for ; Fri, 22 Jan 2021 19:11:03 +0000 (UTC) X-HE-Tag: bit12_010ecee2756e X-Filterd-Recvd-Size: 7557 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Fri, 22 Jan 2021 19:11:02 +0000 (UTC) Received: by mail-pj1-f50.google.com with SMTP id g15so4519084pjd.2 for ; Fri, 22 Jan 2021 11:11:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=O2ZB3udNCNHbkRemCu0DDwTLSpUFiqLQJxKRr0v+cOo=; b=ses9w6o7aiBd7blGwonVHgFbhSzEl6eTcbM/L+9Qtyi6O/vbP2jN12jBcn5McioGmO rwbdjfEhfPbSJRRPNUHCJ4P2cc85qzX0CJ370UxGejFEJQ22OMfOuTbvazPGhLkqnvnq +5o9Tadz2rAADWu/Fsu0veqlNRKXgXe8mF/yPY5uS177uSmgh9I9Ry3zUZ3uaH7vHCu1 UCr69z53p8Vq1/Vej4su1g7pVBYeFMBa5wE68fwZdsEF/D5avzzf1wdGE1xNIpQxZXfH grmjueZLtivFR9L+oamWPaCcphjm84vm/vdCJlJLuLFLrkR22yEk/s8Mme3wots37Yti spDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=O2ZB3udNCNHbkRemCu0DDwTLSpUFiqLQJxKRr0v+cOo=; b=EwqzcGl0Iahbe3GDXHXXT2/uBjxkS9zwx/gJopu2/aRZ2iIC74ScasXxR2TvM4+GLs y1DyUpBiqrbCdry81s7o0znmx+rgWlfkm0m4C0qgmwY5ZsYp6gVzeaBanGN4SvLAPta1 bRe+IuxTnoip34uCYghEScKh78Ljkb7KoTpsCN4IgngE7BvqxbT9MnjQtp4cwPkvLrft JrHnSplXcz/cTEHnRdh57vLIYMlqsVfq3YFT7AKay0EXtpw6AlfmY7i53heTQIIlMo1q eXk49sKSo5EH4iRDTCeP15O3dkRnJP8jGGK0u4AnPawAiFZHJ6arQrISfXWzRzGhiVNx 4cdg== X-Gm-Message-State: AOAM533k7MejQp4l9S8cz/WqFca//fKfVhpxbkqojj4SftSmPGZHLLxO jEOB0AD9IKQF4OgC/g2LvJdUEDEe5GZNukBHogbubw== X-Google-Smtp-Source: ABdhPJzamJj7j9mtm+zwf/YTjuFIE6wBidb5TwLHbw7zfJYraZgGdvFI1QBxklpX5PAgO681Pq9f1DYgF9PZUe93Yak= X-Received: by 2002:a17:90a:31c3:: with SMTP id j3mr6947403pjf.25.1611342661472; Fri, 22 Jan 2021 11:11:01 -0800 (PST) MIME-Version: 1.0 References: <20210120173612.20913-1-will@kernel.org> <20210120173612.20913-9-will@kernel.org> <20210121131101.GD22123@willie-the-truck> <20210121212832.GA23234@willie-the-truck> In-Reply-To: <20210121212832.GA23234@willie-the-truck> From: Nick Desaulniers Date: Fri, 22 Jan 2021 11:10:50 -0800 Message-ID: Subject: Re: [PATCH v4 8/8] mm: Mark anonymous struct field of 'struct vm_fault' as 'const' To: Will Deacon Cc: Linus Torvalds , Luc Van Oostenryck , LKML , Linux Memory Management List , Linux ARM , Catalin Marinas , Jan Kara , Minchan Kim , Andrew Morton , "Kirill A . Shutemov" , Vinayak Menon , Hugh Dickins , kernel-team Content-Type: text/plain; charset="UTF-8" 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 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: > > > > > > On Wed, Jan 20, 2021 at 11:02:06AM -0800, Linus Torvalds wrote: > > > > On Wed, Jan 20, 2021 at 10:27 AM Nick Desaulniers > > > > wrote: > > > > > > > > > > Is there a difference between: [ const unnamed struct and individual const members ] > > > > > > > > Semantically? No. > > > > > > > > Syntactically the "group the const members together" is a lot cleaner, > > > > imho. Not just from a "just a single const" standpoint, but from a > > > > "code as documentation" standpoint. > > > > > > > > But I guess to avoid the clang issue, we could do the "mark individual > > > > fields" thing. > > > > > > I'd prefer to wait until the bug against LLVM has been resolved before we > > > try to work around anything. Although I couldn't find any other examples > > > like this in the kernel, requiring all of the member fields to be marked as > > > 'const' still feels pretty fragile to me; it's only a matter of time before > > > new non-const fields get added, at which point the temptation for developers > > > to remove 'const' from other fields when it gets in the way is pretty high. > > > > What's to stop a new non-const field from getting added outside the > > const qualified anonymous struct? > > What's to stop someone from removing const from the anonymous struct? > > What's to stop a number of callers from manipulating the structure > > temporarily before restoring it when returning by casting away the > > const? > > > > Code review. > > 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? > 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). -- Thanks, ~Nick Desaulniers