All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Oleg Drokin <oleg.drokin@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	James Simmons <jsimmons@infradead.org>,
	Andreas Dilger <andreas.dilger@intel.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Lustre Development List <lustre-devel@lists.lustre.org>
Subject: [PATCH 06/10] staging: lustre: llite: use more private data in dump_pgcache
Date: Tue, 01 May 2018 13:52:39 +1000	[thread overview]
Message-ID: <152514675904.17843.14765347572362739654.stgit@noble> (raw)
In-Reply-To: <152514658325.17843.11455067361317157487.stgit@noble>

The dump_page_cache debugfs file allocates and frees an 'env' in each
call to vvp_pgcache_start,next,show.  This is likely to be fast, but
does introduce the need to check for errors.

It is reasonable to allocate a single 'env' when the file is opened,
and use that throughout.

So create 'seq_private' structure which stores the sbi, env, and
refcheck, and attach this to the seqfile.

Then use it throughout instead of allocating 'env' repeatedly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/vvp_dev.c |  150 ++++++++++++-------------
 1 file changed, 72 insertions(+), 78 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c b/drivers/staging/lustre/lustre/llite/vvp_dev.c
index 987c03b058e6..a2619dc04a7f 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_dev.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_dev.c
@@ -390,6 +390,12 @@ struct vvp_pgcache_id {
 	struct lu_object_header *vpi_obj;
 };
 
