2011-09-10 17:41:38

by Jim Rees

[permalink] [raw]
Subject: [PATCH 0/3] pnfs private workqueue, and two cleanups

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



2011-09-12 13:38:09

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue

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

evil editor... :-/
thanks!

Benny

>
> Signed-off-by: Jim Rees <[email protected]>
> ---
> 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);

2011-09-11 14:51:48

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 2/3] pNFS: introduce pnfs private workqueue

On 2011-09-10 10:41, Jim Rees wrote:
> From: Peng Tao <[email protected]>

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 <[email protected]>
> Signed-off-by: Jim Rees <[email protected]>
> ---
> 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);
>

2011-09-10 17:41:42

by Jim Rees

[permalink] [raw]
Subject: [PATCH 3/3] pNFS: make _set_lo_fail generic

From: Peng Tao <[email protected]>

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

Signed-off-by: Peng Tao <[email protected]>
Signed-off-by: Jim Rees <[email protected]>
---
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


2011-09-11 15:31:47

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue

From: Benny Halevy <[email protected]>

Signed-off-by: Benny Halevy <[email protected]>
---
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


2011-09-11 15:15:07

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 2/3] pNFS: introduce pnfs private workqueue

On 2011-09-11 07:51, Benny Halevy wrote:
> On 2011-09-10 10:41, Jim Rees wrote:
>> From: Peng Tao <[email protected]>
>
> 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 <[email protected]>
>> Signed-off-by: Jim Rees <[email protected]>
>> ---
>> 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);
>>

2011-09-10 17:41:42

by Jim Rees

[permalink] [raw]
Subject: [PATCH 2/3] pNFS: introduce pnfs private workqueue

From: Peng Tao <[email protected]>

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 <[email protected]>
Signed-off-by: Jim Rees <[email protected]>
---
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


2011-09-11 15:31:56

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/2] SQUASHME: pnfs: do pnfsiod_start before registering layout drivers

From: Benny Halevy <[email protected]>

Signed-off-by: Benny Halevy <[email protected]>
---
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


2011-09-11 16:04:47

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 0/3] pnfs private workqueue, and two cleanups

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(-)
>>

2011-09-11 17:21:37

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH 0/3] pnfs private workqueue, and two cleanups

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.

2011-09-10 17:41:40

by Jim Rees

[permalink] [raw]
Subject: [PATCH 1/3] SUNRPC/NFS: make rpc pipe upcall generic

From: Peng Tao <[email protected]>

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

Signed-off-by: Peng Tao <[email protected]>
Signed-off-by: Jim Rees <[email protected]>
---
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


2011-09-11 17:17:52

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH 1/2] SQUASHME: pnfs: simplify and clean up pnfsiod workqueue

From: Jim Rees <[email protected]>
Date: Sun, 11 Sep 2011 13:11:15 -0400
Subject: [PATCH] SQUASHME: pnfs: whitespace

Signed-off-by: Jim Rees <[email protected]>
---
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

2011-09-11 16:01:03

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 0/3] pnfs private workqueue, and two cleanups

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(-)
>