linux-kernel - Re: [PATCH 1/6] staging: lustre: move stack-check macros to libcfs

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:   Mon, 23 Apr 2018 15:03:17 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     NeilBrown <neilb@...e.com>
Cc:     James Simmons <jsimmons@...radead.org>,
        Oleg Drokin <oleg.drokin@...el.com>,
        Andreas Dilger <andreas.dilger@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Lustre Development List <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH 1/6] staging: lustre: move stack-check macros to
 libcfs_debug.h

On Wed, Apr 18, 2018 at 12:17:37PM +1000, NeilBrown wrote:
> On Mon, Apr 16 2018, James Simmons wrote:
> 
> >> CDEBUG_STACK() and CHECK_STACK() are macros to help with
> >> debugging, so move them from
> >>    drivers/staging/lustre/include/linux/libcfs/linux/libcfs.h
> >> to
> >>    drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h
> >> 
> >> This seems a more fitting location, and is a step towards
> >> removing linux/libcfs.h and simplifying the include file structure.
> >
> > Nak. Currently the lustre client always enables debugging but that
> > shouldn't be the case. What we do need is the able to turn off the 
> > crazy debugging stuff. In the development branch of lustre it is
> > done with CDEBUG_ENABLED. We need something like that in Kconfig
> > much like we have CONFIG_LUSTRE_DEBUG_EXPENSIVE_CHECK. Since we like
> > to be able to turn that off this should be moved to just after
> > LIBCFS_DEBUG_MSG_DATA_DECL. Then from CHECK_STACK down to CWARN()
> > it can be build out. When CDEBUG_ENABLED is disabled CDEBUG_LIMIT
> > would be empty.
> 
> So why, exactly, is this an argument to justify a NAK?
> Are you just saying  that the code I moved into libcfs_debug.h should be
> moved to somewhere a bit later in the file?
> That can easily be done when it is needed.  It isn't needed now so why
> insist on it?
> 
> Each patch should do one thing and make clear forward progress.  This
> patch gets rid of an unnecessary file and brings related code together.
> I think that qualifies.

I agree, this just deletes an unused file, it changes no functionality
at all.  Now applied.

greg k-h