Revert "ocfs2: incorrect check for debugfs returns"
This reverts commit e2ac55b6a8e337fac7cc59c6f452caac92ab5ee6.
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 <ying.huang@intel.com>
Acked-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Chengyu Song <csong84@gatech.edu>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 8e19b9d..16eff45 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1312,9 +1312,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 956edf6..8b23aa2 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2959,7 +2959,7 @@
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 837ddce..403c566 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1112,7 +1112,7 @@
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 @@
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 @@
}
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;
}