linux-kernel - Re: [PATCH] libnvdimm: remove redundant __func__ in dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 6 Mar 2018 17:58:42 -0700
From:   Ross Zwisler <ross.zwisler@...ux.intel.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Ross Zwisler <ross.zwisler@...ux.intel.com>,
        linux-nvdimm <linux-nvdimm@...ts.01.org>,
        Johannes Thumshirn <jthumshirn@...e.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] libnvdimm: remove redundant __func__ in dev_dbg

On Tue, Mar 06, 2018 at 08:59:11AM -0800, Dan Williams wrote:
> On Mon, Mar 5, 2018 at 7:54 PM, Ross Zwisler
> <ross.zwisler@...ux.intel.com> wrote:
> > On Mon, Mar 05, 2018 at 05:09:21PM -0800, Dan Williams wrote:
> >> Dynamic debug can be instructed to add the function name to the debug
> >> output using the +f switch, so there is no need for the libnvdimm
> >> modules to do it again. If a user decides to add the +f switch for
> >> libnvdimm's dynamic debug this results in double prints of the function
> >> name.
> >>
> >> Reported-by: Johannes Thumshirn <jthumshirn@...e.de>
> >> Reported-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> >> ---
> >>  drivers/nvdimm/badrange.c       |    3 +-
> >>  drivers/nvdimm/btt_devs.c       |   21 ++++++++--------
> >>  drivers/nvdimm/bus.c            |   13 +++++-----
> >>  drivers/nvdimm/claim.c          |    2 +-
> >>  drivers/nvdimm/core.c           |    4 ++-
> >>  drivers/nvdimm/dax_devs.c       |    5 ++--
> >>  drivers/nvdimm/dimm_devs.c      |    7 ++---
> >>  drivers/nvdimm/label.c          |   51 ++++++++++++++++++---------------------
> >>  drivers/nvdimm/namespace_devs.c |   38 ++++++++++++-----------------
> >>  drivers/nvdimm/pfn_devs.c       |   25 +++++++++----------
> >>  drivers/nvdimm/pmem.c           |    2 +-
> >>  11 files changed, 77 insertions(+), 94 deletions(-)
> >>
> >> diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
> >> index e068d72b4357..df17f1cd696d 100644
> >> --- a/drivers/nvdimm/badrange.c
> >> +++ b/drivers/nvdimm/badrange.c
> >> @@ -176,8 +176,7 @@ static void set_badblock(struct badblocks *bb, sector_t s, int num)
> >>                       (u64) s * 512, (u64) num * 512);
> >>       /* this isn't an error as the hardware will still throw an exception */
> >>       if (badblocks_set(bb, s, num, 1))
> >> -             dev_info_once(bb->dev, "%s: failed for sector %llx\n",
> >> -                             __func__, (u64) s);
> >> +             dev_info_once(bb->dev, "failed for sector %llx\n", (u64) s);
> >
> > I don't think you should remove this one.  dev_info_once() is just a printk(),
> > and doesn't inherit the +f flag from the dynamic debugging code. The __func__
> > here does add value.
> >
> > The rest of these look correct, though I think you missed one in each of
> > nvdimm_map_release()
> 
> This one is now fixed.
> 
> > and validate_dimm().  (I made these changes as well, but
> > you sent out your patch first. :)
> 
> The validate_dimm() one is printing the callee function one level up
> in the call chain, so it's fine as is.

Sounds good.  you can add:

Reviewed-by: Ross Zwisler <ross.zwisler@...ux.intel.com>