From d1532991081982b92ef07a450bdc5bfa68b13e79 Mon Sep 17 00:00:00 2001 From: James Simmons Date: Sun, 28 Jun 2020 08:14:01 -0400 Subject: [PATCH] LU-6142 libcfs: resolve debug.c checkpatch issues Cleanup up libcfs debug.c to resolve various checkpatch issues. This also brings us into alignment with the Linux Lustre client. Change-Id: I011a11ce7f0d5189186f5f9d8e32954ec0ce1ff7 Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/39118 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Arshad Hussain Reviewed-by: Neil Brown Reviewed-by: Oleg Drokin --- libcfs/include/libcfs/libcfs_debug.h | 21 +++++----- libcfs/include/libcfs/libcfs_private.h | 75 ++++++++++++---------------------- libcfs/libcfs/debug.c | 70 +++++++++++++++---------------- libcfs/libcfs/tracefile.h | 2 - lustre/obdclass/cl_page.c | 4 +- 5 files changed, 71 insertions(+), 101 deletions(-) diff --git a/libcfs/include/libcfs/libcfs_debug.h b/libcfs/include/libcfs/libcfs_debug.h index 9c33011..f3a328c 100644 --- a/libcfs/include/libcfs/libcfs_debug.h +++ b/libcfs/include/libcfs/libcfs_debug.h @@ -38,7 +38,6 @@ #ifndef __LIBCFS_DEBUG_H__ #define __LIBCFS_DEBUG_H__ -#include #include #include @@ -49,16 +48,19 @@ extern unsigned int libcfs_subsystem_debug; extern unsigned int libcfs_stack; extern unsigned int libcfs_debug; extern unsigned int libcfs_printk; -extern unsigned int libcfs_console_ratelimit; extern unsigned int libcfs_watchdog_ratelimit; +extern unsigned int libcfs_console_ratelimit; extern unsigned int libcfs_console_max_delay; extern unsigned int libcfs_console_min_delay; extern unsigned int libcfs_console_backoff; extern unsigned int libcfs_debug_binary; extern char libcfs_debug_file_path_arr[PATH_MAX]; +struct task_struct; + int libcfs_debug_mask2str(char *str, int size, int mask, int is_subsys); int libcfs_debug_str2mask(int *mask, const char *str, int is_subsys); +void libcfs_debug_dumpstack(struct task_struct *tsk); /* Has there been an LBUG? */ extern unsigned int libcfs_catastrophe; @@ -284,16 +286,15 @@ do { \ return; \ } while (0) -extern int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata, - const char *format1, ...) - __attribute__ ((format (printf, 2, 3))); +int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata, + const char *format1, ...) + __printf(2, 3); /* other external symbols that tracefile provides: */ -extern int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob, - const char __user *usr_buffer, - int usr_buffer_nob); -extern int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob, - const char *knl_buffer, char *append); +int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob, + const char __user *usr_buffer, int usr_buffer_nob); +int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob, + const char *knl_buffer, char *append); #define LIBCFS_DEBUG_FILE_PATH_DEFAULT "/tmp/lustre-log" diff --git a/libcfs/include/libcfs/libcfs_private.h b/libcfs/include/libcfs/libcfs_private.h index 28db507..daae959 100644 --- a/libcfs/include/libcfs/libcfs_private.h +++ b/libcfs/include/libcfs/libcfs_private.h @@ -112,9 +112,7 @@ do { \ # define LINVRNT(exp) ((void)sizeof!!(exp)) #endif -#define KLASSERT(e) LASSERT(e) - -void lbug_with_loc(struct libcfs_debug_msg_data *) __attribute__((noreturn)); +void __noreturn lbug_with_loc(struct libcfs_debug_msg_data *msg); #define LBUG() \ do { \ @@ -235,9 +233,6 @@ do { \ /******************************************************************************/ -struct task_struct; - -void libcfs_debug_dumpstack(struct task_struct *tsk); void libcfs_debug_dumplog(void); int libcfs_debug_init(unsigned long bufsize); int libcfs_debug_cleanup(void); @@ -256,72 +251,54 @@ void cfs_array_free(void *vars); #if LASSERT_ATOMIC_ENABLED /** assert value of @a is equal to @v */ -#define LASSERT_ATOMIC_EQ(a, v) \ -do { \ - LASSERTF(atomic_read(a) == v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_EQ(a, v) \ + LASSERTF(atomic_read(a) == v, "value: %d\n", atomic_read((a))); /** assert value of @a is unequal to @v */ -#define LASSERT_ATOMIC_NE(a, v) \ -do { \ - LASSERTF(atomic_read(a) != v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_NE(a, v) \ + LASSERTF(atomic_read(a) != v, "value: %d\n", atomic_read((a))); /** assert value of @a is little than @v */ -#define LASSERT_ATOMIC_LT(a, v) \ -do { \ - LASSERTF(atomic_read(a) < v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_LT(a, v) \ + LASSERTF(atomic_read(a) < v, "value: %d\n", atomic_read((a))); /** assert value of @a is little/equal to @v */ -#define LASSERT_ATOMIC_LE(a, v) \ -do { \ - LASSERTF(atomic_read(a) <= v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_LE(a, v) \ + LASSERTF(atomic_read(a) <= v, "value: %d\n", atomic_read((a))); /** assert value of @a is great than @v */ -#define LASSERT_ATOMIC_GT(a, v) \ -do { \ - LASSERTF(atomic_read(a) > v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_GT(a, v) \ + LASSERTF(atomic_read(a) > v, "value: %d\n", atomic_read((a))); /** assert value of @a is great/equal to @v */ -#define LASSERT_ATOMIC_GE(a, v) \ -do { \ - LASSERTF(atomic_read(a) >= v, \ - "value: %d\n", atomic_read((a))); \ -} while (0) +#define LASSERT_ATOMIC_GE(a, v) \ + LASSERTF(atomic_read(a) >= v, "value: %d\n", atomic_read((a))); /** assert value of @a is great than @v1 and little than @v2 */ -#define LASSERT_ATOMIC_GT_LT(a, v1, v2) \ -do { \ - int __v = atomic_read(a); \ - LASSERTF(__v > v1 && __v < v2, "value: %d\n", __v); \ +#define LASSERT_ATOMIC_GT_LT(a, v1, v2) \ +do { \ + int __v = atomic_read(a); \ + LASSERTF(__v > v1 && __v < v2, "value: %d\n", __v);\ } while (0) /** assert value of @a is great than @v1 and little/equal to @v2 */ -#define LASSERT_ATOMIC_GT_LE(a, v1, v2) \ -do { \ - int __v = atomic_read(a); \ - LASSERTF(__v > v1 && __v <= v2, "value: %d\n", __v); \ +#define LASSERT_ATOMIC_GT_LE(a, v1, v2) \ +do { \ + int __v = atomic_read(a); \ + LASSERTF(__v > v1 && __v <= v2, "value: %d\n", __v);\ } while (0) /** assert value of @a is great/equal to @v1 and little than @v2 */ -#define LASSERT_ATOMIC_GE_LT(a, v1, v2) \ -do { \ - int __v = atomic_read(a); \ - LASSERTF(__v >= v1 && __v < v2, "value: %d\n", __v); \ +#define LASSERT_ATOMIC_GE_LT(a, v1, v2) \ +do { \ + int __v = atomic_read(a); \ + LASSERTF(__v >= v1 && __v < v2, "value: %d\n", __v);\ } while (0) /** assert value of @a is great/equal to @v1 and little/equal to @v2 */ #define LASSERT_ATOMIC_GE_LE(a, v1, v2) \ do { \ - int __v = atomic_read(a); \ + int __v = atomic_read(a); \ LASSERTF(__v >= v1 && __v <= v2, "value: %d\n", __v); \ } while (0) diff --git a/libcfs/libcfs/debug.c b/libcfs/libcfs/debug.c index c1082ea..09e3db7 100644 --- a/libcfs/libcfs/debug.c +++ b/libcfs/libcfs/debug.c @@ -37,25 +37,26 @@ # define DEBUG_SUBSYSTEM S_LNET +#include #include +#include #include #include #include -#include #include "tracefile.h" static char debug_file_name[1024]; unsigned int libcfs_subsystem_debug = ~0; +EXPORT_SYMBOL(libcfs_subsystem_debug); module_param(libcfs_subsystem_debug, int, 0644); MODULE_PARM_DESC(libcfs_subsystem_debug, "Lustre kernel debug subsystem mask"); -EXPORT_SYMBOL(libcfs_subsystem_debug); unsigned int libcfs_debug = (D_CANTMASK | D_NETERROR | D_HA | D_CONFIG | D_IOCTL | D_LFSCK | D_TTY); +EXPORT_SYMBOL(libcfs_debug); module_param(libcfs_debug, int, 0644); MODULE_PARM_DESC(libcfs_debug, "Lustre kernel debug mask"); -EXPORT_SYMBOL(libcfs_debug); static int libcfs_param_debug_mb_set(const char *val, cfs_kernel_param_arg_t *kp) @@ -78,12 +79,11 @@ static int libcfs_param_debug_mb_set(const char *val, return 0; } -/* - * While debug_mb setting look like unsigned int, in fact +/* While debug_mb setting look like unsigned int, in fact * it needs quite a bunch of extra processing, so we define special * debug_mb parameter type with corresponding methods to handle this case */ -static struct kernel_param_ops param_ops_debug_mb = { +static const struct kernel_param_ops param_ops_debug_mb = { .set = libcfs_param_debug_mb_set, .get = param_get_uint, }; @@ -149,7 +149,7 @@ static int param_set_console_max_delay(const char *val, libcfs_console_min_delay, INT_MAX); } -static struct kernel_param_ops param_ops_console_max_delay = { +static const struct kernel_param_ops param_ops_console_max_delay = { .set = param_set_console_max_delay, .get = param_get_delay, }; @@ -172,7 +172,7 @@ static int param_set_console_min_delay(const char *val, 1, libcfs_console_max_delay); } -static struct kernel_param_ops param_ops_console_min_delay = { +static const struct kernel_param_ops param_ops_console_min_delay = { .set = param_set_console_min_delay, .get = param_get_delay, }; @@ -212,7 +212,7 @@ static int param_set_uintpos(const char *val, return param_set_uint_minmax(val, kp, 1, -1); } -static struct kernel_param_ops param_ops_uintpos = { +static const struct kernel_param_ops param_ops_uintpos = { .set = param_set_uintpos, .get = param_get_uint, }; @@ -260,13 +260,11 @@ MODULE_PARM_DESC(libcfs_debug_file_path, int libcfs_panic_in_progress; -/* - * libcfs_debug_token2mask() expects the returned - * string in lower-case - */ +/* libcfs_debug_token2mask() expects the returned string in lower-case */ static const char *libcfs_debug_subsys2str(int subsys) { - static const char *libcfs_debug_subsystems[] = LIBCFS_DEBUG_SUBSYS_NAMES; + static const char * const libcfs_debug_subsystems[] = + LIBCFS_DEBUG_SUBSYS_NAMES; if (subsys >= ARRAY_SIZE(libcfs_debug_subsystems)) return NULL; @@ -274,13 +272,11 @@ static const char *libcfs_debug_subsys2str(int subsys) return libcfs_debug_subsystems[subsys]; } -/* - * libcfs_debug_token2mask() expects the returned - * string in lower-case - */ +/* libcfs_debug_token2mask() expects the returned string in lower-case */ static const char *libcfs_debug_dbg2str(int debug) { - static const char *libcfs_debug_masks[] = LIBCFS_DEBUG_MASKS_NAMES; + static const char * const libcfs_debug_masks[] = + LIBCFS_DEBUG_MASKS_NAMES; if (debug >= ARRAY_SIZE(libcfs_debug_masks)) return NULL; @@ -307,7 +303,7 @@ libcfs_debug_mask2str(char *str, int size, int mask, int is_subsys) continue; token = fn(i); - if (token == NULL) /* unused bit */ + if (!token) /* unused bit */ continue; if (len > 0) { /* separator? */ @@ -349,14 +345,11 @@ libcfs_debug_str2mask(int *mask, const char *str, int is_subsys) if (!isspace(str[n-1])) break; matched = n; - t = sscanf(str, "%i%n", &m, &matched); if (t >= 1 && matched == n) { /* don't print warning for lctl set_param debug=0 or -1 */ if (m != 0 && m != -1) - CWARN("You are trying to use a numerical value for the " - "mask - this will be deprecated in a future " - "release.\n"); + CWARN("You are trying to use a numerical value for the mask - this will be deprecated in a future release.\n"); *mask = m; return 0; } @@ -371,11 +364,11 @@ char lnet_debug_log_upcall[1024] = "/usr/lib/lustre/lnet_debug_log_upcall"; * * @file path of the dumped log */ -void libcfs_run_debug_log_upcall(char *file) +static void libcfs_run_debug_log_upcall(char *file) { char *argv[3]; int rc; - char *envp[] = { + static const char * const envp[] = { "HOME=/", "PATH=/sbin:/bin:/usr/sbin:/usr/bin", NULL @@ -385,11 +378,11 @@ void libcfs_run_debug_log_upcall(char *file) argv[0] = lnet_debug_log_upcall; LASSERTF(file, "called on a null filename\n"); - argv[1] = file; //only need to pass the path of the file + argv[1] = file; /* only need to pass the path of the file */ argv[2] = NULL; - rc = call_usermodehelper(argv[0], argv, envp, 1); + rc = call_usermodehelper(argv[0], argv, (char **)envp, 1); if (rc < 0 && rc != -ENOENT) { CERROR("Error %d invoking LNET debug log upcall %s %s; check /sys/kernel/debug/lnet/debug_log_upcall\n", rc, argv[0], argv[1]); @@ -422,6 +415,7 @@ void libcfs_debug_dumplog_internal(void *arg) cfs_tracefile_dump_all_pages(debug_file_name); libcfs_run_debug_log_upcall(debug_file_name); } + current->journal_info = journal_info; } @@ -464,7 +458,7 @@ void libcfs_debug_dumplog(void) EXPORT_SYMBOL(libcfs_debug_dumplog); /* coverity[+kill] */ -void lbug_with_loc(struct libcfs_debug_msg_data *msgdata) +void __noreturn lbug_with_loc(struct libcfs_debug_msg_data *msgdata) { libcfs_catastrophe = 1; libcfs_debug_msg(msgdata, "LBUG\n"); @@ -655,8 +649,8 @@ static void libcfs_unregister_panic_notifier(void) int libcfs_debug_init(unsigned long bufsize) { - int rc = 0; unsigned int max = libcfs_debug_mb; + int rc = 0; if (libcfs_console_max_delay <= 0 || /* not set by user or */ libcfs_console_min_delay <= 0 || /* set to invalid values */ @@ -665,7 +659,7 @@ int libcfs_debug_init(unsigned long bufsize) libcfs_console_min_delay = CDEBUG_DEFAULT_MIN_DELAY; } - if (libcfs_debug_file_path != NULL) { + if (libcfs_debug_file_path) { strlcpy(libcfs_debug_file_path_arr, libcfs_debug_file_path, sizeof(libcfs_debug_file_path_arr)); @@ -678,7 +672,7 @@ int libcfs_debug_init(unsigned long bufsize) max = TCD_MAX_PAGES; } else { max = (max / num_possible_cpus()); - max = (max << (20 - PAGE_SHIFT)); + max <<= (20 - PAGE_SHIFT); } rc = cfs_tracefile_init(max); @@ -708,20 +702,20 @@ int libcfs_debug_clear_buffer(void) return 0; } -/* - * Debug markers, although printed by S_LNET - * should not be be marked as such. - */ +/* Debug markers, although printed by S_LNET should not be be marked as such. */ #undef DEBUG_SUBSYSTEM #define DEBUG_SUBSYSTEM S_UNDEFINED int libcfs_debug_mark_buffer(const char *text) { - CDEBUG(D_TRACE, "**************************************************\n"); + CDEBUG(D_TRACE, + "**************************************************\n"); LCONSOLE(D_WARNING, "DEBUG MARKER: %s\n", text); - CDEBUG(D_TRACE, "**************************************************\n"); + CDEBUG(D_TRACE, + "**************************************************\n"); return 0; } + #undef DEBUG_SUBSYSTEM #define DEBUG_SUBSYSTEM S_LNET diff --git a/libcfs/libcfs/tracefile.h b/libcfs/libcfs/tracefile.h index cd9a9fb..0ab2aec 100644 --- a/libcfs/libcfs/tracefile.h +++ b/libcfs/libcfs/tracefile.h @@ -44,8 +44,6 @@ extern long long cfs_tracefile_size; */ extern char lnet_debug_log_upcall[1024]; -extern void libcfs_run_debug_log_upcall(char *file); - int cfs_tracefile_dump_all_pages(char *filename); void cfs_trace_debug_print(void); void cfs_trace_flush_pages(void); diff --git a/lustre/obdclass/cl_page.c b/lustre/obdclass/cl_page.c index 9723b38..31ddf2c 100644 --- a/lustre/obdclass/cl_page.c +++ b/lustre/obdclass/cl_page.c @@ -350,7 +350,7 @@ struct cl_page *cl_page_find(const struct lu_env *env, if (type == CPT_CACHEABLE) { /* vmpage lock is used to protect the child/parent * relationship */ - KLASSERT(PageLocked(vmpage)); + LASSERT(PageLocked(vmpage)); /* * cl_vmpage_page() can be called here without any locks as * @@ -515,7 +515,7 @@ struct cl_page *cl_vmpage_page(struct page *vmpage, struct cl_object *obj) struct cl_page *page; ENTRY; - KLASSERT(PageLocked(vmpage)); + LASSERT(PageLocked(vmpage)); /* * NOTE: absence of races and liveness of data are guaranteed by page -- 1.8.3.1