For the pnfs-submit branch 2.6.35-rc3 7-6-2010 tree.
These first three patches simplify the inode.c pnfs initialization calls.
0001-SQUASHME-pnfs-submit-remove-pnfs_init_once.patch
0002-SQUASHME-pnfs-submit-remove-pnfs_alloc_init_inode.patch
0003-SQUASHME-pnfs-submit-remove-pnfs_destroy_inode.patch
These two patches fix bugs in the layoutget and layoutcommit error handling
where the calldata->status was not set after the async error handler in the
rpc_call_done routines.
0004-SQUASHME-pnfs-submit-set-layoutcommit-status-after-a.patch
0005-SQUASHME-pnfs-submit-set-layoutget-status-after-asyn.patch
Tested:
CONFIG_NFS_V4_1 set:
pNFS mount connectathon tests pass.
pyNFS server with NFS4ERR_DELAY returned by layoutget handles the error in
the async error handler, and then the calldata->status is set to NFS4_OK.
CONFIG_NFS_V4_1 not set;
NFSv4.0 mount passes connectathon tests.
-->Andy
From: Andy Adamson <[email protected]>
Move WARN_ONs and list check into pnfs_layout_destroy under the i_lock
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/inode.c | 18 +-----------------
fs/nfs/pnfs.c | 9 +++++++++
fs/nfs/pnfs.h | 4 ++++
3 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index fa310b1..229cdab 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1379,27 +1379,11 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
return &nfsi->vfs_inode;
}
-static void pnfs_destroy_inode(struct nfs_inode *nfsi)
-{
-#ifdef CONFIG_NFS_V4_1
- if (!list_empty(&nfsi->layout.segs))
- pnfs_destroy_layout(nfsi);
-
- WARN_ON(!list_empty(&nfsi->layout.segs));
- if (nfsi->layout.refcount)
- printk("%s: WARNING: layout.refcount %d\n", __func__,
- nfsi->layout.refcount);
- WARN_ON(nfsi->layout.refcount);
- WARN_ON(!list_empty(&nfsi->layout.lo_layouts));
- WARN_ON(nfsi->layout.ld_data);
-#endif /* CONFIG_NFS_V4_1 */
-}
-
void nfs_destroy_inode(struct inode *inode)
{
struct nfs_inode *nfsi = NFS_I(inode);
- pnfs_destroy_inode(nfsi);
+ pnfs_destroy_layout(nfsi);
kmem_cache_free(nfs_inode_cachep, nfsi);
}
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index baa3de7..fb9374b 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -379,6 +379,15 @@ pnfs_destroy_layout(struct nfs_inode *nfsi)
lo = grab_current_layout(nfsi);
if (lo) {
pnfs_free_layout(lo, &range);
+ WARN_ON(!list_empty(&nfsi->layout.segs));
+ WARN_ON(!list_empty(&nfsi->layout.lo_layouts));
+ WARN_ON(nfsi->layout.ld_data);
+
+ if (nfsi->layout.refcount != 1)
+ printk(KERN_WARNING "%s: layout refcount not=1 %d\n",
+ __func__, nfsi->layout.refcount);
+ WARN_ON(nfsi->layout.refcount != 1);
+
put_layout(lo);
}
spin_unlock(&nfsi->vfs_inode.i_lock);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index c60eff6..9b0fed4 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -190,6 +190,10 @@ static inline void pnfs_update_layout(struct inode *ino,
#else /* CONFIG_NFS_V4_1 */
+static inline void pnfs_destroy_layout(struct nfs_inode *nfsi)
+{
+}
+
static inline void get_lseg(struct pnfs_layout_segment *lseg)
{
}
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 36de1b3..c5b8a2f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5450,6 +5450,7 @@ static void nfs4_pnfs_layoutget_done(struct rpc_task *task, void *calldata)
if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
nfs_restart_rpc(task, server->nfs_client);
+ lgp->status = task->tk_status;
dprintk("<-- %s\n", __func__);
}
--
1.6.6
From: Andy Adamson <[email protected]>
Place all layout initialization in nfs4_init_once
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/inode.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 7989dea..231cfa3 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1416,8 +1416,13 @@ void nfs_destroy_inode(struct inode *inode)
kmem_cache_free(nfs_inode_cachep, nfsi);
}
-static void pnfs_init_once(struct nfs_inode *nfsi)
+static inline void nfs4_init_once(struct nfs_inode *nfsi)
{
+#ifdef CONFIG_NFS_V4
+ INIT_LIST_HEAD(&nfsi->open_states);
+ nfsi->delegation = NULL;
+ nfsi->delegation_state = 0;
+ init_rwsem(&nfsi->rwsem);
#ifdef CONFIG_NFS_V4_1
init_waitqueue_head(&nfsi->lo_waitq);
seqlock_init(&nfsi->layout.seqlock);
@@ -1426,15 +1431,6 @@ static void pnfs_init_once(struct nfs_inode *nfsi)
nfsi->layout.refcount = 0;
nfsi->layout.ld_data = NULL;
#endif /* CONFIG_NFS_V4_1 */
-}
-
-static inline void nfs4_init_once(struct nfs_inode *nfsi)
-{
-#ifdef CONFIG_NFS_V4
- INIT_LIST_HEAD(&nfsi->open_states);
- nfsi->delegation = NULL;
- nfsi->delegation_state = 0;
- init_rwsem(&nfsi->rwsem);
#endif
}
@@ -1453,7 +1449,6 @@ static void init_once(void *foo)
INIT_HLIST_HEAD(&nfsi->silly_list);
init_waitqueue_head(&nfsi->waitqueue);
nfs4_init_once(nfsi);
- pnfs_init_once(nfsi);
}
static int __init nfs_init_inodecache(void)
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6283996..36de1b3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5549,14 +5549,14 @@ pnfs_layoutcommit_done(struct rpc_task *task, void *calldata)
(struct pnfs_layoutcommit_data *)calldata;
struct nfs_server *server = NFS_SERVER(data->args.inode);
- data->status = task->tk_status;
-
nfs4_sequence_done(server, &data->res.seq_res, task->tk_status);
if (RPC_ASSASSINATED(task))
return;
if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
nfs_restart_rpc(task, server->nfs_client);
+
+ data->status = task->tk_status;
}
static void pnfs_layoutcommit_release(void *lcdata)
--
1.6.6
From: Andy Adamson <[email protected]>
Place all layout initialization in nfs4_init_once
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/inode.c | 19 ++++++-------------
1 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 231cfa3..fa310b1 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1361,18 +1361,6 @@ void nfs4_clear_inode(struct inode *inode)
}
#endif
-static void pnfs_alloc_init_inode(struct nfs_inode *nfsi)
-{
-#ifdef CONFIG_NFS_V4_1
- nfsi->layout.pnfs_layout_state = 0;
- memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
- nfsi->layout.roc_iomode = 0;
- nfsi->layout.lo_cred = NULL;
- nfsi->layout.pnfs_write_begin_pos = 0;
- nfsi->layout.pnfs_write_end_pos = 0;
-#endif /* CONFIG_NFS_V4_1 */
-}
-
struct inode *nfs_alloc_inode(struct super_block *sb)
{
struct nfs_inode *nfsi;
@@ -1388,7 +1376,6 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
#ifdef CONFIG_NFS_V4
nfsi->nfs4_acl = NULL;
#endif /* CONFIG_NFS_V4 */
- pnfs_alloc_init_inode(nfsi);
return &nfsi->vfs_inode;
}
@@ -1430,6 +1417,12 @@ static inline void nfs4_init_once(struct nfs_inode *nfsi)
INIT_LIST_HEAD(&nfsi->layout.segs);
nfsi->layout.refcount = 0;
nfsi->layout.ld_data = NULL;
+ nfsi->layout.pnfs_layout_state = 0;
+ memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
+ nfsi->layout.roc_iomode = 0;
+ nfsi->layout.lo_cred = NULL;
+ nfsi->layout.pnfs_write_begin_pos = 0;
+ nfsi->layout.pnfs_write_end_pos = 0;
#endif /* CONFIG_NFS_V4_1 */
#endif
}
--
1.6.6
On 07/07/2010 12:16 AM, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Place all layout initialization in nfs4_init_once
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> fs/nfs/inode.c | 19 ++++++-------------
> 1 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 231cfa3..fa310b1 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1361,18 +1361,6 @@ void nfs4_clear_inode(struct inode *inode)
> }
> #endif
>
> -static void pnfs_alloc_init_inode(struct nfs_inode *nfsi)
> -{
> -#ifdef CONFIG_NFS_V4_1
> - nfsi->layout.pnfs_layout_state = 0;
> - memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
> - nfsi->layout.roc_iomode = 0;
> - nfsi->layout.lo_cred = NULL;
> - nfsi->layout.pnfs_write_begin_pos = 0;
> - nfsi->layout.pnfs_write_end_pos = 0;
> -#endif /* CONFIG_NFS_V4_1 */
> -}
> -
> struct inode *nfs_alloc_inode(struct super_block *sb)
> {
> struct nfs_inode *nfsi;
> @@ -1388,7 +1376,6 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
> #ifdef CONFIG_NFS_V4
> nfsi->nfs4_acl = NULL;
> #endif /* CONFIG_NFS_V4 */
> - pnfs_alloc_init_inode(nfsi);
> return &nfsi->vfs_inode;
> }
>
> @@ -1430,6 +1417,12 @@ static inline void nfs4_init_once(struct nfs_inode *nfsi)
> INIT_LIST_HEAD(&nfsi->layout.segs);
> nfsi->layout.refcount = 0;
> nfsi->layout.ld_data = NULL;
> + nfsi->layout.pnfs_layout_state = 0;
> + memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
> + nfsi->layout.roc_iomode = 0;
> + nfsi->layout.lo_cred = NULL;
> + nfsi->layout.pnfs_write_begin_pos = 0;
> + nfsi->layout.pnfs_write_end_pos = 0;
If we are already here. What are the rules with zeros? It is costume elsewhere in
Kernel that at construction points all zeros are just not done, if a kzalloc or memset
is guaranteed. Isn't nfs_inode zero_allocated? If not it should. Adding zero initialization
to every new member, and an extra remove line to any member remove is just maintenance
nightmare.
(This patch could be much more beautiful if it was "only removed lines")
Boaz
> #endif /* CONFIG_NFS_V4_1 */
> #endif
> }
On Wed, Jul 7, 2010 at 5:27 AM, Boaz Harrosh <[email protected]> wro=
te:
> On 07/07/2010 12:16 AM, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Place all layout initialization in nfs4_init_once
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> =A0fs/nfs/inode.c | =A0 19 ++++++-------------
>> =A01 files changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 231cfa3..fa310b1 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -1361,18 +1361,6 @@ void nfs4_clear_inode(struct inode *inode)
>> =A0}
>> =A0#endif
>>
>> -static void pnfs_alloc_init_inode(struct nfs_inode *nfsi)
>> -{
>> -#ifdef CONFIG_NFS_V4_1
>> - =A0 =A0 nfsi->layout.pnfs_layout_state =3D 0;
>> - =A0 =A0 memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
>> - =A0 =A0 nfsi->layout.roc_iomode =3D 0;
>> - =A0 =A0 nfsi->layout.lo_cred =3D NULL;
>> - =A0 =A0 nfsi->layout.pnfs_write_begin_pos =3D 0;
>> - =A0 =A0 nfsi->layout.pnfs_write_end_pos =3D 0;
>> -#endif /* CONFIG_NFS_V4_1 */
>> -}
>> -
>> =A0struct inode *nfs_alloc_inode(struct super_block *sb)
>> =A0{
>> =A0 =A0 =A0 struct nfs_inode *nfsi;
>> @@ -1388,7 +1376,6 @@ struct inode *nfs_alloc_inode(struct super_blo=
ck *sb)
>> =A0#ifdef CONFIG_NFS_V4
>> =A0 =A0 =A0 nfsi->nfs4_acl =3D NULL;
>> =A0#endif /* CONFIG_NFS_V4 */
>> - =A0 =A0 pnfs_alloc_init_inode(nfsi);
>> =A0 =A0 =A0 return &nfsi->vfs_inode;
>> =A0}
>>
>> @@ -1430,6 +1417,12 @@ static inline void nfs4_init_once(struct nfs_=
inode *nfsi)
>> =A0 =A0 =A0 INIT_LIST_HEAD(&nfsi->layout.segs);
>> =A0 =A0 =A0 nfsi->layout.refcount =3D 0;
>> =A0 =A0 =A0 nfsi->layout.ld_data =3D NULL;
>> + =A0 =A0 nfsi->layout.pnfs_layout_state =3D 0;
>> + =A0 =A0 memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
>> + =A0 =A0 nfsi->layout.roc_iomode =3D 0;
>> + =A0 =A0 nfsi->layout.lo_cred =3D NULL;
>> + =A0 =A0 nfsi->layout.pnfs_write_begin_pos =3D 0;
>> + =A0 =A0 nfsi->layout.pnfs_write_end_pos =3D 0;
>
> If we are already here. What are the rules with zeros? It is costume =
elsewhere in
> Kernel that at construction points all zeros are just not done, if a =
kzalloc or memset
> is guaranteed. Isn't nfs_inode zero_allocated? If not it should. Addi=
ng zero initialization
> to every new member, and an extra remove line to any member remove is=
just maintenance
> nightmare.
Well, the next patch set will remove all but one zero assignment.
Second; no, nfs_alloc_inode does not zero out all nfs_inode fields.
>
> (This patch could be much more beautiful if it was "only removed line=
s")
The end result will be beautiful. I just wanted to break up all the cha=
nges.
-->Andy
>
> Boaz
>
>
>> =A0#endif /* CONFIG_NFS_V4_1 */
>> =A0#endif
>> =A0}
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>