All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] pnfs private workqueue, and two cleanups
@ 2011-09-10 17:41 Jim Rees
  2011-09-10 17:41 ` [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic Jim Rees
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Jim Rees @ 2011-09-10 17:41 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

These have been sent to the list previously, and I believe are
uncontroversial, but have not shown up on your pnfs-all-latest branch.

Peng Tao (3):
  SUNRPC/NFS: make rpc pipe upcall generic
  pNFS: introduce pnfs private workqueue
  pNFS: make _set_lo_fail generic

 fs/nfs/blocklayout/blocklayout.c    |   36 +++++++++-----------
 fs/nfs/blocklayout/blocklayout.h    |    2 -
 fs/nfs/blocklayout/blocklayoutdev.c |   22 ------------
 fs/nfs/idmap.c                      |   25 +-------------
 fs/nfs/nfs4filelayout.c             |   19 ++---------
 fs/nfs/objlayout/objio_osd.c        |    8 ++++
 fs/nfs/objlayout/objlayout.c        |    4 +-
 fs/nfs/pnfs.c                       |   64 ++++++++++++++++++++++++++++++++++-
 fs/nfs/pnfs.h                       |    5 +++
 include/linux/sunrpc/rpc_pipe_fs.h  |    2 +
 net/sunrpc/auth_gss/auth_gss.c      |   24 +------------
 net/sunrpc/rpc_pipe.c               |   20 +++++++++++
 12 files changed, 122 insertions(+), 109 deletions(-)

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic
  2011-09-10 17:41 [PATCH 0/3] pnfs private workqueue, and two cleanups Jim Rees
@ 2011-09-10 17:41 ` Jim Rees
  2011-09-10 17:41 ` [PATCH 2/3] pNFS: introduce pnfs private workqueue Jim Rees
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Jim Rees @ 2011-09-10 17:41 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

From: Peng Tao <bergwolf@gmail.com>

The same function is used by idmap, gss and blocklayout code. Make it
generic.

Signed-off-by: Peng Tao <peng_tao@emc.com>
Signed-off-by: Jim Rees <rees@umich.edu>
---
 fs/nfs/blocklayout/blocklayout.c    |    2 +-
 fs/nfs/blocklayout/blocklayout.h    |    2 --
 fs/nfs/blocklayout/blocklayoutdev.c |   22 ----------------------
 fs/nfs/idmap.c                      |   25 +------------------------
 include/linux/sunrpc/rpc_pipe_fs.h  |    2 ++
 net/sunrpc/auth_gss/auth_gss.c      |   24 ++----------------------
 net/sunrpc/rpc_pipe.c               |   20 ++++++++++++++++++++
 7 files changed, 26 insertions(+), 71 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index d2432f0..dc23833 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -964,7 +964,7 @@ static struct pnfs_layoutdriver_type blocklayout_type = {
 };
 
 static const struct rpc_pipe_ops bl_upcall_ops = {
-	.upcall		= bl_pipe_upcall,
+	.upcall		= rpc_pipe_generic_upcall,
 	.downcall	= bl_pipe_downcall,
 	.destroy_msg	= bl_pipe_destroy_msg,
 };
diff --git a/fs/nfs/blocklayout/blocklayout.h b/fs/nfs/blocklayout/blocklayout.h
index 58dc256..42acf7e 100644
--- a/fs/nfs/blocklayout/blocklayout.h
+++ b/fs/nfs/blocklayout/blocklayout.h
@@ -169,8 +169,6 @@ extern wait_queue_head_t bl_wq;
 #define BL_DEVICE_REQUEST_ERR          0x2 /* User level process fails */
 
 /* blocklayoutdev.c */
-ssize_t bl_pipe_upcall(struct file *, struct rpc_pipe_msg *,
-		       char __user *, size_t);
 ssize_t bl_pipe_downcall(struct file *, const char __user *, size_t);
 void bl_pipe_destroy_msg(struct rpc_pipe_msg *);
 struct block_device *nfs4_blkdev_get(dev_t dev);
diff --git a/fs/nfs/blocklayout/blocklayoutdev.c b/fs/nfs/blocklayout/blocklayoutdev.c
index 0b1fb0e..d08ba91 100644
--- a/fs/nfs/blocklayout/blocklayoutdev.c
+++ b/fs/nfs/blocklayout/blocklayoutdev.c
@@ -79,28 +79,6 @@ int nfs4_blkdev_put(struct block_device *bdev)
 	return blkdev_put(bdev, FMODE_READ);
 }
 
-/*
- * Shouldn't there be a rpc_generic_upcall() to do this for us?
- */
-ssize_t bl_pipe_upcall(struct file *filp, struct rpc_pipe_msg *msg,
-		       char __user *dst, size_t buflen)
-{
-	char *data = (char *)msg->data + msg->copied;
-	size_t mlen = min(msg->len - msg->copied, buflen);
-	unsigned long left;
-
-	left = copy_to_user(dst, data, mlen);
-	if (left == mlen) {
-		msg->errno = -EFAULT;
-		return -EFAULT;
-	}
-
-	mlen -= left;
-	msg->copied += mlen;
-	msg->errno = 0;
-	return mlen;
-}
-
 static struct bl_dev_msg bl_mount_reply;
 
 ssize_t bl_pipe_downcall(struct file *filp, const char __user *src,
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index f20801a..47d1c6f 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -336,8 +336,6 @@ struct idmap {
 	struct idmap_hashtable	idmap_group_hash;
 };
 
-static ssize_t idmap_pipe_upcall(struct file *, struct rpc_pipe_msg *,
-				 char __user *, size_t);
 static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
 				   size_t);
 static void idmap_pipe_destroy_msg(struct rpc_pipe_msg *);
@@ -345,7 +343,7 @@ static void idmap_pipe_destroy_msg(struct rpc_pipe_msg *);
 static unsigned int fnvhash32(const void *, size_t);
 
 static const struct rpc_pipe_ops idmap_upcall_ops = {
-	.upcall		= idmap_pipe_upcall,
+	.upcall		= rpc_pipe_generic_upcall,
 	.downcall	= idmap_pipe_downcall,
 	.destroy_msg	= idmap_pipe_destroy_msg,
 };
@@ -595,27 +593,6 @@ nfs_idmap_name(struct idmap *idmap, struct idmap_hashtable *h,
 	return ret;
 }
 
-/* RPC pipefs upcall/downcall routines */
-static ssize_t
-idmap_pipe_upcall(struct file *filp, struct rpc_pipe_msg *msg,
-		  char __user *dst, size_t buflen)
-{
-	char *data = (char *)msg->data + msg->copied;
-	size_t mlen = min(msg->len, buflen);
-	unsigned long left;
-
-	left = copy_to_user(dst, data, mlen);
-	if (left == mlen) {
-		msg->errno = -EFAULT;
-		return -EFAULT;
-	}
-
-	mlen -= left;
-	msg->copied += mlen;
-	msg->errno = 0;
-	return mlen;
-}
-
 static ssize_t
 idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 {
diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index 6f942c9..205305d 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -45,6 +45,8 @@ RPC_I(struct inode *inode)
 	return container_of(inode, struct rpc_inode, vfs_inode);
 }
 
+extern ssize_t rpc_pipe_generic_upcall(struct file *, struct rpc_pipe_msg *,
+				       char __user *, size_t);
 extern int rpc_queue_upcall(struct inode *, struct rpc_pipe_msg *);
 
 struct rpc_clnt;
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 364eb45..e9b7693 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -603,26 +603,6 @@ out:
 	return err;
 }
 
-static ssize_t
-gss_pipe_upcall(struct file *filp, struct rpc_pipe_msg *msg,
-		char __user *dst, size_t buflen)
-{
-	char *data = (char *)msg->data + msg->copied;
-	size_t mlen = min(msg->len, buflen);
-	unsigned long left;
-
-	left = copy_to_user(dst, data, mlen);
-	if (left == mlen) {
-		msg->errno = -EFAULT;
-		return -EFAULT;
-	}
-
-	mlen -= left;
-	msg->copied += mlen;
-	msg->errno = 0;
-	return mlen;
-}
-
 #define MSG_BUF_MAXSIZE 1024
 
 static ssize_t
@@ -1590,7 +1570,7 @@ static const struct rpc_credops gss_nullops = {
 };
 
 static const struct rpc_pipe_ops gss_upcall_ops_v0 = {
-	.upcall		= gss_pipe_upcall,
+	.upcall		= rpc_pipe_generic_upcall,
 	.downcall	= gss_pipe_downcall,
 	.destroy_msg	= gss_pipe_destroy_msg,
 	.open_pipe	= gss_pipe_open_v0,
@@ -1598,7 +1578,7 @@ static const struct rpc_pipe_ops gss_upcall_ops_v0 = {
 };
 
 static const struct rpc_pipe_ops gss_upcall_ops_v1 = {
-	.upcall		= gss_pipe_upcall,
+	.upcall		= rpc_pipe_generic_upcall,
 	.downcall	= gss_pipe_downcall,
 	.destroy_msg	= gss_pipe_destroy_msg,
 	.open_pipe	= gss_pipe_open_v1,
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index b181e34..67dbc18 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -77,6 +77,26 @@ rpc_timeout_upcall_queue(struct work_struct *work)
 	rpc_purge_list(rpci, &free_list, destroy_msg, -ETIMEDOUT);
 }
 
+ssize_t rpc_pipe_generic_upcall(struct file *filp, struct rpc_pipe_msg *msg,
+				char __user *dst, size_t buflen)
+{
+	char *data = (char *)msg->data + msg->copied;
+	size_t mlen = min(msg->len - msg->copied, buflen);
+	unsigned long left;
+
+	left = copy_to_user(dst, data, mlen);
+	if (left == mlen) {
+		msg->errno = -EFAULT;
+		return -EFAULT;
+	}
+
+	mlen -= left;
+	msg->copied += mlen;
+	msg->errno = 0;
+	return mlen;
+}
+EXPORT_SYMBOL_GPL(rpc_pipe_generic_upcall);
+
 /**
  * rpc_queue_upcall - queue an upcall message to userspace
  * @inode: inode of upcall pipe on which to queue given message
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 2/3] pNFS: introduce pnfs private workqueue
  2011-09-10 17:41 [PATCH 0/3] pnfs private workqueue, and two cleanups Jim Rees
  2011-09-10 17:41 ` [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic Jim Rees
@ 2011-09-10 17:41 ` Jim Rees
  2011-09-11 14:51   ` Benny Halevy
  2011-09-10 17:41 ` [PATCH 3/3] pNFS: make _set_lo_fail generic Jim Rees
  2011-09-11 16:01 ` [PATCH 0/3] pnfs private workqueue, and two cleanups Benny Halevy
  3 siblings, 1 reply; 13+ messages in thread
From: Jim Rees @ 2011-09-10 17:41 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

From: Peng Tao <bergwolf@gmail.com>

For layoutdriver io done functions, default workqueue is not a good place as
the code is executed in IO path. So add a pnfs private workqueue to handle
them.

Also change block and object layout code to make use of this private
workqueue.

Signed-off-by: Peng Tao <peng_tao@emc.com>
Signed-off-by: Jim Rees <rees@umich.edu>
---
 fs/nfs/blocklayout/blocklayout.c |   17 ++++++++---
 fs/nfs/objlayout/objio_osd.c     |    8 ++++++
 fs/nfs/objlayout/objlayout.c     |    4 +-
 fs/nfs/pnfs.c                    |   52 +++++++++++++++++++++++++++++++++++++-
 fs/nfs/pnfs.h                    |    4 +++
 5 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index dc23833..51f70f0 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -228,7 +228,7 @@ bl_end_par_io_read(void *data)
 	struct nfs_read_data *rdata = data;
 
 	INIT_WORK(&rdata->task.u.tk_work, bl_read_cleanup);
-	schedule_work(&rdata->task.u.tk_work);
+	pnfsiod_queue_work(&rdata->task.u.tk_work);
 }
 
 /* We don't want normal .rpc_call_done callback used, so we replace it
@@ -418,7 +418,7 @@ static void bl_end_par_io_write(void *data)
 	wdata->task.tk_status = 0;
 	wdata->verf.committed = NFS_FILE_SYNC;
 	INIT_WORK(&wdata->task.u.tk_work, bl_write_cleanup);
-	schedule_work(&wdata->task.u.tk_work);
+	pnfsiod_queue_work(&wdata->task.u.tk_work);
 }
 
 /* FIXME STUB - mark intersection of layout and page as bad, so is not
@@ -981,29 +981,35 @@ static int __init nfs4blocklayout_init(void)
 	if (ret)
 		goto out;
 
+	ret = pnfsiod_start();
+	if (ret)
+		goto out_remove;
+
 	init_waitqueue_head(&bl_wq);
 
 	mnt = rpc_get_mount();
 	if (IS_ERR(mnt)) {
 		ret = PTR_ERR(mnt);
-		goto out_remove;
+		goto out_stop;
 	}
 
 	ret = vfs_path_lookup(mnt->mnt_root,
 			      mnt,
 			      NFS_PIPE_DIRNAME, 0, &path);
 	if (ret)
-		goto out_remove;
+		goto out_stop;
 
 	bl_device_pipe = rpc_mkpipe(path.dentry, "blocklayout", NULL,
 				    &bl_upcall_ops, 0);
 	if (IS_ERR(bl_device_pipe)) {
 		ret = PTR_ERR(bl_device_pipe);
-		goto out_remove;
+		goto out_stop;
 	}
 out:
 	return ret;
 
+out_stop:
+	pnfsiod_stop();
 out_remove:
 	pnfs_unregister_layoutdriver(&blocklayout_type);
 	return ret;
@@ -1015,6 +1021,7 @@ static void __exit nfs4blocklayout_exit(void)
 	       __func__);
 
 	pnfs_unregister_layoutdriver(&blocklayout_type);
+	pnfsiod_stop();
 	rpc_unlink(bl_device_pipe);
 }
 
diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
index d0cda12..f28013f 100644
--- a/fs/nfs/objlayout/objio_osd.c
+++ b/fs/nfs/objlayout/objio_osd.c
@@ -1042,7 +1042,14 @@ static int __init
 objlayout_init(void)
 {
 	int ret = pnfs_register_layoutdriver(&objlayout_type);
+	if (ret)
+		goto out;
 
+	ret = pnfsiod_start();
+	if (ret)
+		pnfs_unregister_layoutdriver(&objlayout_type);
+
+out:
 	if (ret)
 		printk(KERN_INFO
 			"%s: Registering OSD pNFS Layout Driver failed: error=%d\n",
@@ -1057,6 +1064,7 @@ static void __exit
 objlayout_exit(void)
 {
 	pnfs_unregister_layoutdriver(&objlayout_type);
+	pnfsiod_stop();
 	printk(KERN_INFO "%s: Unregistered OSD pNFS Layout Driver\n",
 	       __func__);
 }
diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index 1d06f8e..f7c6c21 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -305,7 +305,7 @@ objlayout_read_done(struct objlayout_io_state *state, ssize_t status, bool sync)
 		pnfs_ld_read_done(rdata);
 	else {
 		INIT_WORK(&rdata->task.u.tk_work, _rpc_read_complete);
-		schedule_work(&rdata->task.u.tk_work);
+		pnfsiod_queue_work(&rdata->task.u.tk_work);
 	}
 }
 
@@ -396,7 +396,7 @@ objlayout_write_done(struct objlayout_io_state *state, ssize_t status,
 		pnfs_ld_write_done(wdata);
 	else {
 		INIT_WORK(&wdata->task.u.tk_work, _rpc_write_complete);
-		schedule_work(&wdata->task.u.tk_work);
+		pnfsiod_queue_work(&wdata->task.u.tk_work);
 	}
 }
 
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index e550e88..5ac7a78 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -38,7 +38,7 @@
 /* Locking:
  *
  * pnfs_spinlock:
- *      protects pnfs_modules_tbl.
+ *      protects pnfs_modules_tbl, pnfsiod_workqueue and pnfsiod_users.
  */
 static DEFINE_SPINLOCK(pnfs_spinlock);
 
