Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220209170358.3266629-1-atomlin@redhat.com> <20220209170358.3266629-7-atomlin@redhat.com>
 <a05710d0-3b2c-1405-df44-f1315f8fe765@csgroup.eu>
In-Reply-To: <a05710d0-3b2c-1405-df44-f1315f8fe765@csgroup.eu>
From:   Aaron Tomlin <atomlin@redhat.com>
Date:   Fri, 11 Feb 2022 11:09:19 +0000
Message-ID: <CANfR36hnTVX7enuGo46qgo3PRP1LO3rT8kPVjsX+26m8M3v0mA@mail.gmail.com>
Subject: Re: [PATCH v5 06/13] module: Move strict rwx support to a separate file
To:     Christophe Leroy <christophe.leroy@csgroup.eu>
Cc:     "mcgrof@kernel.org" <mcgrof@kernel.org>,
        "cl@linux.com" <cl@linux.com>,
        "pmladek@suse.com" <pmladek@suse.com>,
        "mbenes@suse.cz" <mbenes@suse.cz>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "jeyu@kernel.org" <jeyu@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
        "live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
        "atomlin@atomlin.com" <atomlin@atomlin.com>,
        "ghalat@redhat.com" <ghalat@redhat.com>,
        "allen.lkml@gmail.com" <allen.lkml@gmail.com>,
        "void@manifault.com" <void@manifault.com>,
        "joe@perches.com" <joe@perches.com>,
        "msuchanek@suse.de" <msuchanek@suse.de>,
        "oleksandr@natalenko.name" <oleksandr@natalenko.name>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu 2022-02-10 12:21 +0000, Christophe Leroy wrote:
> This file generates many checkpatch WARNINGs and CHECKs.
> Don't worry too much about the ones telling you to use WARN_ON() instead
> of BUG_ON() for the time being, but others should be handled.

Yes, with ./scripts/checkpatch.pl --strict'.
Please note: I have used '--ignore=ASSIGN_IN_IF,AVOID_BUG' previously on
that file. Albeit, I will resolve the check violations e.g. "Alignment
should match open parenthesis" etc.

> > +# define debug_align(X) ALIGN(X, PAGE_SIZE)
>
> You can use PAGE_ALIGN() instead.

Agreed: PAGE_ALIGN(X) does expand to ALIGN(X, PAGE_SIZE)

> > +#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
>
> This #ifdef is not needed, frob_text() always exists.

I will leave this for you to remove, in your patch [1].

> > +    BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>
> Could be:
>
>     BUG_ON(!PAGE_ALIGNED(layout->base));
>
>
> Same for all others.

Agreed.

[1]: https://lore.kernel.org/lkml/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu/

Kind regards,
-- 
Aaron Tomlin