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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E3CFC433EF for ; Tue, 10 May 2022 11:14:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240786AbiEJLR4 (ORCPT ); Tue, 10 May 2022 07:17:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51440 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240778AbiEJLRn (ORCPT ); Tue, 10 May 2022 07:17:43 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id EC8782A83F1 for ; Tue, 10 May 2022 04:13:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652181226; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=D5QpDLvjaL/3sbIJjxhXvjRruga3nx8tizHPxU55AHk=; b=ixQltLsdZZmVu/paZGHocU3fq8Is5QE3+LMx8kuG5136bWfyOfK09jU4MGbs8LbXYOxBWB w9Zseyogv6OoAUvsfmC+0UUQu0xFit0fuykPaPwTe/RpErsacj6OGbSiElUJwf/LiqPI25 J5r7pFe4SqbdDNBqQ8mmH78oiE/zw0I= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-610-cMhZcyPwMlS2jAsJZrmERA-1; Tue, 10 May 2022 07:13:44 -0400 X-MC-Unique: cMhZcyPwMlS2jAsJZrmERA-1 Received: by mail-wm1-f69.google.com with SMTP id z23-20020a05600c221700b003942fd37764so1117847wml.8 for ; Tue, 10 May 2022 04:13:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=D5QpDLvjaL/3sbIJjxhXvjRruga3nx8tizHPxU55AHk=; b=3utz9gl86pzZg5/vLUqcJjp74gpdyyou4VcvjiTWMqXWwFvccI/kb0Iy4tzxcIFyVX 8Jx5RsuXr35aFb2nYS1wWiPsHsQ67A+zD8I5RWimMflZI3QGU0fnFo1aoSwFYLp0bxQ7 Ef/ItheZy80dG/MFhS7zFQQxz24rbM1CY9j3UC/3tqXNoTDj6YZoloNSIM1eHCkcuS17 Wsd8xQC3y5W+WZtmZiRh5UoSovkuaJUU/PR1OLiUM5vXkpUm5UX2KNeqBuALKnUJiv9n KwNntmKeWOpFvlgO61TIi2nyxYfTrL5AVakuZkO4e+BnmtK8XqwQX2N0HFc0n9lAWYWa nAQQ== X-Gm-Message-State: AOAM530bd2PW0aiTsWlb7/V9gswpWCYoqgJdHrHGM4wYyTUiVirJTtxw VncecFgM02++RRx/9+0sYnQwUYgQ+wQWuZMVbu9GMcMqXIy+/CxGjq5M63+9aDXjoby4mfVJoIV BsJ9jQ2CMeBPq/Z0twxdXTHLdUVPEvVT1xdQAEMuZ X-Received: by 2002:a05:600c:501e:b0:394:8b0a:3ec9 with SMTP id n30-20020a05600c501e00b003948b0a3ec9mr11945149wmr.0.1652181223538; Tue, 10 May 2022 04:13:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx0gjbA7+3TrCagIYxdCI9NkRd7LcHVggdp1NZufK6Cnw8DV0cCzDb63Ne9hqLyNNa1u57MLcInZtEPF2ZahDw= X-Received: by 2002:a05:600c:501e:b0:394:8b0a:3ec9 with SMTP id n30-20020a05600c501e00b003948b0a3ec9mr11945136wmr.0.1652181223377; Tue, 10 May 2022 04:13:43 -0700 (PDT) MIME-Version: 1.0 References: <20220508100630.2642320-1-keescook@chromium.org> In-Reply-To: From: Andreas Gruenbacher Date: Tue, 10 May 2022 13:13:31 +0200 Message-ID: Subject: Re: [PATCH] gfs2: Use container_of() for gfs2_glock(aspace) To: Kees Cook , Christoph Hellwig Cc: Bob Peterson , kernel test robot , Bill Wendling , cluster-devel , Nathan Chancellor , Nick Desaulniers , Tom Rix , Steven Whitehouse , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Kees and Christoph, thanks for the patch and review. On Tue, May 10, 2022 at 9:29 AM Christoph Hellwig wrote: > > +/* gfs2_glock_get(), "glock" must be first. */ > > +struct glock_aspace { > > + struct gfs2_glock glock; > > + struct address_space mapping; > > +}; > > Why does glock need to be first? The whole point of using container_of > is that we don't have to make that assumption. Just needs to be cleaned up in gfs2_glock_get() and gfs2_glock_dealloc() as well. I'll do that when applying the patch. > > static inline struct address_space *gfs2_glock2aspace(struct gfs2_glock *gl) > > { > > if (gl->gl_ops->go_flags & GLOF_ASPACE) > > - return (struct address_space *)(gl + 1); > > + return &(container_of(gl, struct glock_aspace, glock)->mapping); > > Do we even need the braces here? Will use a local variable here, as suggested below. > > struct inode *inode = mapping->host; > > if (mapping->a_ops == &gfs2_meta_aops) > > - return (((struct gfs2_glock *)mapping) - 1)->gl_name.ln_sbd; > > + return container_of(mapping, struct glock_aspace, mapping)->glock.gl_name.ln_sbd; > > A local variable would be really nice for the reader here to decompose > this a bit: > > struct glock_aspace *a = > container_of(mapping, struct glock_aspace, mapping); > > return a->glock.gl_name.ln_sbd; Yes. Thanks, Andreas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Date: Tue, 10 May 2022 13:13:31 +0200 Subject: [Cluster-devel] [PATCH] gfs2: Use container_of() for gfs2_glock(aspace) In-Reply-To: References: <20220508100630.2642320-1-keescook@chromium.org> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Kees and Christoph, thanks for the patch and review. On Tue, May 10, 2022 at 9:29 AM Christoph Hellwig wrote: > > +/* gfs2_glock_get(), "glock" must be first. */ > > +struct glock_aspace { > > + struct gfs2_glock glock; > > + struct address_space mapping; > > +}; > > Why does glock need to be first? The whole point of using container_of > is that we don't have to make that assumption. Just needs to be cleaned up in gfs2_glock_get() and gfs2_glock_dealloc() as well. I'll do that when applying the patch. > > static inline struct address_space *gfs2_glock2aspace(struct gfs2_glock *gl) > > { > > if (gl->gl_ops->go_flags & GLOF_ASPACE) > > - return (struct address_space *)(gl + 1); > > + return &(container_of(gl, struct glock_aspace, glock)->mapping); > > Do we even need the braces here? Will use a local variable here, as suggested below. > > struct inode *inode = mapping->host; > > if (mapping->a_ops == &gfs2_meta_aops) > > - return (((struct gfs2_glock *)mapping) - 1)->gl_name.ln_sbd; > > + return container_of(mapping, struct glock_aspace, mapping)->glock.gl_name.ln_sbd; > > A local variable would be really nice for the reader here to decompose > this a bit: > > struct glock_aspace *a = > container_of(mapping, struct glock_aspace, mapping); > > return a->glock.gl_name.ln_sbd; Yes. Thanks, Andreas