@@ -47,6 +47,9 @@ static DEFINE_SPINLOCK(pnfs_spinlock);
  */
 static LIST_HEAD(pnfs_modules_tbl);
 
+static struct workqueue_struct *pnfsiod_workqueue;
+static int pnfsiod_users = 0;
+
 /* Return the registered pnfs layout driver module matching given id */
 static struct pnfs_layoutdriver_type *
 find_pnfs_driver_locked(u32 id)
@@ -1478,3 +1481,50 @@ out:
 	dprintk("<-- %s status %d\n", __func__, status);
 	return status;
 }
+
+/*
+ * start up the pnfsiod workqueue
+ */
+int pnfsiod_start(void)
+{
+	struct workqueue_struct *wq;
+	dprintk("RPC:       creating workqueue pnfsiod\n");
+	wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
+	if (wq == NULL)
+		return -ENOMEM;
+	spin_lock(&pnfs_spinlock);
+	pnfsiod_users++;
+	if (pnfsiod_workqueue == NULL) {
+		pnfsiod_workqueue = wq;
+	} else {
+		destroy_workqueue(wq);
+	}
+	spin_unlock(&pnfs_spinlock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pnfsiod_start);
+
+/*
+ * Destroy the pnfsiod workqueue
+ */
+void pnfsiod_stop(void)
+{
+	struct workqueue_struct *wq = NULL;
+
+	spin_lock(&pnfs_spinlock);
+	pnfsiod_users--;
+	if (pnfsiod_users == 0) {
+		wq = pnfsiod_workqueue;
+		pnfsiod_workqueue = NULL;
+	}
+	spin_unlock(&pnfs_spinlock);
+	if (wq)
+		destroy_workqueue(wq);
+}
+EXPORT_SYMBOL_GPL(pnfsiod_stop);
+
+void pnfsiod_queue_work(struct work_struct* work)
+{
+	queue_work(pnfsiod_workqueue, work);
+}
+EXPORT_SYMBOL_GPL(pnfsiod_queue_work);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 01cbfd5..bc1eed5 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -165,6 +165,10 @@ extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp);
 extern int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp);
 
 /* pnfs.c */
+int pnfsiod_start(void);
+void pnfsiod_stop(void);
+void pnfsiod_queue_work(struct work_struct* work);
+
 void get_layout_hdr(struct pnfs_layout_hdr *lo);
 void put_lseg(struct pnfs_layout_segment *lseg);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 3/3] pNFS: make _set_lo_fail generic
  2011-09-10 17:41 [PATCH 0/3] pnfs private workqueue, and two cleanups Jim Rees
  2011-09-10 17:41 ` [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic Jim Rees
  2011-09-10 17:41 ` [PATCH 2/3] pNFS: introduce pnfs private workqueue Jim Rees
@ 2011-09-10 17:41 ` Jim Rees
  2011-09-11 16:01 ` [PATCH 0/3] pnfs private workqueue, and two cleanups Benny Halevy
  3 siblings, 0 replies; 13+ messages in thread
From: Jim Rees @ 2011-09-10 17:41 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

From: Peng Tao <bergwolf@gmail.com>

file layout and block layout both use it to set mark layout io failure
bit. So make it generic.

Signed-off-by: Peng Tao <peng_tao@emc.com>
Signed-off-by: Jim Rees <rees@umich.edu>
---
 fs/nfs/blocklayout/blocklayout.c |   17 +++--------------
 fs/nfs/nfs4filelayout.c          |   19 +++----------------
 fs/nfs/pnfs.c                    |   12 ++++++++++++
 fs/nfs/pnfs.h                    |    1 +
 4 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 51f70f0..2c2dca9 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -176,17 +176,6 @@ retry:
 	return bio;
 }
 
-static void bl_set_lo_fail(struct pnfs_layout_segment *lseg)
-{
-	if (lseg->pls_range.iomode == IOMODE_RW) {
-		dprintk("%s Setting layout IOMODE_RW fail bit\n", __func__);
-		set_bit(lo_fail_bit(IOMODE_RW), &lseg->pls_layout->plh_flags);
-	} else {
-		dprintk("%s Setting layout IOMODE_READ fail bit\n", __func__);
-		set_bit(lo_fail_bit(IOMODE_READ), &lseg->pls_layout->plh_flags);
-	}
-}
-
 /* This is basically copied from mpage_end_io_read */
 static void bl_end_io_read(struct bio *bio, int err)
 {
@@ -206,7 +195,7 @@ static void bl_end_io_read(struct bio *bio, int err)
 	if (!uptodate) {
 		if (!rdata->pnfs_error)
 			rdata->pnfs_error = -EIO;
-		bl_set_lo_fail(rdata->lseg);
+		pnfs_set_lo_fail(rdata->lseg);
 	}
 	bio_put(bio);
 	put_parallel(par);
@@ -370,7 +359,7 @@ static void bl_end_io_write_zero(struct bio *bio, int err)
 	if (!uptodate) {
 		if (!wdata->pnfs_error)
 			wdata->pnfs_error = -EIO;
-		bl_set_lo_fail(wdata->lseg);
+		pnfs_set_lo_fail(wdata->lseg);
 	}
 	bio_put(bio);
 	put_parallel(par);
@@ -386,7 +375,7 @@ static void bl_end_io_write(struct bio *bio, int err)
 	if (!uptodate) {
 		if (!wdata->pnfs_error)
 			wdata->pnfs_error = -EIO;
-		bl_set_lo_fail(wdata->lseg);
+		pnfs_set_lo_fail(wdata->lseg);
 	}
 	bio_put(bio);
 	put_parallel(par);
diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index e8915d4..4c78c62 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -77,19 +77,6 @@ filelayout_get_dserver_offset(struct pnfs_layout_segment *lseg, loff_t offset)
 	BUG();
 }
 
-/* For data server errors we don't recover from */
-static void
-filelayout_set_lo_fail(struct pnfs_layout_segment *lseg)
-{
-	if (lseg->pls_range.iomode == IOMODE_RW) {
-		dprintk("%s Setting layout IOMODE_RW fail bit\n", __func__);
-		set_bit(lo_fail_bit(IOMODE_RW), &lseg->pls_layout->plh_flags);
-	} else {
-		dprintk("%s Setting layout IOMODE_READ fail bit\n", __func__);
-		set_bit(lo_fail_bit(IOMODE_READ), &lseg->pls_layout->plh_flags);
-	}
-}
-
 static int filelayout_async_handle_error(struct rpc_task *task,
 					 struct nfs4_state *state,
 					 struct nfs_client *clp,
@@ -145,7 +132,7 @@ static int filelayout_read_done_cb(struct rpc_task *task,
 		dprintk("%s calling restart ds_clp %p ds_clp->cl_session %p\n",
 			__func__, data->ds_clp, data->ds_clp->cl_session);
 		if (reset) {
-			filelayout_set_lo_fail(data->lseg);
+			pnfs_set_lo_fail(data->lseg);
 			nfs4_reset_read(task, data);
 			clp = NFS_SERVER(data->inode)->nfs_client;
 		}
@@ -221,7 +208,7 @@ static int filelayout_write_done_cb(struct rpc_task *task,
 		dprintk("%s calling restart ds_clp %p ds_clp->cl_session %p\n",
 			__func__, data->ds_clp, data->ds_clp->cl_session);
 		if (reset) {
-			filelayout_set_lo_fail(data->lseg);
+			pnfs_set_lo_fail(data->lseg);
 			nfs4_reset_write(task, data);
 			clp = NFS_SERVER(data->inode)->nfs_client;
 		} else
@@ -256,7 +243,7 @@ static int filelayout_commit_done_cb(struct rpc_task *task,
 			__func__, data->ds_clp, data->ds_clp->cl_session);
 		if (reset) {
 			prepare_to_resend_writes(data);
-			filelayout_set_lo_fail(data->lseg);
+			pnfs_set_lo_fail(data->lseg);
 		} else
 			nfs_restart_rpc(task, data->ds_clp);
 		return -EAGAIN;
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 5ac7a78..c02be6c 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1384,6 +1384,18 @@ static void pnfs_list_write_lseg(struct inode *inode, struct list_head *listp)
 	}
 }
 
+void pnfs_set_lo_fail(struct pnfs_layout_segment *lseg)
+{
+	if (lseg->pls_range.iomode == IOMODE_RW) {
+		dprintk("%s Setting layout IOMODE_RW fail bit\n", __func__);
+		set_bit(lo_fail_bit(IOMODE_RW), &lseg->pls_layout->plh_flags);
+	} else {
+		dprintk("%s Setting layout IOMODE_READ fail bit\n", __func__);
+		set_bit(lo_fail_bit(IOMODE_READ), &lseg->pls_layout->plh_flags);
+	}
+}
+EXPORT_SYMBOL_GPL(pnfs_set_lo_fail);
+
 void
 pnfs_set_layoutcommit(struct nfs_write_data *wdata)
 {
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index bc1eed5..5c83ce3 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -182,6 +182,7 @@ int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
 void pnfs_generic_pg_init_write(struct nfs_pageio_descriptor *, struct nfs_page *);
 int pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc);
 bool pnfs_generic_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev, struct nfs_page *req);
+void pnfs_set_lo_fail(struct pnfs_layout_segment *lseg);
 int pnfs_layout_process(struct nfs4_layoutget *lgp);
 void pnfs_free_lseg_list(struct list_head *tmp_list);
 void pnfs_destroy_layout(struct nfs_inode *);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] pNFS: introduce pnfs private workqueue
  2011-09-10 17:41 ` [PATCH 2/3] pNFS: introduce pnfs private workqueue Jim Rees
@ 2011-09-11 14:51   ` Benny Halevy
  2011-09-11 15:15     ` Benny Halevy
  0 siblings, 1 reply; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 14:51 UTC (permalink / raw)
  To: Jim Rees, Peng Tao; +Cc: linux-nfs, peter honeyman

On 2011-09-10 10:41, Jim Rees wrote:
> From: Peng Tao <bergwolf@gmail.com>

Hi, I have a few comments inline below.
Otherwise, the direction and the patch looks good.

> 
> For layoutdriver io done functions, default workqueue is not a good place as
> the code is executed in IO path. So add a pnfs private workqueue to handle
> them.
> 
> Also change block and object layout code to make use of this private
> workqueue.
> 
> Signed-off-by: Peng Tao <peng_tao@emc.com>
> Signed-off-by: Jim Rees <rees@umich.edu>
> ---
>  fs/nfs/blocklayout/blocklayout.c |   17 ++++++++---
>  fs/nfs/objlayout/objio_osd.c     |    8 ++++++
>  fs/nfs/objlayout/objlayout.c     |    4 +-
>  fs/nfs/pnfs.c                    |   52 +++++++++++++++++++++++++++++++++++++-
>  fs/nfs/pnfs.h                    |    4 +++
>  5 files changed, 77 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
> index dc23833..51f70f0 100644
> --- a/fs/nfs/blocklayout/blocklayout.c
> +++ b/fs/nfs/blocklayout/blocklayout.c
> @@ -228,7 +228,7 @@ bl_end_par_io_read(void *data)
>  	struct nfs_read_data *rdata = data;
>  
>  	INIT_WORK(&rdata->task.u.tk_work, bl_read_cleanup);
> -	schedule_work(&rdata->task.u.tk_work);
> +	pnfsiod_queue_work(&rdata->task.u.tk_work);
>  }
>  
>  /* We don't want normal .rpc_call_done callback used, so we replace it
> @@ -418,7 +418,7 @@ static void bl_end_par_io_write(void *data)
>  	wdata->task.tk_status = 0;
>  	wdata->verf.committed = NFS_FILE_SYNC;
>  	INIT_WORK(&wdata->task.u.tk_work, bl_write_cleanup);
> -	schedule_work(&wdata->task.u.tk_work);
> +	pnfsiod_queue_work(&wdata->task.u.tk_work);
>  }
>  
>  /* FIXME STUB - mark intersection of layout and page as bad, so is not
> @@ -981,29 +981,35 @@ static int __init nfs4blocklayout_init(void)
>  	if (ret)
>  		goto out;
>  
> +	ret = pnfsiod_start();
> +	if (ret)
> +		goto out_remove;
> +
>  	init_waitqueue_head(&bl_wq);
>  
>  	mnt = rpc_get_mount();
>  	if (IS_ERR(mnt)) {
>  		ret = PTR_ERR(mnt);
> -		goto out_remove;
> +		goto out_stop;
>  	}
>  
>  	ret = vfs_path_lookup(mnt->mnt_root,
>  			      mnt,
>  			      NFS_PIPE_DIRNAME, 0, &path);
>  	if (ret)
> -		goto out_remove;
> +		goto out_stop;
>  
>  	bl_device_pipe = rpc_mkpipe(path.dentry, "blocklayout", NULL,
>  				    &bl_upcall_ops, 0);
>  	if (IS_ERR(bl_device_pipe)) {
>  		ret = PTR_ERR(bl_device_pipe);
> -		goto out_remove;
> +		goto out_stop;
>  	}
>  out:
>  	return ret;
>  
> +out_stop:
> +	pnfsiod_stop();
>  out_remove:
>  	pnfs_unregister_layoutdriver(&blocklayout_type);
>  	return ret;
> @@ -1015,6 +1021,7 @@ static void __exit nfs4blocklayout_exit(void)
>  	       __func__);
>  
>  	pnfs_unregister_layoutdriver(&blocklayout_type);
> +	pnfsiod_stop();
>  	rpc_unlink(bl_device_pipe);
>  }
>  
> diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
> index d0cda12..f28013f 100644
> --- a/fs/nfs/objlayout/objio_osd.c
> +++ b/fs/nfs/objlayout/objio_osd.c
> @@ -1042,7 +1042,14 @@ static int __init
>  objlayout_init(void)
>  {
>  	int ret = pnfs_register_layoutdriver(&objlayout_type);
> +	if (ret)
> +		goto out;
>  
> +	ret = pnfsiod_start();
> +	if (ret)
> +		pnfs_unregister_layoutdriver(&objlayout_type);
> +
> +out:
>  	if (ret)
>  		printk(KERN_INFO
>  			"%s: Registering OSD pNFS Layout Driver failed: error=%d\n",
> @@ -1057,6 +1064,7 @@ static void __exit
>  objlayout_exit(void)
>  {
>  	pnfs_unregister_layoutdriver(&objlayout_type);
> +	pnfsiod_stop();
>  	printk(KERN_INFO "%s: Unregistered OSD pNFS Layout Driver\n",
>  	       __func__);
>  }
> diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
> index 1d06f8e..f7c6c21 100644
> --- a/fs/nfs/objlayout/objlayout.c
> +++ b/fs/nfs/objlayout/objlayout.c
> @@ -305,7 +305,7 @@ objlayout_read_done(struct objlayout_io_state *state, ssize_t status, bool sync)
>  		pnfs_ld_read_done(rdata);
>  	else {
>  		INIT_WORK(&rdata->task.u.tk_work, _rpc_read_complete);
> -		schedule_work(&rdata->task.u.tk_work);
> +		pnfsiod_queue_work(&rdata->task.u.tk_work);
>  	}
>  }
>  
> @@ -396,7 +396,7 @@ objlayout_write_done(struct objlayout_io_state *state, ssize_t status,
>  		pnfs_ld_write_done(wdata);
>  	else {
>  		INIT_WORK(&wdata->task.u.tk_work, _rpc_write_complete);
> -		schedule_work(&wdata->task.u.tk_work);
> +		pnfsiod_queue_work(&wdata->task.u.tk_work);
>  	}
>  }
>  
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index e550e88..5ac7a78 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -38,7 +38,7 @@
>  /* Locking:
>   *
>   * pnfs_spinlock:
> - *      protects pnfs_modules_tbl.
> + *      protects pnfs_modules_tbl, pnfsiod_workqueue and pnfsiod_users.
>   */
>  static DEFINE_SPINLOCK(pnfs_spinlock);
>  
> @@ -47,6 +47,9 @@ static DEFINE_SPINLOCK(pnfs_spinlock);
>   */
>  static LIST_HEAD(pnfs_modules_tbl);
>  
> +static struct workqueue_struct *pnfsiod_workqueue;
> +static int pnfsiod_users = 0;

There's no need to initialize static variables to zero.

> +
>  /* Return the registered pnfs layout driver module matching given id */
>  static struct pnfs_layoutdriver_type *
>  find_pnfs_driver_locked(u32 id)
> @@ -1478,3 +1481,50 @@ out:
>  	dprintk("<-- %s status %d\n", __func__, status);
>  	return status;
>  }
> +
> +/*
> + * start up the pnfsiod workqueue
> + */
> +int pnfsiod_start(void)
> +{
> +	struct workqueue_struct *wq;
> +	dprintk("RPC:       creating workqueue pnfsiod\n");

hmm, s/RPC/NFS/ this is not the RPC module any more :)
looks like a cut'n'paste from rpciod_start...

> +	wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
> +	if (wq == NULL)
> +		return -ENOMEM;
> +	spin_lock(&pnfs_spinlock);
> +	pnfsiod_users++;
> +	if (pnfsiod_workqueue == NULL) {
> +		pnfsiod_workqueue = wq;
> +	} else {
> +		destroy_workqueue(wq);
> +	}

The curly braces in this statement are an overkill
(and deviation from CodingStyle)

> +	spin_unlock(&pnfs_spinlock);
> +	return 0;

although this way of accounting is ultimately simple
it's wasteful and since we're not really expecting any concurrent
calls to this function. I'd consider coding this it as follows:

	atomic_inc(&pnfsiod_users);
	if (pnfsiod_workqueue == NULL) {
		wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
		if (wq == NULL)
			return -ENOMEM;
		spin_lock(&pnfs_spinlock);
		if (pnfsiod_workqueue == NULL)
			pnfsiod_workqueue = wq;
		else
			destroy_workqueue(wq);
		spin_unlock(&pnfs_spinlock);
	}

	return 0;

> +}
> +EXPORT_SYMBOL_GPL(pnfsiod_start);
> +
> +/*
> + * Destroy the pnfsiod workqueue
> + */
> +void pnfsiod_stop(void)
> +{
> +	struct workqueue_struct *wq = NULL;
> +
> +	spin_lock(&pnfs_spinlock);
> +	pnfsiod_users--;
> +	if (pnfsiod_users == 0) {
> +		wq = pnfsiod_workqueue;
> +		pnfsiod_workqueue = NULL;
> +	}
> +	spin_unlock(&pnfs_spinlock);
> +	if (wq)
> +		destroy_workqueue(wq);

and continuing my proposal from above:

	if (atomic_dec_and_lock(&pnfsiod_users)) {
		wq = pnfsiod_workqueue;
		pnfsiod_workqueue = NULL;
		spin_unlock(&pnfs_spinlock);
	}
	if (wq)
		destroy_workqueue(wq);

Benny

> +}
> +EXPORT_SYMBOL_GPL(pnfsiod_stop);
> +
> +void pnfsiod_queue_work(struct work_struct* work)
> +{
> +	queue_work(pnfsiod_workqueue, work);
> +}
> +EXPORT_SYMBOL_GPL(pnfsiod_queue_work);
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 01cbfd5..bc1eed5 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -165,6 +165,10 @@ extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp);
>  extern int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp);
>  
>  /* pnfs.c */
> +int pnfsiod_start(void);
> +void pnfsiod_stop(void);
> +void pnfsiod_queue_work(struct work_struct* work);
> +
>  void get_layout_hdr(struct pnfs_layout_hdr *lo);
>  void put_lseg(struct pnfs_layout_segment *lseg);
>  

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] pNFS: introduce pnfs private workqueue
  2011-09-11 14:51   ` Benny Halevy
