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:   Thu, 10 Feb 2022 14:18:23 +0530
Message-ID: <436367503.2691559.1644482903367@mail-kr5-0>
Mime-Version: 1.0
Subject: RE: [PATCH 1/1] kallsyms: print module name in %ps/S case when
 KALLSYMS is disabled
Reply-To: maninder1.s@samsung.com
Sender: Maninder Singh <maninder1.s@samsung.com>
From:   Maninder Singh <maninder1.s@samsung.com>
To:     Petr Mladek <pmladek@suse.com>,
        Luis Chamberlain <mcgrof@kernel.org>
CC:     Vimal Agrawal <avimalin@gmail.com>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "senozhatsky@chromium.org" <senozhatsky@chromium.org>,
        "andriy.shevchenko@linux.intel.com" 
        <andriy.shevchenko@linux.intel.com>,
        "linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "wangkefeng.wang@huawei.com" <wangkefeng.wang@huawei.com>,
        "mbenes@suse.cz" <mbenes@suse.cz>,
        "swboyd@chromium.org" <swboyd@chromium.org>,
        "ojeda@kernel.org" <ojeda@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "will@kernel.org" <will@kernel.org>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        Vaneet Narang <v.narang@samsung.com>,
        Aaron Tomlin <atomlin@redhat.com>
In-Reply-To: <YgTKqoRIwahzWyh0@alley>
Content-Type: multipart/mixed;
        boundary="----=_Part_2691558_779447602.1644482903367"
CMS-TYPE: 105P
References: <YgTKqoRIwahzWyh0@alley>
        <20220201040044.1528568-1-maninder1.s@samsung.com>
        <20220209114038.GA8279@pathway.suse.cz>
        <YgRH7hwFC2AGISdP@bombadil.infradead.org>
        <CGME20220201040100epcas5p180ef094058fc9c76b4b94d9d673fc5fc@epcms5p4>
Precedence: bulk

------=_Part_2691558_779447602.1644482903367
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"

Hi All,

Thanks for your inputs.

> On Wed 2022-02-09 15:02:06, Luis Chamberlain wrote:
> > On Wed, Feb 09, 2022 at 12:40:38PM +0100, Petr Mladek wrote:
> > > > --- a/include/linux/kallsyms.h
> > > > +++ b/include/linux/kallsyms.h
> > > > @@ -163,6 +163,33 @@ static inline bool kallsyms_show_value(const struct cred *cred)
> > > >          return false;
> > > >  }
> > > >  
> > > > +#ifdef CONFIG_MODULES
> > > > +static inline int fill_minimal_module_info(char *sym, int size, unsigned long value)
> > > > +{
> > > > +        struct module *mod;
> > > > +        unsigned long offset;
> > > > +        int ret = 0;
> > > > +
> > > > +        preempt_disable();
> > > > +        mod = __module_address(value);
> > > > +        if (mod) {
> > > > +                offset = value - (unsigned long)mod->core_layout.base;
> > > > +                snprintf(sym, size - 1, "0x%lx+0x%lx [%s]",
> > > > +                                (unsigned long)mod->core_layout.base, offset, mod->name);
> > > > +
> > > > +                sym[size - 1] = '\0';
> > > > +                ret = 1;
> > > > +        }
> > > > +
> > > > +        preempt_enable();
> > > > +        return ret;
> > > > +}
> > > 
> > > It looks too big for an inlined function. Anyway, we will need
> > > something even more complex, see below.
> > 
> > Interesting, these observations might apply to Vimal's work as well [0].
> > 
> > [0] https://lkml.kernel.org/r/YgKyC4ZRud0JW1PF@bombadil.infradead.org
>  
> Honestly, I am not sure what is the best practice. My understanding is
> that inlined functions are used primary for speed up at runtime.
>  

Main reason of making it inline was:
(1) kallsysm.c was not getting compiled(with disabled config), so could not add defination there.
(2) lib/vsnprintf.c was not correct place to define new function of kallsyms(fill_minimal_module_info)
(3) I thought static int will be part of each .c file which includes kallsyms.h and compiler can make noise for unused functions,
and also increase code size, where as static inline will be added only if some code is calling that function otherwise will be discarded.

