Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1579036 > unrolled thread

[PATCH 0/3] staging: lustre: llite: fixups for ll_migrate()

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] staging: lustre: llite: fixups for ll_migrate() James Simmons <jsimmons@infradead.org> - 2017-02-11 18:20 +0100
    [PATCH 2/3] staging: lustre: llite: root inode checking for migration James Simmons <jsimmons@infradead.org> - 2017-02-11 18:20 +0100
      Re: [PATCH 2/3] staging: lustre: llite: root inode checking for  migration Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-02-12 13:20 +0100
    [PATCH 1/3] staging: lustre: llite: check request != NULL in ll_migrate James Simmons <jsimmons@infradead.org> - 2017-02-11 18:20 +0100
    [PATCH 3/3] staging: lustre: llite: check reply status in ll_migrate() James Simmons <jsimmons@infradead.org> - 2017-02-11 18:20 +0100

#1579036 — [PATCH 0/3] staging: lustre: llite: fixups for ll_migrate()

This patch series covers all the current fixes for the function
ll_migrate(). Originally only the first patch was submitted but
it was pointed out other flaws existed in the code. Two more
patches that cover those flaws are included in this patch set.
These patches are order dependent.

Niu Yawei (1):
  staging: lustre: llite: check reply status in ll_migrate()

wang di (2):
  staging: lustre: llite: check request != NULL in ll_migrate
  staging: lustre: llite: root inode checking for migration

 drivers/staging/lustre/lustre/llite/file.c | 63 ++++++++++++++++--------------
 1 file changed, 34 insertions(+), 29 deletions(-)

--
1.8.3.1

[toc] | [next] | [standalone]

#1579038 — [PATCH 2/3] staging: lustre: llite: root inode checking for migration

From: wang di <di.wang@intel.com>

Do not migrate root inode.

Signed-off-by: wang di <di.wang@intel.com>
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7577
Reviewed-on: http://review.whamcloud.com/17669
Reviewed-by: Lai Siyao <lai.siyao@intel.com>
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/file.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 0c83bd7..271608d 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2626,18 +2626,18 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 		       ll_get_fsname(parent->i_sb, NULL, 0), name,
 		       PFID(&op_data->op_fid3));
 		rc = -EINVAL;
-		goto out_free;
+		goto out_unlock;
 	}
 
 	rc = ll_get_mdt_idx_by_fid(ll_i2sbi(parent), &op_data->op_fid3);
 	if (rc < 0)
-		goto out_free;
+		goto out_unlock;
 
 	if (rc == mdtidx) {
 		CDEBUG(D_INFO, "%s:"DFID" is already on MDT%d.\n", name,
 		       PFID(&op_data->op_fid3), mdtidx);
 		rc = 0;
-		goto out_free;
+		goto out_unlock;
 	}
 again:
 	if (S_ISREG(child_inode->i_mode)) {
@@ -2645,13 +2645,13 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 		if (IS_ERR(och)) {
 			rc = PTR_ERR(och);
 			och = NULL;
-			goto out_free;
+			goto out_unlock;
 		}
 
 		rc = ll_data_version(child_inode, &data_version,
 				     LL_DV_WR_FLUSH);
 		if (rc)
-			goto out_free;
+			goto out_close;
 
 		op_data->op_handle = och->och_fh;
 		op_data->op_data = och->och_mod;
@@ -2671,12 +2671,12 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 		body = req_capsule_server_get(&request->rq_pill, &RMF_MDT_BODY);
 		if (!body) {
 			rc = -EPROTO;
-			goto out_free;
+			goto out_close;
 		}
 
 		/*
 		 * If the server does release layout lock, then we cleanup
-		 * the client och here, otherwise release it in out_free:
+		 * the client och here, otherwise release it in out_close:
 		 */
 		if (och && body->mbo_valid & OBD_MD_CLOSE_INTENT_EXECED) {
 			obd_mod_put(och->och_mod);
@@ -2694,15 +2694,15 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 		request = NULL;
 		goto again;
 	}
-out_free:
-	if (child_inode) {
-		if (och) /* close the file */
-			ll_lease_close(och, child_inode, NULL);
+out_close:
+	if (och) /* close the file */
+		ll_lease_close(och, child_inode, NULL);
+	if (!rc)
 		clear_nlink(child_inode);
-		inode_unlock(child_inode);
-		iput(child_inode);
-	}
-
+out_unlock:
+	inode_unlock(child_inode);
+	iput(child_inode);
+out_free:
 	ll_finish_md_op_data(op_data);
 	return rc;
 }
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1579203 — Re: [PATCH 2/3] staging: lustre: llite: root inode checking for migration

