From 7671ab9ea46df793536fb140e48d81646dca4e60 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 21 Apr 2015 09:17:28 -0700 Subject: [PATCH] Revert "ocfs2: incorrect check for debugfs returns" This reverts commit 30e407603542b2981e71bd535dba1790d7a0ae7f. Huang Ying reports that this causes a hang at boot with debugfs disabled. It is true that the debugfs error checks are kind of confusing, and this code certainly merits more cleanup and thinking about it, but there's something wrong with the trivial "check not just for NULL, but for error pointers too" patch. Yes, with debugfs disabled, we will end up setting the o2hb_debug_dir pointer variable to an error pointer (-ENODEV), and then continue as if everything was fine. But since debugfs is disabled, all the _users_ of that pointer end up being compiled away, so even though the pointer can not be dereferenced, that's still fine. So it's confusing and somewhat questionable, but the "more correct" error checks end up causing more trouble than they fix. Reported-by: Huang Ying Acked-by: Andrew Morton Acked-by: Chengyu Song Signed-off-by: Linus Torvalds --- fs/ocfs2/cluster/heartbeat.c | 42 ++++++++++-------------------------- fs/ocfs2/dlmglue.c | 2 +- fs/ocfs2/super.c | 9 ++++---- 3 files changed, 16 insertions(+), 37 deletions(-) diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index 8e19b9d7aba8f..16eff45727eea 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -1312,9 +1312,7 @@ static int o2hb_debug_init(void) int ret = -ENOMEM; o2hb_debug_dir = debugfs_create_dir(O2HB_DEBUG_DIR, NULL); - if (IS_ERR_OR_NULL(o2hb_debug_dir)) { - ret = o2hb_debug_dir ? - PTR_ERR(o2hb_debug_dir) : -ENOMEM; + if (!o2hb_debug_dir) { mlog_errno(ret); goto bail; } @@ -1327,9 +1325,7 @@ static int o2hb_debug_init(void) sizeof(o2hb_live_node_bitmap), O2NM_MAX_NODES, o2hb_live_node_bitmap); - if (IS_ERR_OR_NULL(o2hb_debug_livenodes)) { - ret = o2hb_debug_livenodes ? - PTR_ERR(o2hb_debug_livenodes) : -ENOMEM; + if (!o2hb_debug_livenodes) { mlog_errno(ret); goto bail; } @@ -1342,9 +1338,7 @@ static int o2hb_debug_init(void) sizeof(o2hb_live_region_bitmap), O2NM_MAX_REGIONS, o2hb_live_region_bitmap); - if (IS_ERR_OR_NULL(o2hb_debug_liveregions)) { - ret = o2hb_debug_liveregions ? - PTR_ERR(o2hb_debug_liveregions) : -ENOMEM; + if (!o2hb_debug_liveregions) { mlog_errno(ret); goto bail; } @@ -1358,9 +1352,7 @@ static int o2hb_debug_init(void) sizeof(o2hb_quorum_region_bitmap), O2NM_MAX_REGIONS, o2hb_quorum_region_bitmap); - if (IS_ERR_OR_NULL(o2hb_debug_quorumregions)) { - ret = o2hb_debug_quorumregions ? - PTR_ERR(o2hb_debug_quorumregions) : -ENOMEM; + if (!o2hb_debug_quorumregions) { mlog_errno(ret); goto bail; } @@ -1374,9 +1366,7 @@ static int o2hb_debug_init(void) sizeof(o2hb_failed_region_bitmap), O2NM_MAX_REGIONS, o2hb_failed_region_bitmap); - if (IS_ERR_OR_NULL(o2hb_debug_failedregions)) { - ret = o2hb_debug_failedregions ? - PTR_ERR(o2hb_debug_failedregions) : -ENOMEM; + if (!o2hb_debug_failedregions) { mlog_errno(ret); goto bail; } @@ -2010,8 +2000,7 @@ static int o2hb_debug_region_init(struct o2hb_region *reg, struct dentry *dir) reg->hr_debug_dir = debugfs_create_dir(config_item_name(®->hr_item), dir); - if (IS_ERR_OR_NULL(reg->hr_debug_dir)) { - ret = reg->hr_debug_dir ? PTR_ERR(reg->hr_debug_dir) : -ENOMEM; + if (!reg->hr_debug_dir) { mlog_errno(ret); goto bail; } @@ -2024,9 +2013,7 @@ static int o2hb_debug_region_init(struct o2hb_region *reg, struct dentry *dir) O2HB_DB_TYPE_REGION_LIVENODES, sizeof(reg->hr_live_node_bitmap), O2NM_MAX_NODES, reg); - if (IS_ERR_OR_NULL(reg->hr_debug_livenodes)) { - ret = reg->hr_debug_livenodes ? - PTR_ERR(reg->hr_debug_livenodes) : -ENOMEM; + if (!reg->hr_debug_livenodes) { mlog_errno(ret); goto bail; } @@ -2038,9 +2025,7 @@ static int o2hb_debug_region_init(struct o2hb_region *reg, struct dentry *dir) sizeof(*(reg->hr_db_regnum)), O2HB_DB_TYPE_REGION_NUMBER, 0, O2NM_MAX_NODES, reg); - if (IS_ERR_OR_NULL(reg->hr_debug_regnum)) { - ret = reg->hr_debug_regnum ? - PTR_ERR(reg->hr_debug_regnum) : -ENOMEM; + if (!reg->hr_debug_regnum) { mlog_errno(ret); goto bail; } @@ -2052,9 +2037,7 @@ static int o2hb_debug_region_init(struct o2hb_region *reg, struct dentry *dir) sizeof(*(reg->hr_db_elapsed_time)), O2HB_DB_TYPE_REGION_ELAPSED_TIME, 0, 0, reg); - if (IS_ERR_OR_NULL(reg->hr_debug_elapsed_time)) { - ret = reg->hr_debug_elapsed_time ? - PTR_ERR(reg->hr_debug_elapsed_time) : -ENOMEM; + if (!reg->hr_debug_elapsed_time) { mlog_errno(ret); goto bail; } @@ -2066,16 +2049,13 @@ static int o2hb_debug_region_init(struct o2hb_region *reg, struct dentry *dir) sizeof(*(reg->hr_db_pinned)), O2HB_DB_TYPE_REGION_PINNED, 0, 0, reg); - if (IS_ERR_OR_NULL(reg->hr_debug_pinned)) { - ret = reg->hr_debug_pinned ? - PTR_ERR(reg->hr_debug_pinned) : -ENOMEM; + if (!reg->hr_debug_pinned) { mlog_errno(ret); goto bail; } - return 0; + ret = 0; bail: - debugfs_remove_recursive(reg->hr_debug_dir); return ret; } diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c index 956edf67be20b..8b23aa2f52dda 100644 --- a/fs/ocfs2/dlmglue.c +++ b/fs/ocfs2/dlmglue.c @@ -2959,7 +2959,7 @@ static int ocfs2_dlm_init_debug(struct ocfs2_super *osb) osb->osb_debug_root, osb, &ocfs2_dlm_debug_fops); - if (IS_ERR_OR_NULL(dlm_debug->d_locking_state)) { + if (!dlm_debug->d_locking_state) { ret = -EINVAL; mlog(ML_ERROR, "Unable to create locking state debugfs file.\n"); diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index 837ddce4b659f..403c5660b3064 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -1112,7 +1112,7 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent) osb->osb_debug_root = debugfs_create_dir(osb->uuid_str, ocfs2_debugfs_root); - if (IS_ERR_OR_NULL(osb->osb_debug_root)) { + if (!osb->osb_debug_root) { status = -EINVAL; mlog(ML_ERROR, "Unable to create per-mount debugfs root.\n"); goto read_super_error; @@ -1122,7 +1122,7 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent) osb->osb_debug_root, osb, &ocfs2_osb_debug_fops); - if (IS_ERR_OR_NULL(osb->osb_ctxt)) { + if (!osb->osb_ctxt) { status = -EINVAL; mlog_errno(status); goto read_super_error; @@ -1606,9 +1606,8 @@ static int __init ocfs2_init(void) } ocfs2_debugfs_root = debugfs_create_dir("ocfs2", NULL); - if (IS_ERR_OR_NULL(ocfs2_debugfs_root)) { - status = ocfs2_debugfs_root ? - PTR_ERR(ocfs2_debugfs_root) : -ENOMEM; + if (!ocfs2_debugfs_root) { + status = -ENOMEM; mlog(ML_ERROR, "Unable to create ocfs2 debugfs root.\n"); goto out4; } -- 2.39.5