[PATCH 0/9] Remove wrapper function and tidy up the code

[PATCH 0/9] Remove wrapper function and tidy up the code

[Shivani Bhardwaj]

This patchset removes an unnecessary wrapper function and replaces all
its calls in different files with the standard function that it wrapped.
Also, one patch removes its prototype.
After applying this patch, code becomes cleaner.

Shivani Bhardwaj (9):
  Staging: lustre: dir: Replace function calls
  Staging: lustre: file: Replace function calls with standard function
  Staging: lustre: namei: Replace calls with kfree
  Staging: lustre: xattr_cache: Change function calls
  Staging: lustre: symlink: Substitute standard function
  Staging: lustre: llite_nfs: Replace with standard function
  Staging: lustre: llite_close: Substitute function calls
  Staging: lustre: llite_lib: Remove wrapper function
  Staging: lustre: llite_internal: Remove function prototype

 drivers/staging/lustre/lustre/llite/dir.c          | 14 ++++++-------
 drivers/staging/lustre/lustre/llite/file.c         | 24 +++++++++++-----------
 drivers/staging/lustre/lustre/llite/llite_close.c  |  2 +-
 .../staging/lustre/lustre/llite/llite_internal.h   |  1 -
 drivers/staging/lustre/lustre/llite/llite_lib.c    | 13 ++++--------
 drivers/staging/lustre/lustre/llite/llite_nfs.c    |  2 +-
 drivers/staging/lustre/lustre/llite/namei.c        | 12 +++++------
 drivers/staging/lustre/lustre/llite/symlink.c      |  2 +-
 drivers/staging/lustre/lustre/llite/xattr_cache.c  |  2 +-
 9 files changed, 33 insertions(+), 39 deletions(-)

-- 
2.1.0

[PATCH 1/9] Staging: lustre: dir: Replace function calls

[Shivani Bhardwaj]