On Sat, Feb 11, 2017 at 12:12:38PM -0500, James Simmons wrote:
> From: wang di <di.wang@intel.com>
> 
> Do not migrate root inode.

That says _what_ happens here, but you aren't giving any hint as to
_why_ this is needed.

I'll take this, but be more careful about your changelog comments in the
future...

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1579040 — [PATCH 1/3] staging: lustre: llite: check request != NULL in ll_migrate

From: wang di <di.wang@intel.com>

Check if the request is NULL, before retrieve reply body
from the request.

Signed-off-by: wang di <di.wang@intel.com>
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7396
Reviewed-on: http://review.whamcloud.com/17079
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/file.c | 41 +++++++++++++++++-------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 9870901..0c83bd7 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2667,28 +2667,33 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 	if (!rc)
 		ll_update_times(request, parent);
 
-	body = req_capsule_server_get(&request->rq_pill, &RMF_MDT_BODY);
-	if (!body) {
-		rc = -EPROTO;
-		goto out_free;
-	}
+	if (request) {
+		body = req_capsule_server_get(&request->rq_pill, &RMF_MDT_BODY);
+		if (!body) {
+			rc = -EPROTO;
+			goto out_free;
+		}
 
-	/*
-	 * If the server does release layout lock, then we cleanup
-	 * the client och here, otherwise release it in out_free:
-	 */
-	if (och && body->mbo_valid & OBD_MD_CLOSE_INTENT_EXECED) {
-		obd_mod_put(och->och_mod);
-		md_clear_open_replay_data(ll_i2sbi(parent)->ll_md_exp, och);
-		och->och_fh.cookie = DEAD_HANDLE_MAGIC;
-		kfree(och);
-		och = NULL;
-	}
+		/*
+		 * If the server does release layout lock, then we cleanup
+		 * the client och here, otherwise release it in out_free:
+		 */
+		if (och && body->mbo_valid & OBD_MD_CLOSE_INTENT_EXECED) {
+			obd_mod_put(och->och_mod);
+			md_clear_open_replay_data(ll_i2sbi(parent)->ll_md_exp,
+						  och);
+			och->och_fh.cookie = DEAD_HANDLE_MAGIC;
+			kfree(och);
+			och = NULL;
+		}
 
-	ptlrpc_req_finished(request);
+		ptlrpc_req_finished(request);
+	}
 	/* Try again if the file layout has changed. */
-	if (rc == -EAGAIN && S_ISREG(child_inode->i_mode))
+	if (rc == -EAGAIN && S_ISREG(child_inode->i_mode)) {
+		request = NULL;
 		goto again;
+	}
 out_free:
 	if (child_inode) {
 		if (och) /* close the file */
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1579041 — [PATCH 3/3] staging: lustre: llite: check reply status in ll_migrate()

From: Niu Yawei <yawei.niu@intel.com>

ll_migrate() should check reply status before trying to read
reply buffer, checking if request is NULL doesn't make sense.

Signed-off-by: Niu Yawei <yawei.niu@intel.com>
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8807
Reviewed-on: https://review.whamcloud.com/23666
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Lai Siyao <lai.siyao@intel.com>
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/file.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 271608d..10adfcd 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2664,15 +2664,12 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 	op_data->op_cli_flags = CLI_MIGRATE;
 	rc = md_rename(ll_i2sbi(parent)->ll_md_exp, op_data, name,
 		       namelen, name, namelen, &request);
-	if (!rc)
+	if (!rc) {
+		LASSERT(request);
 		ll_update_times(request, parent);
 
-	if (request) {
 		body = req_capsule_server_get(&request->rq_pill, &RMF_MDT_BODY);
-		if (!body) {
-			rc = -EPROTO;
-			goto out_close;
-		}
+		LASSERT(body);
 
 		/*
 		 * If the server does release layout lock, then we cleanup
@@ -2686,14 +2683,17 @@ int ll_migrate(struct inode *parent, struct file *file, int mdtidx,
 			kfree(och);
 			och = NULL;
 		}
+	}
 
+	if (request) {
 		ptlrpc_req_finished(request);
+		request = NULL;
 	}
+
 	/* Try again if the file layout has changed. */
-	if (rc == -EAGAIN && S_ISREG(child_inode->i_mode)) {
-		request = NULL;
+	if (rc == -EAGAIN && S_ISREG(child_inode->i_mode))
 		goto again;
-	}
+
 out_close:
 	if (och) /* close the file */
 		ll_lease_close(och, child_inode, NULL);
-- 
1.8.3.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web