ocfs2: remove redundant and incorrect mlog_error

We have already mlog all error and positive status is not error.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/quota_global.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

fs/ocfs2/quota_global.c +0 -2

index 92fcd57..0a86e30 100644
@@ -399,8 +399,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 			      msecs_to_jiffies(oinfo->dqi_syncms));
 
 out_err:
-	if (status)
-		mlog_errno(status);
 	return status;
 out_unlock:
 	ocfs2_unlock_global_qf(oinfo, 0);

Signature · 1 line

1.7.4.4

Quoted 1 line · Tiger Yang

We have already mlog all error and positive status is not error.

Positive status is turned into -EIO. There are actually a
couple of places in this function that do not mlog_errno(status) and rely on this print. I think you should add them to your patch.
For example:

371 status = ocfs2_qinfo_lock(oinfo, 0);
372 if (status< 0)
373 goto out_unlock;

Joel

Quoted 7 lines · Joel Becker

Quoted 1 line · Tiger Yang

We have already mlog all error and positive status is not error.

Positive status is turned into -EIO. There are actually a
couple of places in this function that do not mlog_errno(status) and rely on this print. I think you should add them to your patch.
For example:

Quoted 7 lines

371 status = ocfs2_qinfo_lock(oinfo, 0);
372 if (status< 0)
373 goto out_unlock;

Joel

Hi, Joel,
There is something devious about this function.
1 If status == sizeof(struct ocfs2_global_disk_dqinfo)) then positive status will be return.
2 After goto out_unlock, they goto out_err again.
I make a new one to fix this. please review the attached patch.

Thanks,
Tiger

Quoted 1 line

From 36951746bb68250ce13a62fd2a3290fa8af30c54 Mon Sep 17 00:00:00 2001

From: Tiger Yang <[email protected]>
Date: Thu, 2 Jun 2011 11:27:11 +0800
Subject: [PATCH 1/1] ocfs2: remove redundant and incorrect mlog_error

We have already mlog all error and positive status is not error.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/quota_global.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

fs/ocfs2/quota_global.c +3 -8

index 92fcd57..cb0f017 100644
@@ -358,10 +358,8 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 	oinfo->dqi_gqi_count = 0;
 	oinfo->dqi_gqinode = gqinode;
 	status = ocfs2_lock_global_qf(oinfo, 0);
-	if (status < 0) {
-		mlog_errno(status);
+	if (status < 0)
 		goto out_err;
-	}
 
 	status = ocfs2_extent_map_get_blocks(gqinode, 0, &oinfo->dqi_giblk,
 					     &pcount, NULL);

@@ -381,7 +379,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 		     status);
 		if (status >= 0)
 			status = -EIO;
-		mlog_errno(status);
 		goto out_err;
 	}
 	info->dqi_bgrace = le32_to_cpu(dinfo.dqi_bgrace);

@@ -398,14 +395,12 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 	schedule_delayed_work(&oinfo->dqi_sync_work,
 			      msecs_to_jiffies(oinfo->dqi_syncms));
 
-out_err:
-	if (status)
-		mlog_errno(status);
 	return status;
 out_unlock:
 	ocfs2_unlock_global_qf(oinfo, 0);
+out_err:
 	mlog_errno(status);
-	goto out_err;
+	return status;
 }
 
 /* Write information to global quota file. Expects exlusive lock on quota

Signature · 1 line

1.7.4.4

Quoted 32 lines · Tiger Yang

Quoted 7 lines · Joel Becker

Quoted 1 line · Tiger Yang

We have already mlog all error and positive status is not error.

Positive status is turned into -EIO. There are actually a
couple of places in this function that do not mlog_errno(status) and rely on this print. I think you should add them to your patch.
For example:

Quoted 7 lines

371 status = ocfs2_qinfo_lock(oinfo, 0);
372 if (status< 0)
373 goto out_unlock;

Joel

Hi, Joel,
There is something devious about this function.
1 If status == sizeof(struct ocfs2_global_disk_dqinfo)) then positive status will be return.
2 After goto out_unlock, they goto out_err again.
I make a new one to fix this. please review the attached patch.

Thanks,
Tiger

Quoted 1 line

From 36951746bb68250ce13a62fd2a3290fa8af30c54 Mon Sep 17 00:00:00 2001

From: Tiger Yang <[email protected]>
Date: Thu, 2 Jun 2011 11:27:11 +0800
Subject: [PATCH 1/1] ocfs2: remove redundant and incorrect mlog_error

Quoted 3 lines

We have already mlog all error and positive status is not error.

Signed-off-by Tiger Yang <[email protected]>

This patch is now part of the fixes branch of ocfs2.git.

Joel

Quoted 48 lines

---
 fs/ocfs2/quota_global.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

fs/ocfs2/quota_global.c +3 -8

index 92fcd57..cb0f017 100644
@@ -358,10 +358,8 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 	oinfo->dqi_gqi_count = 0;
 	oinfo->dqi_gqinode = gqinode;
 	status = ocfs2_lock_global_qf(oinfo, 0);
-	if (status < 0) {
-		mlog_errno(status);
+	if (status < 0)
 		goto out_err;
-	}
 
 	status = ocfs2_extent_map_get_blocks(gqinode, 0, &oinfo->dqi_giblk,
 					     &pcount, NULL);

@@ -381,7 +379,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 		     status);
 		if (status >= 0)
 			status = -EIO;
-		mlog_errno(status);
 		goto out_err;
 	}
 	info->dqi_bgrace = le32_to_cpu(dinfo.dqi_bgrace);

@@ -398,14 +395,12 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 	schedule_delayed_work(&oinfo->dqi_sync_work,
 			      msecs_to_jiffies(oinfo->dqi_syncms));
 
-out_err:
-	if (status)
-		mlog_errno(status);
 	return status;
 out_unlock:
 	ocfs2_unlock_global_qf(oinfo, 0);
+out_err:
 	mlog_errno(status);
-	goto out_err;
+	return status;
 }
 
 /* Write information to global quota file. Expects exlusive lock on quota

Signature · 1 line

1.7.4.4

Signature · 11 lines

"The opposite of a correct statement is a false statement. The
 opposite of a profound truth may well be another profound truth."
         - Niels Bohr 

			http://www.jlbec.org/
			jlbec at evilplan.org

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel