Messages in this thread | | | Date | Mon, 23 Apr 2018 15:03:17 +0200 | From | Greg Kroah-Hartman <> | 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
| |