But as peter said better version will be to make a new defination of __sprint_symbol (probably in kernel/module.c)
to handle all cases of %ps/S/B/b when KALLSYSMS is disabled.

I will try to prepare changes and share V2 patch.

Thanks,
Maninder Singh
------=_Part_2691558_779447602.1644482903367
Content-Type: application/octet-stream
Content-Disposition: attachment; filename="rcptInfo.txt"
Content-Transfer-Encoding: base64

DQogICA9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT0NCiAgICAgIFN1YmplY3QgICAgOiBSZTogW1BBVENIIDEvMV0g
a2FsbHN5bXM6IHByaW50IG1vZHVsZSBuYW1lIGluICVwcy9TIGNhc2Ugd2hlbiBLQUxMU1lNUyBp
cyBkaXNhYmxlZA0KICAgICAgRnJvbSAgICAgICA6IG51bGwNCiAgICAgIFNlbnQgRGF0ZSAgOiAy
MDIyLTAyLTEwIDEzOjUwICBHTVQrNTozMA0KICAgPT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQogICAgICAgICAg
ICAgICAgICBOYW1lICAgICAgICAgICAgICAgIFR5cGUgICAgICAgICAgSm9iIFRpdGxlICAgICAg
ICAgICAgICAgICAgICAgICBEZXB0LiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBDb21w
YW55ICAgICAgICAgICAgICAgIA0KICAgPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQogICAgICBtY2dyb2ZAa2Vy
bmVsLm9yZyAgICAgICAgICAgICAgVE8NCiAgICAgIGF2aW1hbGluQGdtYWlsLmNvbSAgICAgICAg
ICAgICBDQw0KICAgICAgTWFuaW5kZXIgU2luZ2ggICAgICAgICAgICAgICAgIENDICAgICAgICAg
U3RhZmYgRW5naW5lZXIgICAgICAgICAgICAgU3lzdGVtIFMvVyBHcm91cCAvU1JJLURlbGhpICAg
ICAgICAgICAgICAgU2Ftc3VuZ8KgRWxlY3Ryb25pY3PCoA0KICAgICAgcm9zdGVkdEBnb29kbWlz
Lm9yZyAgICAgICAgICAgIENDDQogICAgICBzZW5vemhhdHNreUBjaHJvbWl1bS5vcmcgICAgICAg
Q0MNCiAgICAgIGFuZHJpeS5zaGV2Y2hlbmtvQGxpbnV4LmkuLi4gICBDQw0KICAgICAgbGludXhA
cmFzbXVzdmlsbGVtb2VzLmRrICAgICAgIENDDQogICAgICBha3BtQGxpbnV4LWZvdW5kYXRpb24u
b3JnICAgICAgQ0MNCiAgICAgIHdhbmdrZWZlbmcud2FuZ0BodWF3ZWkuY29tICAgICBDQw0KICAg
ICAgbWJlbmVzQHN1c2UuY3ogICAgICAgICAgICAgICAgIENDDQogICAgICBzd2JveWRAY2hyb21p
dW0ub3JnICAgICAgICAgICAgQ0MNCiAgICAgIG9qZWRhQGtlcm5lbC5vcmcgICAgICAgICAgICAg
ICBDQw0KICAgICAgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZyAgIENDDQogICAgICB3aWxs
QGtlcm5lbC5vcmcgICAgICAgICAgICAgICAgQ0MNCiAgICAgIGNhdGFsaW4ubWFyaW5hc0Bhcm0u
Y29tICAgICAgICBDQw0KICAgICAgVmFuZWV0IE5hcmFuZyAgICAgICAgICAgICAgICAgIENDICAg
ICAgICAgQXNzb2NpYXRlIEFyY2hpdGVjdCAgICAgICAgU3lzdGVtIFMvVyBHcm91cCAvU1JJLURl
bGhpICAgICAgICAgICAgICAgU2Ftc3VuZyBFbGVjdHJvbmljcw0KICAgICAgYXRvbWxpbkByZWRo
YXQuY29tICAgICAgICAgICAgIENDDQogICA9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCg==

------=_Part_2691558_779447602.1644482903367--