Details
-
Improvement
-
Resolution: Fixed
-
Minor
-
None
-
None
-
9223372036854775807
Description
Statically allocated completions, list heads, mutexes, timer lists, atomics, spinlocks, wait queues, etc should be statically initialized using kernel provided macros. Dynamic initialization can be identified by auditing module_init() functions:
static __init int ptlrpc_init(void) { ... #if RS_DEBUG spin_lock_init(&ptlrpc_rs_debug_lock); #endif INIT_LIST_HEAD(&ptlrpc_all_services); mutex_init(&ptlrpc_all_services_mutex); mutex_init(&pinger_mutex); mutex_init(&ptlrpcd_mutex); ... }
Here spin_lock_init(&ptlrpc_rs_debug_lock) can be dropped if prlrpc_rs_debug_lock is defined using DEFINE_SPINLOCK() and similarly the other initializations shown above.
Regex searches can also identify top level definitions of these structures that fail to use the initializer macros:
lustre-release$ git grep -E -e '^(static[ \t]+)?struct[ \t]+((completion)|(list_head)|(mutex)|(timer_list))' -e '^(static[ \t]+)((atomic_t)|(spinlock_t)|(wait_queue_head_t))' libcfs/include/libcfs/util/list.h:struct list_head { libcfs/libcfs/debug.c:static wait_queue_head_t debug_ctlwq; libcfs/libcfs/tracefile.c:static atomic_t cfs_tage_allocated = ATOMIC_INIT(0); libcfs/libcfs/watchdog.c:static struct completion lcw_start_completion; libcfs/libcfs/watchdog.c:static struct completion lcw_stop_completion; libcfs/libcfs/watchdog.c:static wait_queue_head_t lcw_event_waitq; libcfs/libcfs/watchdog.c:static struct list_head lcw_pending_timers = LIST_HEAD_INIT(lcw_pending_timers); lnet/include/lnet/lib-lnet.h:struct list_head *lnet_mt_match_head(struct lnet_match_table *mtable, lnet/include/lnet/nidstr.h:struct list_head; lnet/klnds/o2iblnd/o2iblnd.c:struct list_head * lnet/klnds/o2iblnd/o2iblnd.h:struct list_head *kiblnd_pool_alloc_node(kib_poolset_t *ps); lnet/lnet/lib-ptl.c:struct list_head * lnet/lnet/module.c:static struct mutex lnet_config_mutex; lustre/ldlm/ldlm_internal.h:struct list_head *ldlm_namespace_inactive_list(enum ldlm_side client) lustre/ldlm/ldlm_lockd.c:static struct mutex ldlm_ref_mutex; lustre/ldlm/ldlm_lockd.c:static spinlock_t waiting_locks_spinlock; /* BH lock (timer) */ lustre/ldlm/ldlm_lockd.c:static struct list_head waiting_locks_list; lustre/ldlm/ldlm_lockd.c:static struct timer_list waiting_locks_timer; lustre/ldlm/ldlm_pool.c:static struct completion ldlm_pools_comp; lustre/ldlm/ldlm_reclaim.c:static atomic_t ldlm_nr_reclaimer; lustre/ldlm/ldlm_resource.c:struct mutex ldlm_srv_namespace_lock; lustre/ldlm/ldlm_resource.c:struct list_head ldlm_srv_namespace_list; lustre/ldlm/ldlm_resource.c:struct mutex ldlm_cli_namespace_lock; lustre/ldlm/ldlm_resource.c:struct list_head ldlm_cli_active_namespace_list; lustre/ldlm/ldlm_resource.c:struct list_head ldlm_cli_inactive_namespace_list; lustre/lfsck/lfsck_lib.c:static struct list_head lfsck_instance_list; lustre/lfsck/lfsck_lib.c:static struct list_head lfsck_ost_orphan_list; lustre/lfsck/lfsck_lib.c:static struct list_head lfsck_mdt_orphan_list; lustre/mgc/mgc_request.c:static struct list_head config_llog_list = LIST_HEAD_INIT(config_llog_list); lustre/mgc/mgc_request.c:static wait_queue_head_t rq_waitq; lustre/mgc/mgc_request.c:static atomic_t mgc_count = ATOMIC_INIT(0); lustre/obdclass/cl_object.c:static spinlock_t *cl_object_attr_guard(struct cl_object *o) lustre/obdclass/class_obd.c:struct list_head obd_types; lustre/obdclass/genops.c:static struct list_head obd_zombie_imports; lustre/obdclass/genops.c:static struct list_head obd_zombie_exports; lustre/obdclass/genops.c:static spinlock_t obd_zombie_impexp_lock; lustre/obdclass/genops.c:struct list_head obd_stale_exports; lustre/obdclass/genops.c:static struct completion obd_zombie_start; lustre/obdclass/genops.c:static struct completion obd_zombie_stop; lustre/obdclass/genops.c:static wait_queue_head_t obd_zombie_waitq; lustre/obdclass/kernelcomm.c:static struct list_head kkuc_groups[KUC_GRP_MAX+1] = {}; lustre/obdclass/local_storage.c:static struct list_head ls_list_head = LIST_HEAD_INIT(ls_list_head); lustre/obdclass/lu_object.c:static struct list_head lu_device_types; lustre/obdclass/lu_object.c:static struct list_head lu_sites; lustre/obdclass/lu_object.c:static atomic_t lu_key_initing_cnt = ATOMIC_INIT(0); lustre/obdclass/lu_object.c:static struct list_head lu_context_remembered; lustre/obdclass/lu_ref.c:static struct list_head lu_ref_refs; lustre/obdclass/lu_ref.c:static spinlock_t lu_ref_refs_guard; lustre/obdclass/lustre_handles.c:static spinlock_t handle_base_lock; lustre/obdclass/lustre_peer.c:static struct list_head g_uuid_list; lustre/obdclass/lustre_peer.c:static spinlock_t g_uuid_lock; lustre/obdclass/obd_config.c:static struct list_head lustre_profile_list = lustre/obdclass/obd_mount_server.c:static struct list_head server_mount_info_list = lustre/obdclass/obd_mount_server.c:static struct list_head lwp_register_list = lustre/osc/osc_request.c:struct list_head osc_shrink_list = LIST_HEAD_INIT(osc_shrink_list); lustre/osd-ldiskfs/osd_iam.c:static struct list_head iam_formats = LIST_HEAD_INIT(iam_formats); lustre/osd-ldiskfs/osd_oi.c:static struct mutex oi_init_lock; lustre/osp/osp_sync.c:static struct list_head osp_id_tracker_list = lustre/ptlrpc/client.c:static spinlock_t ptlrpc_last_xid_lock; lustre/ptlrpc/gss/gss_krb5_mech.c:static spinlock_t krb5_seq_lock; lustre/ptlrpc/gss/gss_mech_switch.c:static struct list_head registered_mechs = LIST_HEAD_INIT(registered_mechs); lustre/ptlrpc/gss/gss_pipefs.c:static atomic_t upcall_seq = ATOMIC_INIT(0); lustre/ptlrpc/gss/gss_pipefs.c:static struct list_head upcall_lists[MECH_MAX]; lustre/ptlrpc/gss/gss_pipefs.c:static spinlock_t upcall_locks[MECH_MAX]; lustre/ptlrpc/gss/gss_svc_upcall.c:static spinlock_t __ctx_index_lock; lustre/ptlrpc/pack_generic.c:struct list_head ptlrpc_rs_debug_lru = lustre/ptlrpc/pinger.c:struct mutex pinger_mutex; lustre/ptlrpc/pinger.c:static struct list_head pinger_imports = lustre/ptlrpc/pinger.c:static struct list_head timeout_list = lustre/ptlrpc/pinger.c:static wait_queue_head_t pet_waitq; lustre/ptlrpc/pinger.c:static struct list_head pet_list; lustre/ptlrpc/ptlrpcd.c:struct mutex ptlrpcd_mutex; lustre/ptlrpc/sec.c:static atomic_t sptlrpc_sec_id = ATOMIC_INIT(1); lustre/ptlrpc/sec_config.c:static struct mutex sptlrpc_conf_lock; lustre/ptlrpc/sec_config.c:static struct list_head sptlrpc_confs; lustre/ptlrpc/sec_gc.c:static struct mutex sec_gc_mutex; lustre/ptlrpc/sec_gc.c:static spinlock_t sec_gc_list_lock; lustre/ptlrpc/sec_gc.c:static struct list_head sec_gc_list; lustre/ptlrpc/sec_gc.c:static spinlock_t sec_gc_ctx_list_lock; lustre/ptlrpc/sec_gc.c:static struct list_head sec_gc_ctx_list; lustre/ptlrpc/sec_gc.c:static atomic_t sec_gc_wait_del = ATOMIC_INIT(0); lustre/ptlrpc/service.c:struct list_head ptlrpc_all_services; lustre/ptlrpc/service.c:struct mutex ptlrpc_all_services_mutex; lustre/quota/qsd_config.c:static struct list_head qfs_list = LIST_HEAD_INIT(qfs_list); lustre/tests/it_test.c:static struct list_head header = LIST_HEAD_INIT(header);
Note that there are some easily identified false positives among the matches.
Often this means that the structure can be declared static in the file where it is defined.
One patch per module (where needed) would probably be about right. Except for LNet which is by far the worst offender. Fixing it would mean breaking up the the_lnet object. So it wouldn't be a small change but the change could easily be broken into small chunks by working file by file. For example several members (including ln_eq_waitq) are only used in lib-eq.c and could be removed and replaced with a static object in lib-eq.c (static DECLARE_WAIT_QUEUE_HEAD(lnet_eq_waitq)).
I'll upload a patch for ldlm as an example.
Attachments
Issue Links
- is related to
-
LU-6142 Enforce Linux kernel coding style in all code
- Open