+struct seq_private {
+	struct ll_sb_info	*sbi;
+	struct lu_env		*env;
+	u16			refcheck;
+};
+
 static void vvp_pgcache_id_unpack(loff_t pos, struct vvp_pgcache_id *id)
 {
 	BUILD_BUG_ON(sizeof(pos) != sizeof(__u64));
@@ -531,95 +537,71 @@ static void vvp_pgcache_page_show(const struct lu_env *env,
 
 static int vvp_pgcache_show(struct seq_file *f, void *v)
 {
+	struct seq_private	*priv = f->private;
 	loff_t		   pos;
-	struct ll_sb_info       *sbi;
 	struct cl_object	*clob;
-	struct lu_env	   *env;
 	struct vvp_pgcache_id    id;
-	u16 refcheck;
-	int		      result;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		pos = *(loff_t *)v;
-		vvp_pgcache_id_unpack(pos, &id);
-		sbi = f->private;
-		clob = vvp_pgcache_obj(env, &sbi->ll_cl->cd_lu_dev, &id);
-		if (clob) {
-			struct inode *inode = vvp_object_inode(clob);
-			struct cl_page *page = NULL;
-			struct page *vmpage;
-
-			result = find_get_pages_contig(inode->i_mapping,
-						       id.vpi_index, 1,
-						       &vmpage);
-			if (result > 0) {
-				lock_page(vmpage);
-				page = cl_vmpage_page(vmpage, clob);
-				unlock_page(vmpage);
-				put_page(vmpage);
-			}
+	pos = *(loff_t *)v;
+	vvp_pgcache_id_unpack(pos, &id);
+	clob = vvp_pgcache_obj(priv->env, &priv->sbi->ll_cl->cd_lu_dev, &id);
+	if (clob) {
+		struct inode *inode = vvp_object_inode(clob);
+		struct cl_page *page = NULL;
+		struct page *vmpage;
+		int result;
+
+		result = find_get_pages_contig(inode->i_mapping,
+					       id.vpi_index, 1,
+					       &vmpage);
+		if (result > 0) {
+			lock_page(vmpage);
+			page = cl_vmpage_page(vmpage, clob);
+			unlock_page(vmpage);
+			put_page(vmpage);
+		}
 
-			seq_printf(f, "%8x@" DFID ": ", id.vpi_index,
-				   PFID(lu_object_fid(&clob->co_lu)));
-			if (page) {
-				vvp_pgcache_page_show(env, f, page);
-				cl_page_put(env, page);
-			} else {
-				seq_puts(f, "missing\n");
-			}
-			lu_object_ref_del(&clob->co_lu, "dump", current);
-			cl_object_put(env, clob);
+		seq_printf(f, "%8x@" DFID ": ", id.vpi_index,
+			   PFID(lu_object_fid(&clob->co_lu)));
+		if (page) {
+			vvp_pgcache_page_show(priv->env, f, page);
+			cl_page_put(priv->env, page);
 		} else {
-			seq_printf(f, "%llx missing\n", pos);
+			seq_puts(f, "missing\n");
 		}
-		cl_env_put(env, &refcheck);
-		result = 0;
+		lu_object_ref_del(&clob->co_lu, "dump", current);
+		cl_object_put(priv->env, clob);
 	} else {
-		result = PTR_ERR(env);
+		seq_printf(f, "%llx missing\n", pos);
 	}
-	return result;
+	return 0;
 }
 
 static void *vvp_pgcache_start(struct seq_file *f, loff_t *pos)
 {
-	struct ll_sb_info *sbi;
-	struct lu_env     *env;
-	u16 refcheck;
-
-	sbi = f->private;
+	struct seq_private	*priv = f->private;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		sbi = f->private;
-		if (sbi->ll_site->ls_obj_hash->hs_cur_bits >
-		    64 - PGC_OBJ_SHIFT) {
-			pos = ERR_PTR(-EFBIG);
-		} else {
-			*pos = vvp_pgcache_find(env, &sbi->ll_cl->cd_lu_dev,
-						*pos);
-			if (*pos == ~0ULL)
-				pos = NULL;
-		}
-		cl_env_put(env, &refcheck);
+	if (priv->sbi->ll_site->ls_obj_hash->hs_cur_bits >
+	    64 - PGC_OBJ_SHIFT) {
+		pos = ERR_PTR(-EFBIG);
+	} else {
+		*pos = vvp_pgcache_find(priv->env, &priv->sbi->ll_cl->cd_lu_dev,
+					*pos);
+		if (*pos == ~0ULL)
+			pos = NULL;
 	}
+
 	return pos;
 }
 
 static void *vvp_pgcache_next(struct seq_file *f, void *v, loff_t *pos)
 {
-	struct ll_sb_info *sbi;
-	struct lu_env     *env;
-	u16 refcheck;
+	struct seq_private *priv = f->private;
+
+	*pos = vvp_pgcache_find(priv->env, &priv->sbi->ll_cl->cd_lu_dev, *pos + 1);
+	if (*pos == ~0ULL)
+		pos = NULL;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		sbi = f->private;
-		*pos = vvp_pgcache_find(env, &sbi->ll_cl->cd_lu_dev, *pos + 1);
-		if (*pos == ~0ULL)
-			pos = NULL;
-		cl_env_put(env, &refcheck);
-	}
 	return pos;
 }
 
@@ -637,17 +619,29 @@ static const struct seq_operations vvp_pgcache_ops = {
 
 static int vvp_dump_pgcache_seq_open(struct inode *inode, struct file *filp)
 {
-	struct seq_file *seq;
-	int rc;
-
-	rc = seq_open(filp, &vvp_pgcache_ops);
-	if (rc)
-		return rc;
+	struct seq_private *priv;
+
+	priv = __seq_open_private(filp, &vvp_pgcache_ops, sizeof(*priv));
+	if (!priv)
+		return -ENOMEM;
+
+	priv->sbi = inode->i_private;
+	priv->env = cl_env_get(&priv->refcheck);
+	if (IS_ERR(priv->env)) {
+		int err = PTR_ERR(priv->env);
+		seq_release_private(inode, filp);
+		return err;
+	}
+	return 0;
+}
 
-	seq = filp->private_data;
-	seq->private = inode->i_private;
+static int vvp_dump_pgcache_seq_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *seq = file->private_data;
+	struct seq_private *priv = seq->private;
 
-	return 0;
+	cl_env_put(priv->env, &priv->refcheck);
+	return seq_release_private(inode, file);
 }
 
 const struct file_operations vvp_dump_pgcache_file_ops = {
@@ -655,5 +649,5 @@ const struct file_operations vvp_dump_pgcache_file_ops = {
 	.open    = vvp_dump_pgcache_seq_open,
 	.read    = seq_read,
 	.llseek	 = seq_lseek,
-	.release = seq_release,
+	.release = vvp_dump_pgcache_seq_release,
 };

WARNING: multiple messages have this Message-ID (diff)
From: NeilBrown <neilb@suse.com>
To: Oleg Drokin <oleg.drokin@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	James Simmons <jsimmons@infradead.org>,
	Andreas Dilger <andreas.dilger@intel.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Lustre Development List <lustre-devel@lists.lustre.org>
Subject: [lustre-devel] [PATCH 06/10] staging: lustre: llite: use more private data in dump_pgcache
Date: Tue, 01 May 2018 13:52:39 +1000	[thread overview]
Message-ID: <152514675904.17843.14765347572362739654.stgit@noble> (raw)
In-Reply-To: <152514658325.17843.11455067361317157487.stgit@noble>

The dump_page_cache debugfs file allocates and frees an 'env' in each
call to vvp_pgcache_start,next,show.  This is likely to be fast, but
does introduce the need to check for errors.

It is reasonable to allocate a single 'env' when the file is opened,
and use that throughout.

So create 'seq_private' structure which stores the sbi, env, and
refcheck, and attach this to the seqfile.

Then use it throughout instead of allocating 'env' repeatedly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/vvp_dev.c |  150 ++++++++++++-------------
 1 file changed, 72 insertions(+), 78 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c b/drivers/staging/lustre/lustre/llite/vvp_dev.c
index 987c03b058e6..a2619dc04a7f 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_dev.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_dev.c
@@ -390,6 +390,12 @@ struct vvp_pgcache_id {
 	struct lu_object_header *vpi_obj;
 };
 
+struct seq_private {
+	struct ll_sb_info	*sbi;
+	struct lu_env		*env;
+	u16			refcheck;
+};
+
 static void vvp_pgcache_id_unpack(loff_t pos, struct vvp_pgcache_id *id)
 {
 	BUILD_BUG_ON(sizeof(pos) != sizeof(__u64));
@@ -531,95 +537,71 @@ static void vvp_pgcache_page_show(const struct lu_env *env,
 
 static int vvp_pgcache_show(struct seq_file *f, void *v)
 {
+	struct seq_private	*priv = f->private;
 	loff_t		   pos;
-	struct ll_sb_info       *sbi;
 	struct cl_object	*clob;
-	struct lu_env	   *env;
 	struct vvp_pgcache_id    id;
-	u16 refcheck;
-	int		      result;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		pos = *(loff_t *)v;
-		vvp_pgcache_id_unpack(pos, &id);
-		sbi = f->private;
-		clob = vvp_pgcache_obj(env, &sbi->ll_cl->cd_lu_dev, &id);
-		if (clob) {
-			struct inode *inode = vvp_object_inode(clob);
-			struct cl_page *page = NULL;
-			struct page *vmpage;
-
-			result = find_get_pages_contig(inode->i_mapping,
-						       id.vpi_index, 1,
-						       &vmpage);
-			if (result > 0) {
-				lock_page(vmpage);
-				page = cl_vmpage_page(vmpage, clob);
-				unlock_page(vmpage);
-				put_page(vmpage);
-			}
+	pos = *(loff_t *)v;
+	vvp_pgcache_id_unpack(pos, &id);
+	clob = vvp_pgcache_obj(priv->env, &priv->sbi->ll_cl->cd_lu_dev, &id);
+	if (clob) {
+		struct inode *inode = vvp_object_inode(clob);
+		struct cl_page *page = NULL;
+		struct page *vmpage;
+		int result;
+
+		result = find_get_pages_contig(inode->i_mapping,
+					       id.vpi_index, 1,
+					       &vmpage);
+		if (result > 0) {
+			lock_page(vmpage);
+			page = cl_vmpage_page(vmpage, clob);
+			unlock_page(vmpage);
+			put_page(vmpage);
+		}
 
-			seq_printf(f, "%8x@" DFID ": ", id.vpi_index,
-				   PFID(lu_object_fid(&clob->co_lu)));
-			if (page) {
-				vvp_pgcache_page_show(env, f, page);
-				cl_page_put(env, page);
-			} else {
-				seq_puts(f, "missing\n");
-			}
-			lu_object_ref_del(&clob->co_lu, "dump", current);
-			cl_object_put(env, clob);
+		seq_printf(f, "%8x@" DFID ": ", id.vpi_index,
+			   PFID(lu_object_fid(&clob->co_lu)));
+		if (page) {
+			vvp_pgcache_page_show(priv->env, f, page);
+			cl_page_put(priv->env, page);
 		} else {
-			seq_printf(f, "%llx missing\n", pos);
+			seq_puts(f, "missing\n");
 		}
-		cl_env_put(env, &refcheck);
-		result = 0;
+		lu_object_ref_del(&clob->co_lu, "dump", current);
+		cl_object_put(priv->env, clob);
 	} else {
-		result = PTR_ERR(env);
+		seq_printf(f, "%llx missing\n", pos);
 	}
-	return result;
+	return 0;
 }
 
 static void *vvp_pgcache_start(struct seq_file *f, loff_t *pos)
 {
-	struct ll_sb_info *sbi;
-	struct lu_env     *env;
-	u16 refcheck;
-
-	sbi = f->private;
+	struct seq_private	*priv = f->private;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		sbi = f->private;
-		if (sbi->ll_site->ls_obj_hash->hs_cur_bits >
-		    64 - PGC_OBJ_SHIFT) {
-			pos = ERR_PTR(-EFBIG);
-		} else {
-			*pos = vvp_pgcache_find(env, &sbi->ll_cl->cd_lu_dev,
-						*pos);
-			if (*pos == ~0ULL)
-				pos = NULL;
-		}
-		cl_env_put(env, &refcheck);
+	if (priv->sbi->ll_site->ls_obj_hash->hs_cur_bits >
+	    64 - PGC_OBJ_SHIFT) {
+		pos = ERR_PTR(-EFBIG);
+	} else {
+		*pos = vvp_pgcache_find(priv->env, &priv->sbi->ll_cl->cd_lu_dev,
+					*pos);
+		if (*pos == ~0ULL)
+			pos = NULL;
 	}
+
 	return pos;
 }
 
 static void *vvp_pgcache_next(struct seq_file *f, void *v, loff_t *pos)
 {
-	struct ll_sb_info *sbi;
-	struct lu_env     *env;
-	u16 refcheck;
+	struct seq_private *priv = f->private;
+
+	*pos = vvp_pgcache_find(priv->env, &priv->sbi->ll_cl->cd_lu_dev, *pos + 1);
+	if (*pos == ~0ULL)
+		pos = NULL;
 
-	env = cl_env_get(&refcheck);
-	if (!IS_ERR(env)) {
-		sbi = f->private;
-		*pos = vvp_pgcache_find(env, &sbi->ll_cl->cd_lu_dev, *pos + 1);
-		if (*pos == ~0ULL)
-			pos = NULL;
-		cl_env_put(env, &refcheck);
-	}
 	return pos;
 }
 
@@ -637,17 +619,29 @@ static const struct seq_operations vvp_pgcache_ops = {
 
 static int vvp_dump_pgcache_seq_open(struct inode *inode, struct file *filp)
 {
-	struct seq_file *seq;
-	int rc;
-
-	rc = seq_open(filp, &vvp_pgcache_ops);
-	if (rc)
-		return rc;
+	struct seq_private *priv;
+
+	priv = __seq_open_private(filp, &vvp_pgcache_ops, sizeof(*priv));
+	if (!priv)
+		return -ENOMEM;
+
+	priv->sbi = inode->i_private;
+	priv->env = cl_env_get(&priv->refcheck);
+	if (IS_ERR(priv->env)) {
+		int err = PTR_ERR(priv->env);
+		seq_release_private(inode, filp);
+		return err;
+	}
+	return 0;
+}
 
-	seq = filp->private_data;
-	seq->private = inode->i_private;
+static int vvp_dump_pgcache_seq_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *seq = file->private_data;
+	struct seq_private *priv = seq->private;
 
-	return 0;
+	cl_env_put(priv->env, &priv->refcheck);
+	return seq_release_private(inode, file);
 }
 
 const struct file_operations vvp_dump_pgcache_file_ops = {
@@ -655,5 +649,5 @@ const struct file_operations vvp_dump_pgcache_file_ops = {
 	.open    = vvp_dump_pgcache_seq_open,
 	.read    = seq_read,
 	.llseek	 = seq_lseek,
-	.release = seq_release,
+	.release = vvp_dump_pgcache_seq_release,
 };

  parent reply	other threads:[~2018-05-01  3:52 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-01  3:52 [PATCH 00/10] staging: lustre: assorted improvements NeilBrown
2018-05-01  3:52 ` [lustre-devel] " NeilBrown
2018-05-01  3:52 ` [PATCH 02/10] staging: lustre: make struct lu_site_bkt_data private NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  4:10   ` Dilger, Andreas
2018-05-01  4:10     ` Dilger, Andreas
2018-05-02  3:02   ` James Simmons
2018-05-02  3:02     ` [lustre-devel] " James Simmons
2018-05-03 23:39     ` NeilBrown
2018-05-03 23:39       ` [lustre-devel] " NeilBrown
2018-05-07  1:42       ` Greg Kroah-Hartman
2018-05-07  1:42         ` [lustre-devel] " Greg Kroah-Hartman
2018-05-01  3:52 ` [PATCH 01/10] staging: lustre: ldlm: store name directly in namespace NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  4:04   ` Dilger, Andreas
2018-05-01  4:04     ` [lustre-devel] " Dilger, Andreas
2018-05-02 18:11   ` James Simmons
2018-05-02 18:11     ` [lustre-devel] " James Simmons
2018-05-01  3:52 ` [PATCH 03/10] staging: lustre: lu_object: discard extra lru count NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  4:19   ` Dilger, Andreas
2018-05-01  4:19     ` [lustre-devel] " Dilger, Andreas
2018-05-04  0:08     ` NeilBrown
2018-05-04  0:08       ` [lustre-devel] " NeilBrown
2018-05-01  3:52 ` [PATCH 07/10] staging: lustre: llite: remove redundant lookup in dump_pgcache NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  3:52 ` [PATCH 08/10] staging: lustre: move misc-device registration closer to related code NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-02 18:12   ` James Simmons
2018-05-02 18:12     ` [lustre-devel] " James Simmons
2018-05-01  3:52 ` [PATCH 04/10] staging: lustre: lu_object: move retry logic inside htable_lookup NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  8:22   ` Dilger, Andreas
2018-05-01  8:22     ` Dilger, Andreas
2018-05-02 18:21     ` James Simmons
2018-05-02 18:21       ` James Simmons
2018-05-04  0:30       ` NeilBrown
2018-05-04  0:30         ` NeilBrown
2018-05-04  1:30     ` NeilBrown
2018-05-04  1:30       ` NeilBrown
2018-05-01  3:52 ` [PATCH 05/10] staging: lustre: fold lu_object_new() into lu_object_find_at() NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-01  3:52 ` [PATCH 10/10] staging: lustre: fix error deref in ll_splice_alias() NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-02  3:05   ` James Simmons
2018-05-02  3:05     ` [lustre-devel] " James Simmons
2018-05-04  0:34     ` NeilBrown
2018-05-04  0:34       ` [lustre-devel] " NeilBrown
2018-05-01  3:52 ` NeilBrown [this message]
2018-05-01  3:52   ` [lustre-devel] [PATCH 06/10] staging: lustre: llite: use more private data in dump_pgcache NeilBrown
2018-05-01  3:52 ` [PATCH 09/10] staging: lustre: move remaining code from linux-module.c to module.c NeilBrown
2018-05-01  3:52   ` [lustre-devel] " NeilBrown
2018-05-02 18:13   ` James Simmons
2018-05-02 18:13     ` [lustre-devel] " James Simmons
2018-05-07  0:54 [PATCH 00/10 - v2] staging: lustre: assorted improvements NeilBrown
2018-05-07  0:54 ` [PATCH 06/10] staging: lustre: llite: use more private data in dump_pgcache NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=152514675904.17843.14765347572362739654.stgit@noble \
    --to=neilb@suse.com \
    --cc=andreas.dilger@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jsimmons@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.