Replace the calls of the function ll_finish_md_op_data() with the
standard function kfree().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/dir.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c
index 5c2ef92..ba3f469 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -182,7 +182,7 @@ static int ll_dir_filler(void *_hash, struct page *page0)
 	op_data->op_npages = npages;
 	op_data->op_offset = hash;
 	rc = md_readpage(exp, op_data, page_pool, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc < 0) {
 		/* page0 is special, which was added into page cache early */
 		delete_from_page_cache(page0);
@@ -363,7 +363,7 @@ struct page *ll_get_dir_page(struct inode *dir, __u64 hash,
 		rc = md_enqueue(ll_i2sbi(dir)->ll_md_exp, &einfo, &it,
 				op_data, &lockh, NULL, 0, NULL, 0);
 
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 
 		request = (struct ptlrpc_request *)it.d.lustre.it_data;
 		if (request)
@@ -669,7 +669,7 @@ static int ll_dir_setdirstripe(struct inode *dir, struct lmv_user_md *lump,
 			from_kuid(&init_user_ns, current_fsuid()),
 			from_kgid(&init_user_ns, current_fsgid()),
 			cfs_curproc_cap_pack(), 0, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (err)
 		goto err_exit;
 err_exit:
@@ -730,7 +730,7 @@ int ll_dir_setstripe(struct inode *inode, struct lov_user_md *lump,
 	/* swabbing is done in lov_setstripe() on server side */
 	rc = md_setattr(sbi->ll_md_exp, op_data, lump, lum_size,
 			NULL, 0, &req, NULL);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	ptlrpc_req_finished(req);
 	if (rc) {
 		if (rc != -EPERM && rc != -EACCES)
@@ -802,7 +802,7 @@ int ll_dir_getstripe(struct inode *inode, struct lov_mds_md **lmmp,
 
 	op_data->op_valid = OBD_MD_FLEASIZE | OBD_MD_FLDIREA;
 	rc = md_getattr(sbi->ll_md_exp, op_data, &req);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc < 0) {
 		CDEBUG(D_INFO, "md_getattr failed on inode %lu/%u: rc %d\n",
 		       inode->i_ino,
@@ -868,7 +868,7 @@ int ll_get_mdt_idx(struct inode *inode)
 	op_data->op_flags |= MF_GET_MDT_IDX;
 	rc = md_getattr(sbi->ll_md_exp, op_data, NULL);
 	mdtidx = op_data->op_mds;
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc < 0) {
 		CDEBUG(D_INFO, "md_getattr_name: %d\n", rc);
 		return rc;
@@ -1301,7 +1301,7 @@ static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		op_data->op_valid = OBD_MD_FLID;
 		rc = md_getattr_name(sbi->ll_md_exp, op_data, &request);
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		if (rc < 0) {
 			CDEBUG(D_INFO, "md_getattr_name: %d\n", rc);
 			goto out_free;
-- 
2.1.0

[PATCH 2/9] Staging: lustre: file: Replace function calls with standard function

[Shivani Bhardwaj]

Replace the calls of the function ll_finish_md_op_data() with the
standard function kfree().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/file.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 02f2759..186b5af 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -202,7 +202,7 @@ static int ll_close_inode_openhandle(struct obd_export *md_exp,
 			rc = -EBUSY;
 	}
 
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 out:
 	if (exp_connect_som(exp) && !epoch_close &&
@@ -420,7 +420,7 @@ static int ll_intent_file_open(struct dentry *dentry, void *lmm,
 	itp->it_flags |= MDS_OPEN_BY_FID;
 	rc = md_intent_lock(sbi->ll_md_exp, op_data, lmm, lmmsize, itp,
 			    0 /*unused */, &req, ll_md_blocking_ast, 0);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc == -ESTALE) {
 		/* reason for keep own exit path - don`t flood log
 		* with messages with -ESTALE errors.
@@ -819,7 +819,7 @@ ll_lease_open(struct inode *inode, struct file *file, fmode_t fmode,
 	 * open in ll_md_blocking_ast(). Otherwise as ll_md_blocking_lease_ast
 	 * doesn't deal with openhandle, so normal openhandle will be leaked. */
 				LDLM_FL_NO_LRU | LDLM_FL_EXCL);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	ptlrpc_req_finished(req);
 	if (rc < 0)
 		goto out_release_it;
@@ -1393,7 +1393,7 @@ int ll_lov_getstripe_ea_info(struct inode *inode, const char *filename,
 
 	op_data->op_valid = OBD_MD_FLEASIZE | OBD_MD_FLDIREA;
 	rc = md_getattr_name(sbi->ll_md_exp, op_data, &req);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc < 0) {
 		CDEBUG(D_INFO, "md_getattr_name failed on %s: rc %d\n",
 		       filename, rc);
@@ -2056,7 +2056,7 @@ static int ll_swap_layouts(struct file *file1, struct file *file2,
 
 	rc = obd_iocontrol(LL_IOC_LOV_SWAP_LAYOUTS, ll_i2mdexp(llss->inode1),
 			   sizeof(*op_data), op_data, NULL);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 putgl:
 	if (gid != 0) {
@@ -2131,7 +2131,7 @@ static int ll_hsm_state_set(struct inode *inode, struct hsm_state_set *hss)
 	rc = obd_iocontrol(LL_IOC_HSM_STATE_SET, ll_i2mdexp(inode),
 			   sizeof(*op_data), op_data, NULL);
 
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 	return rc;
 }
@@ -2350,7 +2350,7 @@ ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_to_user((void *)arg, hus, sizeof(*hus)))
 			rc = -EFAULT;
 
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		kfree(hus);
 		return rc;
 	}
@@ -2389,7 +2389,7 @@ ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_to_user((char *)arg, hca, sizeof(*hca)))
 			rc = -EFAULT;
 
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		kfree(hca);
 		return rc;
 	}
@@ -2761,7 +2761,7 @@ ll_file_flock(struct file *file, int cmd, struct file_lock *file_lock)
 		rc = rc2;
 	}
 
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 	return rc;
 }
@@ -2896,7 +2896,7 @@ static int __ll_inode_revalidate(struct dentry *dentry, __u64 ibits)
 				       based lookup */
 				    &oit, 0, &req,
 				    ll_md_blocking_ast, 0);
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		oit.it_create_mode &= ~M_CHECK_STALE;
 		if (rc < 0) {
 			rc = ll_inode_revalidate_fini(inode, rc);
@@ -2938,7 +2938,7 @@ static int __ll_inode_revalidate(struct dentry *dentry, __u64 ibits)
 
 		op_data->op_valid = valid;
 		rc = md_getattr(sbi->ll_md_exp, op_data, &req);
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		if (rc) {
 			rc = ll_inode_revalidate_fini(inode, rc);
 			return rc;
@@ -3533,7 +3533,7 @@ again:
 		ptlrpc_req_finished(it.d.lustre.it_data);
 	it.d.lustre.it_data = NULL;
 
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 	mode = it.d.lustre.it_lock_mode;
 	it.d.lustre.it_lock_mode = 0;
-- 
2.1.0

[PATCH 3/9] Staging: lustre: namei: Replace calls with kfree

[Shivani Bhardwaj]

Replace the calls of the function ll_finish_md_op_data() with thr
stndard function kfree().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/namei.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 2ca2200..f058ebe2 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -531,7 +531,7 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
 
 	rc = md_intent_lock(ll_i2mdexp(parent), op_data, NULL, 0, it,
 			    lookup_flags, &req, ll_md_blocking_ast, 0);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc < 0) {
 		retval = ERR_PTR(rc);
 		goto out;
@@ -786,7 +786,7 @@ static int ll_new_node(struct inode *dir, struct dentry *dentry,
 			from_kuid(&init_user_ns, current_fsuid()),
 			from_kgid(&init_user_ns, current_fsgid()),
 			cfs_curproc_cap_pack(), rdev, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (err)
 		goto err_exit;
 
@@ -961,7 +961,7 @@ static int ll_unlink(struct inode *dir, struct dentry *dentry)
 	ll_get_child_fid(dentry, &op_data->op_fid3);
 	op_data->op_fid2 = op_data->op_fid3;
 	rc = md_unlink(ll_i2sbi(dir)->ll_md_exp, op_data, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc)
 		goto out;
 
@@ -1011,7 +1011,7 @@ static int ll_rmdir(struct inode *dir, struct dentry *dentry)
 	ll_get_child_fid(dentry, &op_data->op_fid3);
 	op_data->op_fid2 = op_data->op_fid3;
 	rc = md_unlink(ll_i2sbi(dir)->ll_md_exp, op_data, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc == 0) {
 		ll_update_times(request, dir);
 		ll_stats_ops_tally(ll_i2sbi(dir), LPROC_LL_RMDIR, 1);
@@ -1060,7 +1060,7 @@ static int ll_link(struct dentry *old_dentry, struct inode *dir,
 		return PTR_ERR(op_data);
 
 	err = md_link(sbi->ll_md_exp, op_data, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (err)
 		goto out;
 
@@ -1096,7 +1096,7 @@ static int ll_rename(struct inode *old_dir, struct dentry *old_dentry,
 			old_dentry->d_name.len,
 			new_dentry->d_name.name,
 			new_dentry->d_name.len, &request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (!err) {
 		ll_update_times(request, old_dir);
 		ll_update_times(request, new_dir);
-- 
2.1.0

[PATCH 4/9] Staging: lustre: xattr_cache: Change function calls

[Shivani Bhardwaj]

Change the calls of the function ll_finish_md_op_data() to the standard
function kfree().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/xattr_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr_cache.c b/drivers/staging/lustre/lustre/llite/xattr_cache.c
index e1e599c..b7f3efc 100644
--- a/drivers/staging/lustre/lustre/llite/xattr_cache.c
+++ b/drivers/staging/lustre/lustre/llite/xattr_cache.c
@@ -306,7 +306,7 @@ static int ll_xattr_find_get_lock(struct inode *inode,
 	op_data->op_valid = OBD_MD_FLXATTR | OBD_MD_FLXATTRLS;
 
 	rc = md_enqueue(exp, &einfo, oit, op_data, &lockh, NULL, 0, NULL, 0);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 	if (rc < 0) {
 		CDEBUG(D_CACHE,
-- 
2.1.0

[PATCH 5/9] Staging: lustre: symlink: Substitute standard function

[Shivani Bhardwaj]

Substitute the standard function kfree() for the function
ll_finish_md_op_data().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/symlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/symlink.c b/drivers/staging/lustre/lustre/llite/symlink.c
index 69b2036..6b0d1cc 100644
--- a/drivers/staging/lustre/lustre/llite/symlink.c
+++ b/drivers/staging/lustre/lustre/llite/symlink.c
@@ -73,7 +73,7 @@ static int ll_readlink_internal(struct inode *inode,
 
 	op_data->op_valid = OBD_MD_LINKNAME;
 	rc = md_getattr(sbi->ll_md_exp, op_data, request);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc) {
 		if (rc != -ENOENT)
 			CERROR("inode %lu: rc = %d\n", inode->i_ino, rc);
-- 
2.1.0

[PATCH 6/9] Staging: lustre: llite_nfs: Replace with standard function

[Shivani Bhardwaj]

Replace the calls of the function ll_finish_md_op_data() with the
standard function kfree().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/llite_nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
index e578a11..a3a89a7 100644
--- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
+++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
@@ -306,7 +306,7 @@ static struct dentry *ll_get_parent(struct dentry *dchild)
 		return (void *)op_data;
 
 	rc = md_getattr_name(sbi->ll_md_exp, op_data, &req);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (rc) {
 		CERROR("failure %d inode %lu get parent\n", rc, dir->i_ino);
 		return ERR_PTR(rc);
-- 
2.1.0

[PATCH 7/9] Staging: lustre: llite_close: Substitute function calls

[Shivani Bhardwaj]

Substitute standard function kfree() in place of the function
ll_finish_md_op_data().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/llite_close.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_close.c b/drivers/staging/lustre/lustre/llite/llite_close.c
index 3f348a3..c4f22ec 100644
--- a/drivers/staging/lustre/lustre/llite/llite_close.c
+++ b/drivers/staging/lustre/lustre/llite/llite_close.c
@@ -301,7 +301,7 @@ static void ll_done_writing(struct inode *inode)
 		CERROR("inode %lu mdc done_writing failed: rc = %d\n",
 		       inode->i_ino, rc);
 out:
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 	if (och) {
 		md_clear_open_replay_data(ll_i2sbi(inode)->ll_md_exp, och);
 		kfree(och);
-- 
2.1.0

[PATCH 8/9] Staging: lustre: llite_lib: Remove wrapper function

[Shivani Bhardwaj]

Remove the function ll_finish_md_op_data() and replace all its calls
with the standrd function ll_finish_md_op_data().

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/llite_lib.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 4a8c759..143be87 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1355,7 +1355,7 @@ out:
 		if (!rc)
 			rc = rc1;
 	}
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 
 	if (!S_ISDIR(inode->i_mode)) {
 		mutex_lock(&inode->i_mutex);
@@ -1732,7 +1732,7 @@ int ll_iocontrol(struct inode *inode, struct file *file,
 
 		op_data->op_valid = OBD_MD_FLFLAGS;
 		rc = md_getattr(sbi->ll_md_exp, op_data, &req);
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		if (rc) {
 			CERROR("failure %d inode %lu\n", rc, inode->i_ino);
 			return -abs(rc);
@@ -1763,7 +1763,7 @@ int ll_iocontrol(struct inode *inode, struct file *file,
 		op_data->op_attr.ia_valid |= ATTR_ATTR_FLAG;
 		rc = md_setattr(sbi->ll_md_exp, op_data,
 				NULL, 0, NULL, 0, &req, NULL);
-		ll_finish_md_op_data(op_data);
+		kfree(op_data);
 		ptlrpc_req_finished(req);
 		if (rc)
 			return rc;
@@ -1934,7 +1934,7 @@ void ll_open_cleanup(struct super_block *sb, struct ptlrpc_request *open_req)
 	op_data->op_mod_time = get_seconds();
 	md_close(exp, op_data, NULL, &close_req);
 	ptlrpc_req_finished(close_req);
-	ll_finish_md_op_data(op_data);
+	kfree(op_data);
 }
 
 int ll_prep_inode(struct inode **inode, struct ptlrpc_request *req,
@@ -2170,11 +2170,6 @@ struct md_op_data *ll_prep_md_op_data(struct md_op_data *op_data,
 	return op_data;
 }
 
-void ll_finish_md_op_data(struct md_op_data *op_data)
-{
-	kfree(op_data);
-}
-
 int ll_show_options(struct seq_file *seq, struct dentry *dentry)
 {
 	struct ll_sb_info *sbi;
-- 
2.1.0

[PATCH 9/9] Staging: lustre: llite_internal: Remove function prototype

[Shivani Bhardwaj]

Remove the prototype of the function ll_finish_md_op_data() as it is no
longer needed.

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/lustre/lustre/llite/llite_internal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 157c3284..d12d483 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -792,7 +792,6 @@ struct md_op_data *ll_prep_md_op_data(struct md_op_data *op_data,
 				      struct inode *i1, struct inode *i2,
 				      const char *name, int namelen,
 				      int mode, __u32 opc, void *data);
-void ll_finish_md_op_data(struct md_op_data *op_data);
 int ll_get_obd_name(struct inode *inode, unsigned int cmd, unsigned long arg);
 char *ll_get_fsname(struct super_block *sb, char *buf, int buflen);
 void ll_open_cleanup(struct super_block *sb, struct ptlrpc_request *open_req);
-- 
2.1.0

Re: [PATCH 9/9] Staging: lustre: llite_internal: Remove function prototype

[Dan Carpenter]

This one should have been folded in with the last one.  Really the whole
series could have been sent as one patch, but especially the last two
should have been folded together.

regards,
dan carpenter

Re: [PATCH 9/9] Staging: lustre: llite_internal: Remove function prototype

[Shivani Bhardwaj]

On Sat, Nov 7, 2015 at 4:15 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> This one should have been folded in with the last one.  Really the whole
> series could have been sent as one patch, but especially the last two
> should have been folded together.
>
> regards,
> dan carpenter
>

Thanks Dan! Could you please suggest whether I should be resending the
complete series?
I'll take care for future patches.

Thank you
Shivani

Re: [PATCH 9/9] Staging: lustre: llite_internal: Remove function prototype

[Dan Carpenter]

Don't resend, it's fine.  I don't like re-reviewing patches and you
don't like sending them.  It's not like it introduces a bug or generate
a warning with our current tools.

But it's still important to understand how the one thing per patch rule
works (don't do half a thing per patch) so I wanted you to be aware of
it for future patches as you said.

regards,
dan carpenter

Re: [PATCH 9/9] Staging: lustre: llite_internal: Remove function prototype

[Shivani Bhardwaj]

On Sun, Nov 8, 2015 at 2:17 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Don't resend, it's fine.  I don't like re-reviewing patches and you
> don't like sending them.  It's not like it introduces a bug or generate
> a warning with our current tools.
>
> But it's still important to understand how the one thing per patch rule
> works (don't do half a thing per patch) so I wanted you to be aware of
> it for future patches as you said.
>

I'll take care next time.

Thank you
Shivani

> regards,
> dan carpenter
>

Re: [PATCH 1/9] Staging: lustre: dir: Replace function calls

[Drokin, Oleg]

On Nov 7, 2015, at 2:41 AM, Shivani Bhardwaj wrote:

> Replace the calls of the function ll_finish_md_op_data() with the
> standard function kfree().

For functions like this that have meaningflul name and also
might include additional logic (even though they don't now),
does it make sense to do this?

I don't feel this is a case of "lustre_specific_kfree_wrapper()"
elimination.
Same for some other wrappers being eliminated, those looked
at times as a way to improve code readability to me.

> 
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
> drivers/staging/lustre/lustre/llite/dir.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c
> index 5c2ef92..ba3f469 100644
> --- a/drivers/staging/lustre/lustre/llite/dir.c
> +++ b/drivers/staging/lustre/lustre/llite/dir.c
> @@ -182,7 +182,7 @@ static int ll_dir_filler(void *_hash, struct page *page0)
> 	op_data->op_npages = npages;
> 	op_data->op_offset = hash;
> 	rc = md_readpage(exp, op_data, page_pool, &request);
> -	ll_finish_md_op_data(op_data);
> +	kfree(op_data);
> 	if (rc < 0) {
> 		/* page0 is special, which was added into page cache early */
> 		delete_from_page_cache(page0);
> @@ -363,7 +363,7 @@ struct page *ll_get_dir_page(struct inode *dir, __u64 hash,
> 		rc = md_enqueue(ll_i2sbi(dir)->ll_md_exp, &einfo, &it,
> 				op_data, &lockh, NULL, 0, NULL, 0);
> 
> -		ll_finish_md_op_data(op_data);
> +		kfree(op_data);
> 
> 		request = (struct ptlrpc_request *)it.d.lustre.it_data;
> 		if (request)
> @@ -669,7 +669,7 @@ static int ll_dir_setdirstripe(struct inode *dir, struct lmv_user_md *lump,
> 			from_kuid(&init_user_ns, current_fsuid()),
> 			from_kgid(&init_user_ns, current_fsgid()),
> 			cfs_curproc_cap_pack(), 0, &request);
> -	ll_finish_md_op_data(op_data);
> +	kfree(op_data);
> 	if (err)
> 		goto err_exit;
> err_exit:
> @@ -730,7 +730,7 @@ int ll_dir_setstripe(struct inode *inode, struct lov_user_md *lump,
> 	/* swabbing is done in lov_setstripe() on server side */
> 	rc = md_setattr(sbi->ll_md_exp, op_data, lump, lum_size,
> 			NULL, 0, &req, NULL);
> -	ll_finish_md_op_data(op_data);
> +	kfree(op_data);
> 	ptlrpc_req_finished(req);
> 	if (rc) {
> 		if (rc != -EPERM && rc != -EACCES)
> @@ -802,7 +802,7 @@ int ll_dir_getstripe(struct inode *inode, struct lov_mds_md **lmmp,
> 
> 	op_data->op_valid = OBD_MD_FLEASIZE | OBD_MD_FLDIREA;
> 	rc = md_getattr(sbi->ll_md_exp, op_data, &req);
> -	ll_finish_md_op_data(op_data);
> +	kfree(op_data);
> 	if (rc < 0) {
> 		CDEBUG(D_INFO, "md_getattr failed on inode %lu/%u: rc %d\n",
> 		       inode->i_ino,
> @@ -868,7 +868,7 @@ int ll_get_mdt_idx(struct inode *inode)
> 	op_data->op_flags |= MF_GET_MDT_IDX;
> 	rc = md_getattr(sbi->ll_md_exp, op_data, NULL);
> 	mdtidx = op_data->op_mds;
> -	ll_finish_md_op_data(op_data);
> +	kfree(op_data);
> 	if (rc < 0) {
> 		CDEBUG(D_INFO, "md_getattr_name: %d\n", rc);
> 		return rc;
> @@ -1301,7 +1301,7 @@ static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> 
> 		op_data->op_valid = OBD_MD_FLID;
> 		rc = md_getattr_name(sbi->ll_md_exp, op_data, &request);
> -		ll_finish_md_op_data(op_data);
> +		kfree(op_data);
> 		if (rc < 0) {
> 			CDEBUG(D_INFO, "md_getattr_name: %d\n", rc);
> 			goto out_free;
> -- 
> 2.1.0
> 

Re: [PATCH 1/9] Staging: lustre: dir: Replace function calls

[Dan Carpenter]

If we ever need the wrapper, we can add it again.  These wrappers only
hurt readability because they trick you into thinking there is some
reference counting or something but it's just an ordinary kfree().

regards,
dan carpenter

Re: [PATCH 8/9] Staging: lustre: llite_lib: Remove wrapper function

[Michał Kępień]

> Remove the function ll_finish_md_op_data() and replace all its calls
> with the standrd function ll_finish_md_op_data().

I believe you meant to write "standard function kfree()".

-- 
Best regards,
Michał Kępień

Re: [PATCH 8/9] Staging: lustre: llite_lib: Remove wrapper function

[Shivani Bhardwaj]

On Mon, Nov 9, 2015 at 7:07 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> Remove the function ll_finish_md_op_data() and replace all its calls
>> with the standrd function ll_finish_md_op_data().
>
> I believe you meant to write "standard function kfree()".
>

Yes. I am so sorry. Should I be sending the whole series again?
Thank you
Shivani

> --
> Best regards,
> Michał Kępień

RE: [PATCH 8/9] Staging: lustre: llite_lib: Remove wrapper function

[Simmons, James A.]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 728 bytes --]

>On Mon, Nov 9, 2015 at 7:07 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>>> Remove the function ll_finish_md_op_data() and replace all its calls
>>> with the standrd function ll_finish_md_op_data().
>>
>> I believe you meant to write "standard function kfree()".
>>
>
>Yes. I am so sorry. Should I be sending the whole series again?
>Thank you
>Shivani

Yes please redo the series. I saw Oleg's concern and I would recommend that
besides the conversion to kfree that you add comments about what is being deleted.
I.E

/* Free struct md_op_data  data*/
kfree(...)
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 8/9] Staging: lustre: llite_lib: Remove wrapper function

[Shivani Bhardwaj]

On Tue, Nov 10, 2015 at 12:48 AM, Simmons, James A. <simmonsja@ornl.gov> wrote:
>>On Mon, Nov 9, 2015 at 7:07 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>>>> Remove the function ll_finish_md_op_data() and replace all its calls
>>>> with the standrd function ll_finish_md_op_data().
>>>
>>> I believe you meant to write "standard function kfree()".
>>>
>>
>>Yes. I am so sorry. Should I be sending the whole series again?
>>Thank you
>>Shivani
>
> Yes please redo the series. I saw Oleg's concern and I would recommend that
> besides the conversion to kfree that you add comments about what is being deleted.
> I.E
>
> /* Free struct md_op_data  data*/
> kfree(...)

I'll do that.
Thank you