@ 2011-09-11 15:15     ` Benny Halevy
  2011-09-11 15:31       ` [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue Benny Halevy
  2011-09-11 15:31       ` [PATCH 2/2] SQUASHME: pnfs: do pnfsiod_start before registering layout drivers Benny Halevy
  0 siblings, 2 replies; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 15:15 UTC (permalink / raw)
  To: Jim Rees, Peng Tao; +Cc: linux-nfs, peter honeyman

On 2011-09-11 07:51, Benny Halevy wrote:
> On 2011-09-10 10:41, Jim Rees wrote:
>> From: Peng Tao <bergwolf@gmail.com>
> 
> Hi, I have a few comments inline below.
> Otherwise, the direction and the patch looks good.
> 
>>
>> For layoutdriver io done functions, default workqueue is not a good place as
>> the code is executed in IO path. So add a pnfs private workqueue to handle
>> them.
>>
>> Also change block and object layout code to make use of this private
>> workqueue.
>>
>> Signed-off-by: Peng Tao <peng_tao@emc.com>
>> Signed-off-by: Jim Rees <rees@umich.edu>
>> ---
>>  fs/nfs/blocklayout/blocklayout.c |   17 ++++++++---
>>  fs/nfs/objlayout/objio_osd.c     |    8 ++++++
>>  fs/nfs/objlayout/objlayout.c     |    4 +-
>>  fs/nfs/pnfs.c                    |   52 +++++++++++++++++++++++++++++++++++++-
>>  fs/nfs/pnfs.h                    |    4 +++
>>  5 files changed, 77 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
>> index dc23833..51f70f0 100644
>> --- a/fs/nfs/blocklayout/blocklayout.c
>> +++ b/fs/nfs/blocklayout/blocklayout.c
>> @@ -228,7 +228,7 @@ bl_end_par_io_read(void *data)
>>  	struct nfs_read_data *rdata = data;
>>  
>>  	INIT_WORK(&rdata->task.u.tk_work, bl_read_cleanup);
>> -	schedule_work(&rdata->task.u.tk_work);
>> +	pnfsiod_queue_work(&rdata->task.u.tk_work);
>>  }
>>  
>>  /* We don't want normal .rpc_call_done callback used, so we replace it
>> @@ -418,7 +418,7 @@ static void bl_end_par_io_write(void *data)
>>  	wdata->task.tk_status = 0;
>>  	wdata->verf.committed = NFS_FILE_SYNC;
>>  	INIT_WORK(&wdata->task.u.tk_work, bl_write_cleanup);
>> -	schedule_work(&wdata->task.u.tk_work);
>> +	pnfsiod_queue_work(&wdata->task.u.tk_work);
>>  }
>>  
>>  /* FIXME STUB - mark intersection of layout and page as bad, so is not
>> @@ -981,29 +981,35 @@ static int __init nfs4blocklayout_init(void)
>>  	if (ret)
>>  		goto out;
>>  
>> +	ret = pnfsiod_start();
>> +	if (ret)
>> +		goto out_remove;
>> +
>>  	init_waitqueue_head(&bl_wq);
>>  
>>  	mnt = rpc_get_mount();
>>  	if (IS_ERR(mnt)) {
>>  		ret = PTR_ERR(mnt);
>> -		goto out_remove;
>> +		goto out_stop;
>>  	}
>>  
>>  	ret = vfs_path_lookup(mnt->mnt_root,
>>  			      mnt,
>>  			      NFS_PIPE_DIRNAME, 0, &path);
>>  	if (ret)
>> -		goto out_remove;
>> +		goto out_stop;
>>  
>>  	bl_device_pipe = rpc_mkpipe(path.dentry, "blocklayout", NULL,
>>  				    &bl_upcall_ops, 0);
>>  	if (IS_ERR(bl_device_pipe)) {
>>  		ret = PTR_ERR(bl_device_pipe);
>> -		goto out_remove;
>> +		goto out_stop;
>>  	}
>>  out:
>>  	return ret;
>>  
>> +out_stop:
>> +	pnfsiod_stop();
>>  out_remove:
>>  	pnfs_unregister_layoutdriver(&blocklayout_type);
>>  	return ret;
>> @@ -1015,6 +1021,7 @@ static void __exit nfs4blocklayout_exit(void)
>>  	       __func__);
>>  
>>  	pnfs_unregister_layoutdriver(&blocklayout_type);
>> +	pnfsiod_stop();
>>  	rpc_unlink(bl_device_pipe);
>>  }
>>  
>> diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
>> index d0cda12..f28013f 100644
>> --- a/fs/nfs/objlayout/objio_osd.c
>> +++ b/fs/nfs/objlayout/objio_osd.c
>> @@ -1042,7 +1042,14 @@ static int __init
>>  objlayout_init(void)
>>  {
>>  	int ret = pnfs_register_layoutdriver(&objlayout_type);
>> +	if (ret)
>> +		goto out;
>>  
>> +	ret = pnfsiod_start();
>> +	if (ret)
>> +		pnfs_unregister_layoutdriver(&objlayout_type);
>> +
>> +out:

Also, it makes more sense to init the workqueue before registering the
layout driver since it's a prerequisite.

>>  	if (ret)
>>  		printk(KERN_INFO
>>  			"%s: Registering OSD pNFS Layout Driver failed: error=%d\n",
>> @@ -1057,6 +1064,7 @@ static void __exit
>>  objlayout_exit(void)
>>  {
>>  	pnfs_unregister_layoutdriver(&objlayout_type);
>> +	pnfsiod_stop();
>>  	printk(KERN_INFO "%s: Unregistered OSD pNFS Layout Driver\n",
>>  	       __func__);
>>  }
>> diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
>> index 1d06f8e..f7c6c21 100644
>> --- a/fs/nfs/objlayout/objlayout.c
>> +++ b/fs/nfs/objlayout/objlayout.c
>> @@ -305,7 +305,7 @@ objlayout_read_done(struct objlayout_io_state *state, ssize_t status, bool sync)
>>  		pnfs_ld_read_done(rdata);
>>  	else {
>>  		INIT_WORK(&rdata->task.u.tk_work, _rpc_read_complete);
>> -		schedule_work(&rdata->task.u.tk_work);
>> +		pnfsiod_queue_work(&rdata->task.u.tk_work);
>>  	}
>>  }
>>  
>> @@ -396,7 +396,7 @@ objlayout_write_done(struct objlayout_io_state *state, ssize_t status,
>>  		pnfs_ld_write_done(wdata);
>>  	else {
>>  		INIT_WORK(&wdata->task.u.tk_work, _rpc_write_complete);
>> -		schedule_work(&wdata->task.u.tk_work);
>> +		pnfsiod_queue_work(&wdata->task.u.tk_work);
>>  	}
>>  }
>>  
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index e550e88..5ac7a78 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -38,7 +38,7 @@
>>  /* Locking:
>>   *
>>   * pnfs_spinlock:
>> - *      protects pnfs_modules_tbl.
>> + *      protects pnfs_modules_tbl, pnfsiod_workqueue and pnfsiod_users.
>>   */
>>  static DEFINE_SPINLOCK(pnfs_spinlock);
>>  
>> @@ -47,6 +47,9 @@ static DEFINE_SPINLOCK(pnfs_spinlock);
>>   */
>>  static LIST_HEAD(pnfs_modules_tbl);
>>  
>> +static struct workqueue_struct *pnfsiod_workqueue;
>> +static int pnfsiod_users = 0;
> 
> There's no need to initialize static variables to zero.
> 
>> +
>>  /* Return the registered pnfs layout driver module matching given id */
>>  static struct pnfs_layoutdriver_type *
>>  find_pnfs_driver_locked(u32 id)
>> @@ -1478,3 +1481,50 @@ out:
>>  	dprintk("<-- %s status %d\n", __func__, status);
>>  	return status;
>>  }
>> +
>> +/*
>> + * start up the pnfsiod workqueue
>> + */
>> +int pnfsiod_start(void)
>> +{
>> +	struct workqueue_struct *wq;
>> +	dprintk("RPC:       creating workqueue pnfsiod\n");
> 
> hmm, s/RPC/NFS/ this is not the RPC module any more :)
> looks like a cut'n'paste from rpciod_start...
> 
>> +	wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
>> +	if (wq == NULL)
>> +		return -ENOMEM;
>> +	spin_lock(&pnfs_spinlock);
>> +	pnfsiod_users++;
>> +	if (pnfsiod_workqueue == NULL) {
>> +		pnfsiod_workqueue = wq;
>> +	} else {
>> +		destroy_workqueue(wq);
>> +	}
> 
> The curly braces in this statement are an overkill
> (and deviation from CodingStyle)
> 
>> +	spin_unlock(&pnfs_spinlock);
>> +	return 0;
> 
> although this way of accounting is ultimately simple
> it's wasteful and since we're not really expecting any concurrent
> calls to this function. I'd consider coding this it as follows:
> 
> 	atomic_inc(&pnfsiod_users);
> 	if (pnfsiod_workqueue == NULL) {
> 		wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
> 		if (wq == NULL)

sorry, we need to call nfsiod_stop here.
I'll send a SQUASHME patch with my proposed changes
to this one to make it clearer...

Benny

> 			return -ENOMEM;
> 		spin_lock(&pnfs_spinlock);
> 		if (pnfsiod_workqueue == NULL)
> 			pnfsiod_workqueue = wq;
> 		else
> 			destroy_workqueue(wq);
> 		spin_unlock(&pnfs_spinlock);
> 	}
> 
> 	return 0;
> 
>> +}
>> +EXPORT_SYMBOL_GPL(pnfsiod_start);
>> +
>> +/*
>> + * Destroy the pnfsiod workqueue
>> + */
>> +void pnfsiod_stop(void)
>> +{
>> +	struct workqueue_struct *wq = NULL;
>> +
>> +	spin_lock(&pnfs_spinlock);
>> +	pnfsiod_users--;
>> +	if (pnfsiod_users == 0) {
>> +		wq = pnfsiod_workqueue;
>> +		pnfsiod_workqueue = NULL;
>> +	}
>> +	spin_unlock(&pnfs_spinlock);
>> +	if (wq)
>> +		destroy_workqueue(wq);
> 
> and continuing my proposal from above:
> 
> 	if (atomic_dec_and_lock(&pnfsiod_users)) {
> 		wq = pnfsiod_workqueue;
> 		pnfsiod_workqueue = NULL;
> 		spin_unlock(&pnfs_spinlock);
> 	}
> 	if (wq)
> 		destroy_workqueue(wq);
> 
> Benny
> 
>> +}
>> +EXPORT_SYMBOL_GPL(pnfsiod_stop);
>> +
>> +void pnfsiod_queue_work(struct work_struct* work)
>> +{
>> +	queue_work(pnfsiod_workqueue, work);
>> +}
>> +EXPORT_SYMBOL_GPL(pnfsiod_queue_work);
>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>> index 01cbfd5..bc1eed5 100644
>> --- a/fs/nfs/pnfs.h
>> +++ b/fs/nfs/pnfs.h
>> @@ -165,6 +165,10 @@ extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp);
>>  extern int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp);
>>  
>>  /* pnfs.c */
>> +int pnfsiod_start(void);
>> +void pnfsiod_stop(void);
>> +void pnfsiod_queue_work(struct work_struct* work);
>> +
>>  void get_layout_hdr(struct pnfs_layout_hdr *lo);
>>  void put_lseg(struct pnfs_layout_segment *lseg);
>>  

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue
  2011-09-11 15:15     ` Benny Halevy
@ 2011-09-11 15:31       ` Benny Halevy
  2011-09-11 17:17         ` Jim Rees
  2011-09-11 15:31       ` [PATCH 2/2] SQUASHME: pnfs: do pnfsiod_start before registering layout drivers Benny Halevy
  1 sibling, 1 reply; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 15:31 UTC (permalink / raw)
  To: Jim Rees, Peng Tao; +Cc: linux-nfs, Benny Halevy

From: Benny Halevy <bhalevy@tonian.com>

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfs/pnfs.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 5ac7a78..0bed5fc 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -38,7 +38,7 @@
 /* Locking:
  *
  * pnfs_spinlock:
- *      protects pnfs_modules_tbl, pnfsiod_workqueue and pnfsiod_users.
+ *      protects pnfs_modules_tbl, pnfsiod_workqueue.
  */
 static DEFINE_SPINLOCK(pnfs_spinlock);
 
@@ -48,7 +48,7 @@ static DEFINE_SPINLOCK(pnfs_spinlock);
 static LIST_HEAD(pnfs_modules_tbl);
 
 static struct workqueue_struct *pnfsiod_workqueue;
-static int pnfsiod_users = 0;
+static atomic_t pnfsiod_users;
 
 /* Return the registered pnfs layout driver module matching given id */
 static struct pnfs_layoutdriver_type *
@@ -1488,18 +1488,22 @@ out:
 int pnfsiod_start(void)
 {
 	struct workqueue_struct *wq;
-	dprintk("RPC:       creating workqueue pnfsiod\n");
-	wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
-	if (wq == NULL)
-		return -ENOMEM;
-	spin_lock(&pnfs_spinlock);
-	pnfsiod_users++;
+	
+	dprintk("NFS:       creating workqueue pnfsiod\n");
+	atomic_inc(&pnfsiod_users);
 	if (pnfsiod_workqueue == NULL) {
-		pnfsiod_workqueue = wq;
-	} else {
-		destroy_workqueue(wq);
+		wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
+		if (wq == NULL) {
+			pnfsiod_stop();	
+			return -ENOMEM;
+		}
+		spin_lock(&pnfs_spinlock);
+		if (pnfsiod_workqueue == NULL)
+			pnfsiod_workqueue = wq;
+		else
+			destroy_workqueue(wq);
+		spin_unlock(&pnfs_spinlock);
 	}
-	spin_unlock(&pnfs_spinlock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pnfsiod_start);
@@ -1511,13 +1515,11 @@ void pnfsiod_stop(void)
 {
 	struct workqueue_struct *wq = NULL;
 
-	spin_lock(&pnfs_spinlock);
-	pnfsiod_users--;
-	if (pnfsiod_users == 0) {
+	if (atomic_dec_and_lock(&pnfsiod_users, &pnfs_spinlock)) {
 		wq = pnfsiod_workqueue;
 		pnfsiod_workqueue = NULL;
+		spin_unlock(&pnfs_spinlock);
 	}
-	spin_unlock(&pnfs_spinlock);
 	if (wq)
 		destroy_workqueue(wq);
 }
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 2/2] SQUASHME: pnfs: do pnfsiod_start before registering layout drivers
  2011-09-11 15:15     ` Benny Halevy
  2011-09-11 15:31       ` [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue Benny Halevy
@ 2011-09-11 15:31       ` Benny Halevy
  1 sibling, 0 replies; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 15:31 UTC (permalink / raw)
  To: Jim Rees, Peng Tao; +Cc: linux-nfs, Benny Halevy

From: Benny Halevy <bhalevy@tonian.com>

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfs/blocklayout/blocklayout.c |   16 ++++++++--------
 fs/nfs/objlayout/objio_osd.c     |   12 ++++++------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 51f70f0..77bb3e7 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -977,41 +977,41 @@ static int __init nfs4blocklayout_init(void)
 
 	dprintk("%s: NFSv4 Block Layout Driver Registering...\n", __func__);
 
-	ret = pnfs_register_layoutdriver(&blocklayout_type);
+	ret = pnfsiod_start();
 	if (ret)
 		goto out;
 
-	ret = pnfsiod_start();
+	ret = pnfs_register_layoutdriver(&blocklayout_type);
 	if (ret)
-		goto out_remove;
+		goto out_stop;
 
 	init_waitqueue_head(&bl_wq);
 
 	mnt = rpc_get_mount();
 	if (IS_ERR(mnt)) {
 		ret = PTR_ERR(mnt);
-		goto out_stop;
+		goto out_remove;
 	}
 
 	ret = vfs_path_lookup(mnt->mnt_root,
 			      mnt,
 			      NFS_PIPE_DIRNAME, 0, &path);
 	if (ret)
-		goto out_stop;
+		goto out_remove;
 
 	bl_device_pipe = rpc_mkpipe(path.dentry, "blocklayout", NULL,
 				    &bl_upcall_ops, 0);
 	if (IS_ERR(bl_device_pipe)) {
 		ret = PTR_ERR(bl_device_pipe);
-		goto out_stop;
+		goto out_remove;
 	}
 out:
 	return ret;
 
-out_stop:
-	pnfsiod_stop();
 out_remove:
 	pnfs_unregister_layoutdriver(&blocklayout_type);
+out_stop:
+	pnfsiod_stop();
 	return ret;
 }
 
diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
index f28013f..7e8f0cc 100644
--- a/fs/nfs/objlayout/objio_osd.c
+++ b/fs/nfs/objlayout/objio_osd.c
@@ -1041,15 +1041,15 @@ MODULE_LICENSE("GPL");
 static int __init
 objlayout_init(void)
 {
-	int ret = pnfs_register_layoutdriver(&objlayout_type);
-	if (ret)
-		goto out;
+	int ret;
 
 	ret = pnfsiod_start();
-	if (ret)
-		pnfs_unregister_layoutdriver(&objlayout_type);
+	if (!ret) {
+		ret = pnfs_register_layoutdriver(&objlayout_type);
+		if (ret)
+			pnfsiod_stop();
+	}
 
-out:
 	if (ret)
 		printk(KERN_INFO
 			"%s: Registering OSD pNFS Layout Driver failed: error=%d\n",
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] pnfs private workqueue, and two cleanups
  2011-09-10 17:41 [PATCH 0/3] pnfs private workqueue, and two cleanups Jim Rees
                   ` (2 preceding siblings ...)
  2011-09-10 17:41 ` [PATCH 3/3] pNFS: make _set_lo_fail generic Jim Rees
@ 2011-09-11 16:01 ` Benny Halevy
  2011-09-11 16:04   ` Benny Halevy
  3 siblings, 1 reply; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 16:01 UTC (permalink / raw)
  To: Jim Rees; +Cc: linux-nfs, peter honeyman

I merged the patches as well as my two SQUASHMEs
under pnfs-all-3.1-rc4-2011-09-11

Thanks!

Benny

On 2011-09-10 10:41, Jim Rees wrote:
> These have been sent to the list previously, and I believe are
> uncontroversial, but have not shown up on your pnfs-all-latest branch.
> 
> Peng Tao (3):
>   SUNRPC/NFS: make rpc pipe upcall generic
>   pNFS: introduce pnfs private workqueue
>   pNFS: make _set_lo_fail generic
> 
>  fs/nfs/blocklayout/blocklayout.c    |   36 +++++++++-----------
>  fs/nfs/blocklayout/blocklayout.h    |    2 -
>  fs/nfs/blocklayout/blocklayoutdev.c |   22 ------------
>  fs/nfs/idmap.c                      |   25 +-------------
>  fs/nfs/nfs4filelayout.c             |   19 ++---------
>  fs/nfs/objlayout/objio_osd.c        |    8 ++++
>  fs/nfs/objlayout/objlayout.c        |    4 +-
>  fs/nfs/pnfs.c                       |   64 ++++++++++++++++++++++++++++++++++-
>  fs/nfs/pnfs.h                       |    5 +++
>  include/linux/sunrpc/rpc_pipe_fs.h  |    2 +
>  net/sunrpc/auth_gss/auth_gss.c      |   24 +------------
>  net/sunrpc/rpc_pipe.c               |   20 +++++++++++
>  12 files changed, 122 insertions(+), 109 deletions(-)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] pnfs private workqueue, and two cleanups
  2011-09-11 16:01 ` [PATCH 0/3] pnfs private workqueue, and two cleanups Benny Halevy
@ 2011-09-11 16:04   ` Benny Halevy
  2011-09-11 17:21     ` Jim Rees
  0 siblings, 1 reply; 13+ messages in thread
From: Benny Halevy @ 2011-09-11 16:04 UTC (permalink / raw)
  To: Jim Rees; +Cc: linux-nfs, peter honeyman

On 2011-09-11 09:01, Benny Halevy wrote:
> I merged the patches as well as my two SQUASHMEs
> under pnfs-all-3.1-rc4-2011-09-11

hit "Send" too soon...
In fact I merged all the patches in linux-pnfs-blk/for-benny
Including my SQUASHMEs, I merged the following patches:

fd06920 SUNRPC/NFS: make rpc pipe upcall generic
0359f0e pNFS: introduce pnfs private workqueue
fee4dab SQUASHME: pnfs: simplify and clean up pnfsiod workqueue
ab93fb1 SQUASHME: pnfs: do pnfsiod_start before registering layout drivers
9d904b1 pNFS: make _set_lo_fail generic
be077c0 pnfsblock: init pg_bsize properly
3c03366 pNFS: recoalesce when ld write pagelist fails
a4e8a6e pNFS: recoalesce when ld read pagelist fails

In addition, I fixed up some compile warnings in pnfsd/spnfs/spnfs-block

Benny

> 
> Thanks!
> 
> Benny
> 
> On 2011-09-10 10:41, Jim Rees wrote:
>> These have been sent to the list previously, and I believe are
>> uncontroversial, but have not shown up on your pnfs-all-latest branch.
>>
>> Peng Tao (3):
>>   SUNRPC/NFS: make rpc pipe upcall generic
>>   pNFS: introduce pnfs private workqueue
>>   pNFS: make _set_lo_fail generic
>>
>>  fs/nfs/blocklayout/blocklayout.c    |   36 +++++++++-----------
>>  fs/nfs/blocklayout/blocklayout.h    |    2 -
>>  fs/nfs/blocklayout/blocklayoutdev.c |   22 ------------
>>  fs/nfs/idmap.c                      |   25 +-------------
>>  fs/nfs/nfs4filelayout.c             |   19 ++---------
>>  fs/nfs/objlayout/objio_osd.c        |    8 ++++
>>  fs/nfs/objlayout/objlayout.c        |    4 +-
>>  fs/nfs/pnfs.c                       |   64 ++++++++++++++++++++++++++++++++++-
>>  fs/nfs/pnfs.h                       |    5 +++
>>  include/linux/sunrpc/rpc_pipe_fs.h  |    2 +
>>  net/sunrpc/auth_gss/auth_gss.c      |   24 +------------
>>  net/sunrpc/rpc_pipe.c               |   20 +++++++++++
>>  12 files changed, 122 insertions(+), 109 deletions(-)
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue
  2011-09-11 15:31       ` [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue Benny Halevy
@ 2011-09-11 17:17         ` Jim Rees
  2011-09-12 13:31           ` Benny Halevy
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Rees @ 2011-09-11 17:17 UTC (permalink / raw)
  To: Benny Halevy; +Cc: Peng Tao, linux-nfs, Benny Halevy

From: Jim Rees <rees@umich.edu>
Date: Sun, 11 Sep 2011 13:11:15 -0400
Subject: [PATCH] SQUASHME: pnfs: whitespace

Signed-off-by: Jim Rees <rees@umich.edu>
---
 fs/nfs/pnfs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 0ef4361..e5e11b4 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1488,13 +1488,13 @@ out:
 int pnfsiod_start(void)
 {
 	struct workqueue_struct *wq;
-	
+
 	dprintk("NFS:       creating workqueue pnfsiod\n");
 	atomic_inc(&pnfsiod_users);
 	if (pnfsiod_workqueue == NULL) {
 		wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
 		if (wq == NULL) {
-			pnfsiod_stop();	
+			pnfsiod_stop();
 			return -ENOMEM;
 		}
 		spin_lock(&pnfs_spinlock);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] pnfs private workqueue, and two cleanups
  2011-09-11 16:04   ` Benny Halevy
@ 2011-09-11 17:21     ` Jim Rees
  0 siblings, 0 replies; 13+ messages in thread
From: Jim Rees @ 2011-09-11 17:21 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

Benny Halevy wrote:

  On 2011-09-11 09:01, Benny Halevy wrote:
  > I merged the patches as well as my two SQUASHMEs
  > under pnfs-all-3.1-rc4-2011-09-11
  
  hit "Send" too soon...
  In fact I merged all the patches in linux-pnfs-blk/for-benny
  Including my SQUASHMEs, I merged the following patches:
  
  fd06920 SUNRPC/NFS: make rpc pipe upcall generic
  0359f0e pNFS: introduce pnfs private workqueue
  fee4dab SQUASHME: pnfs: simplify and clean up pnfsiod workqueue
  ab93fb1 SQUASHME: pnfs: do pnfsiod_start before registering layout drivers
  9d904b1 pNFS: make _set_lo_fail generic
  be077c0 pnfsblock: init pg_bsize properly
  3c03366 pNFS: recoalesce when ld write pagelist fails
  a4e8a6e pNFS: recoalesce when ld read pagelist fails

These last two are still being worked on, but I don't think it will hurt
anyone not using blocklayout to have them in your repo.  Thanks.

Also see the small whitespace cleanup I just sent.

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue
  2011-09-11 17:17         ` Jim Rees
@ 2011-09-12 13:31           ` Benny Halevy
  0 siblings, 0 replies; 13+ messages in thread
From: Benny Halevy @ 2011-09-12 13:31 UTC (permalink / raw)
  To: Jim Rees; +Cc: Peng Tao, linux-nfs

On 2011-09-11 10:17, Jim Rees wrote:
> From: Jim Rees <rees@umich.edu>
> Date: Sun, 11 Sep 2011 13:11:15 -0400
> Subject: [PATCH] SQUASHME: pnfs: whitespace

evil editor... :-/
thanks!

Benny

> 
> Signed-off-by: Jim Rees <rees@umich.edu>
> ---
>  fs/nfs/pnfs.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 0ef4361..e5e11b4 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1488,13 +1488,13 @@ out:
>  int pnfsiod_start(void)
>  {
>  	struct workqueue_struct *wq;
> -	
> +
>  	dprintk("NFS:       creating workqueue pnfsiod\n");
>  	atomic_inc(&pnfsiod_users);
>  	if (pnfsiod_workqueue == NULL) {
>  		wq = alloc_workqueue("pnfsiod", WQ_MEM_RECLAIM, 0);
>  		if (wq == NULL) {
> -			pnfsiod_stop();	
> +			pnfsiod_stop();
>  			return -ENOMEM;
>  		}
>  		spin_lock(&pnfs_spinlock);

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-09-12 13:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-10 17:41 [PATCH 0/3] pnfs private workqueue, and two cleanups Jim Rees
2011-09-10 17:41 ` [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic Jim Rees
2011-09-10 17:41 ` [PATCH 2/3] pNFS: introduce pnfs private workqueue Jim Rees
2011-09-11 14:51   ` Benny Halevy
2011-09-11 15:15     ` Benny Halevy
2011-09-11 15:31       ` [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue Benny Halevy
2011-09-11 17:17         ` Jim Rees
2011-09-12 13:31           ` Benny Halevy
2011-09-11 15:31       ` [PATCH 2/2] SQUASHME: pnfs: do pnfsiod_start before registering layout drivers Benny Halevy
2011-09-10 17:41 ` [PATCH 3/3] pNFS: make _set_lo_fail generic Jim Rees
2011-09-11 16:01 ` [PATCH 0/3] pnfs private workqueue, and two cleanups Benny Halevy
2011-09-11 16:04   ` Benny Halevy
2011-09-11 17:21     ` Jim Rees

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.