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;
Date:   Wed, 31 May 2023 10:22:05 -0700
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Maninder Singh <maninder1.s@samsung.com>
Cc:     ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
        andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
        yhs@fb.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
        jolsa@kernel.org, thunder.leizhen@huawei.com, boqun.feng@gmail.com,
        vincenzopalazzodev@gmail.com, ojeda@kernel.org, jgross@suse.com,
        brauner@kernel.org, michael.christie@oracle.com,
        samitolvanen@google.com, glider@google.com, peterz@infradead.org,
        keescook@chromium.org, stephen.s.brennan@oracle.com,
        alan.maguire@oracle.com, pmladek@suse.com,
        linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
        Onkarnath <onkarnath.1@samsung.com>
Subject: Re: [PATCH v2 1/2] bpf: make defination of bpf_dump_raw_ok based on
 CONFIG_KALLSYMS
Message-ID: <ZHeCPYcq23gp1hRa@bombadil.infradead.org>
References: <CGME20230531084800epcas5p1c02d5a7ffcef2bb7cf626bf52beb0eee@epcas5p1.samsung.com>
 <20230531084745.877337-1-maninder1.s@samsung.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230531084745.877337-1-maninder1.s@samsung.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Wed, May 31, 2023 at 02:17:44PM +0530, Maninder Singh wrote:
> No functional change with this commit.
> 
> As of now bpf_dump_raw_ok() is dependent on kallsyms_show_value().
> Rearranging the code to return false directly in defination of
> bpf_dump_raw_ok() based on CONFIG_KALLSYMS.
> 
> @reason: next patch will make kallsyms_show_value() defination
> generic and bpf_dump_raw_ok() will return true otherwise.

This reads a bit funny and the split here doesn't help the patch
reviewier, so you can help with that a bit with the commit log
and be a bit clearer to throw in the few pieces of nuggets the
reader should be aware of when reviewing your patch right away.
So how about instaed:

bpf_dump_raw_ok() depends on kallsyms_show_value() and we already
have a false definition for the !CONFIG_KALLSYMS case. But we'll
soon expand on kallsyms_show_value() and so to make the code
easier to follow just provide a direct !CONFIG_KALLSYMS definition
for bpf_dump_raw_ok() as well.

This makes it easier to read and makes it clear bpf_dump_raw_ok()
will immediately return false when !CONFIG_KALLSYMS, making it
easier to understand what happens for heavy users such as bfp.

With this you can add:

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis