So in blocklayoutdriver, we can use GFP_KERNEL to do memory
allocation in bl_setup_layoutcommit().
Signed-off-by: Tao Guo <[email protected]>
---
fs/nfs/pnfs.c | 42 +++++++++++++++++++++---------------------
1 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index aedda1e..8474731 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -2073,15 +2073,23 @@ pnfs_layoutcommit_done(struct pnfs_layoutcommit_data *data)
* Set up the argument/result storage required for the RPC call.
*/
static int
-pnfs_layoutcommit_setup(struct pnfs_layoutcommit_data *data, int sync)
+pnfs_layoutcommit_setup(struct inode *inode,
+ struct pnfs_layoutcommit_data *data, int sync)
{
- struct nfs_inode *nfsi = NFS_I(data->args.inode);
- struct nfs_server *nfss = NFS_SERVER(data->args.inode);
+ struct nfs_inode *nfsi = NFS_I(inode);
+ struct nfs_server *nfss = NFS_SERVER(inode);
int result = 0;
dprintk("%s Begin (sync:%d)\n", __func__, sync);
+ data->is_sync = sync;
+ data->cred = nfsi->layoutcommit_ctx->cred;
+ data->ctx = nfsi->layoutcommit_ctx;
+ data->args.inode = inode;
data->args.fh = NFS_FH(data->args.inode);
data->args.layout_type = nfss->pnfs_curr_ld->id;
+ pnfs_get_layout_stateid(&data->args.stateid, &nfsi->layout);
+ data->res.fattr = &data->fattr;
+ nfs_fattr_init(&data->fattr);
/* TODO: Need to determine the correct values */
data->args.time_modify_changed = 0;
@@ -2095,6 +2103,7 @@ pnfs_layoutcommit_setup(struct pnfs_layoutcommit_data *data, int sync)
i_size_read(&nfsi->vfs_inode) - 1);
data->args.bitmask = nfss->attr_bitmask;
data->res.server = nfss;
+ spin_unlock(&nfsi->lo_lock);
/* Call layout driver to set the arguments.
*/
@@ -2104,10 +2113,6 @@ pnfs_layoutcommit_setup(struct pnfs_layoutcommit_data *data, int sync)
if (result)
goto out;
}
- pnfs_get_layout_stateid(&data->args.stateid, &nfsi->layout);
- data->res.fattr = &data->fattr;
- nfs_fattr_init(&data->fattr);
-
out:
dprintk("%s End Status %d\n", __func__, result);
return result;
@@ -2131,36 +2136,31 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync)
return -ENOMEM;
spin_lock(&nfsi->lo_lock);
- if (!nfsi->layoutcommit_ctx)
- goto out_unlock;
-
- data->args.inode = inode;
- data->cred = nfsi->layoutcommit_ctx->cred;
- data->ctx = nfsi->layoutcommit_ctx;
+ if (!nfsi->layoutcommit_ctx) {
+ spin_unlock(&nfsi->lo_lock);
+ goto out_free;
+ }
- /* Set up layout commit args*/
- status = pnfs_layoutcommit_setup(data, sync);
+ /* Set up layout commit args */
+ status = pnfs_layoutcommit_setup(inode, data, sync);
if (status)
- goto out_unlock;
+ goto out_free;
/* Clear layoutcommit properties in the inode so
* new lc info can be generated
*/
+ spin_lock(&nfsi->lo_lock);
nfsi->pnfs_write_begin_pos = 0;
nfsi->pnfs_write_end_pos = 0;
nfsi->layoutcommit_ctx = NULL;
-
- /* release lock on pnfs layoutcommit attrs */
spin_unlock(&nfsi->lo_lock);
- data->is_sync = sync;
status = pnfs4_proc_layoutcommit(data);
out:
dprintk("%s end (err:%d)\n", __func__, status);
return status;
-out_unlock:
+out_free:
pnfs_layoutcommit_free(data);
- spin_unlock(&nfsi->lo_lock);
goto out;
}
--
1.6.3.3
On May 20, 2010, at 6:25 AM, Tao Guo wrote:
> So in blocklayoutdriver, we can use GFP_KERNEL to do memory
> allocation in bl_setup_layoutcommit().
>
> Signed-off-by: Tao Guo <[email protected]>
> ---
> fs/nfs/pnfs.c | 42 +++++++++++++++++++++---------------------
> 1 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index aedda1e..8474731 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -2073,15 +2073,23 @@ pnfs_layoutcommit_done(struct
> pnfs_layoutcommit_data *data)
> * Set up the argument/result storage required for the RPC call.
> */
> static int
> -pnfs_layoutcommit_setup(struct pnfs_layoutcommit_data *data, int
> sync)
> +pnfs_layoutcommit_setup(struct inode *inode,
> + struct pnfs_layoutcommit_data *data, int sync)
> {
> - struct nfs_inode *nfsi = NFS_I(data->args.inode);
> - struct nfs_server *nfss = NFS_SERVER(data->args.inode);
> + struct nfs_inode *nfsi = NFS_I(inode);
> + struct nfs_server *nfss = NFS_SERVER(inode);
> int result = 0;
>
> dprintk("%s Begin (sync:%d)\n", __func__, sync);
> + data->is_sync = sync;
> + data->cred = nfsi->layoutcommit_ctx->cred;
> + data->ctx = nfsi->layoutcommit_ctx;
> + data->args.inode = inode;
> data->args.fh = NFS_FH(data->args.inode);
> data->args.layout_type = nfss->pnfs_curr_ld->id;
> + pnfs_get_layout_stateid(&data->args.stateid, &nfsi->layout);
> + data->res.fattr = &data->fattr;
> + nfs_fattr_init(&data->fattr);
>
> /* TODO: Need to determine the correct values */
> data->args.time_modify_changed = 0;
> @@ -2095,6 +2103,7 @@ pnfs_layoutcommit_setup(struct
> pnfs_layoutcommit_data *data, int sync)
> i_size_read(&nfsi->vfs_inode) - 1);
> data->args.bitmask = nfss->attr_bitmask;
> data->res.server = nfss;
> + spin_unlock(&nfsi->lo_lock);
>
> /* Call layout driver to set the arguments.
> */
> @@ -2104,10 +2113,6 @@ pnfs_layoutcommit_setup(struct
> pnfs_layoutcommit_data *data, int sync)
> if (result)
> goto out;
> }
> - pnfs_get_layout_stateid(&data->args.stateid, &nfsi->layout);
> - data->res.fattr = &data->fattr;
> - nfs_fattr_init(&data->fattr);
> -
> out:
> dprintk("%s End Status %d\n", __func__, result);
> return result;
> @@ -2131,36 +2136,31 @@ pnfs_layoutcommit_inode(struct inode *inode,
> int sync)
> return -ENOMEM;
>
> spin_lock(&nfsi->lo_lock);
> - if (!nfsi->layoutcommit_ctx)
> - goto out_unlock;
> -
> - data->args.inode = inode;
> - data->cred = nfsi->layoutcommit_ctx->cred;
> - data->ctx = nfsi->layoutcommit_ctx;
> + if (!nfsi->layoutcommit_ctx) {
> + spin_unlock(&nfsi->lo_lock);
> + goto out_free;
> + }
>
> - /* Set up layout commit args*/
> - status = pnfs_layoutcommit_setup(data, sync);
> + /* Set up layout commit args */
> + status = pnfs_layoutcommit_setup(inode, data, sync);
> if (status)
> - goto out_unlock;
> + goto out_free;
>
> /* Clear layoutcommit properties in the inode so
> * new lc info can be generated
> */
> + spin_lock(&nfsi->lo_lock);
> nfsi->pnfs_write_begin_pos = 0;
> nfsi->pnfs_write_end_pos = 0;
> nfsi->layoutcommit_ctx = NULL;
These should be cleared before the spin_lock is released in
pnfs_layoutcommit_setup.
They should be cleared (and the layoutcommit_cxt put) upon a
layoutdriver setup_layoutcommit failure.
-->Andy
> -
> - /* release lock on pnfs layoutcommit attrs */
> spin_unlock(&nfsi->lo_lock);
>
> - data->is_sync = sync;
> status = pnfs4_proc_layoutcommit(data);
> out:
> dprintk("%s end (err:%d)\n", __func__, status);
> return status;
> -out_unlock:
> +out_free:
> pnfs_layoutcommit_free(data);
> - spin_unlock(&nfsi->lo_lock);
> goto out;
> }
>
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 21, 2010 at 12:13 AM, Andy Adamson <[email protected]> wrot=
e:
>
> On May 20, 2010, at 6:25 AM, Tao Guo wrote:
>
>> So in blocklayoutdriver, we can use GFP_KERNEL to do memory
>> allocation in bl_setup_layoutcommit().
>>
>> Signed-off-by: Tao Guo <[email protected]>
>> ---
>> fs/nfs/pnfs.c | =C2=A0 42 +++++++++++++++++++++---------------------
>> 1 files changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index aedda1e..8474731 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -2073,15 +2073,23 @@ pnfs_layoutcommit_done(struct
>> pnfs_layoutcommit_data *data)
>> =C2=A0* Set up the argument/result storage required for the RPC call=
=2E
>> =C2=A0*/
>> static int
>> -pnfs_layoutcommit_setup(struct pnfs_layoutcommit_data *data, int sy=
nc)
>> +pnfs_layoutcommit_setup(struct inode *inode,
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 struct pnfs_layoutcommit_data *data, int sync)
>> {
>> - =C2=A0 =C2=A0 =C2=A0 struct nfs_inode *nfsi =3D NFS_I(data->args.i=
node);
>> - =C2=A0 =C2=A0 =C2=A0 struct nfs_server *nfss =3D NFS_SERVER(data->=
args.inode);
>> + =C2=A0 =C2=A0 =C2=A0 struct nfs_inode *nfsi =3D NFS_I(inode);
>> + =C2=A0 =C2=A0 =C2=A0 struct nfs_server *nfss =3D NFS_SERVER(inode)=
;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0int result =3D 0;
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0dprintk("%s Begin (sync:%d)\n", __func__,=
sync);
>> + =C2=A0 =C2=A0 =C2=A0 data->is_sync =3D sync;
>> + =C2=A0 =C2=A0 =C2=A0 data->cred =C2=A0=3D nfsi->layoutcommit_ctx->=
cred;
>> + =C2=A0 =C2=A0 =C2=A0 data->ctx =3D nfsi->layoutcommit_ctx;
>> + =C2=A0 =C2=A0 =C2=A0 data->args.inode =3D inode;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0data->args.fh =3D NFS_FH(data->args.inode=
);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0data->args.layout_type =3D nfss->pnfs_cur=
r_ld->id;
>> + =C2=A0 =C2=A0 =C2=A0 pnfs_get_layout_stateid(&data->args.stateid, =
&nfsi->layout);
>> + =C2=A0 =C2=A0 =C2=A0 data->res.fattr =3D &data->fattr;
>> + =C2=A0 =C2=A0 =C2=A0 nfs_fattr_init(&data->fattr);
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* TODO: Need to determine the correct va=
lues */
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0data->args.time_modify_changed =3D 0;
>> @@ -2095,6 +2103,7 @@ pnfs_layoutcommit_setup(struct
>> pnfs_layoutcommit_data *data, int sync)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0i=
_size_read(&nfsi->vfs_inode) - 1);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0data->args.bitmask =3D nfss->attr_bitmask=
;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0data->res.server =3D nfss;
>> + =C2=A0 =C2=A0 =C2=A0 spin_unlock(&nfsi->lo_lock);
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Call layout driver to set the argument=
s.
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
>> @@ -2104,10 +2113,6 @@ pnfs_layoutcommit_setup(struct
>> pnfs_layoutcommit_data *data, int sync)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (result)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0goto out;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> - =C2=A0 =C2=A0 =C2=A0 pnfs_get_layout_stateid(&data->args.stateid, =
&nfsi->layout);
>> - =C2=A0 =C2=A0 =C2=A0 data->res.fattr =3D &data->fattr;
>> - =C2=A0 =C2=A0 =C2=A0 nfs_fattr_init(&data->fattr);
>> -
>> out:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0dprintk("%s End Status %d\n", __func__, r=
esult);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0return result;
>> @@ -2131,36 +2136,31 @@ pnfs_layoutcommit_inode(struct inode *inode,=
int
>> sync)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -ENOME=
M;
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0spin_lock(&nfsi->lo_lock);
>> - =C2=A0 =C2=A0 =C2=A0 if (!nfsi->layoutcommit_ctx)
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto out_unlock;
>> -
>> - =C2=A0 =C2=A0 =C2=A0 data->args.inode =3D inode;
>> - =C2=A0 =C2=A0 =C2=A0 data->cred =C2=A0=3D nfsi->layoutcommit_ctx->=
cred;
>> - =C2=A0 =C2=A0 =C2=A0 data->ctx =3D nfsi->layoutcommit_ctx;
>> + =C2=A0 =C2=A0 =C2=A0 if (!nfsi->layoutcommit_ctx) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 spin_unlock(&nfsi=
->lo_lock);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto out_free;
>> + =C2=A0 =C2=A0 =C2=A0 }
>>
>> - =C2=A0 =C2=A0 =C2=A0 /* Set up layout commit args*/
>> - =C2=A0 =C2=A0 =C2=A0 status =3D pnfs_layoutcommit_setup(data, sync=
);
>> + =C2=A0 =C2=A0 =C2=A0 /* Set up layout commit args */
>> + =C2=A0 =C2=A0 =C2=A0 status =3D pnfs_layoutcommit_setup(inode, dat=
a, sync);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (status)
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto out_unlock;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto out_free;
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Clear layoutcommit properties in the i=
node so
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 * new lc info can be generated
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
>> + =C2=A0 =C2=A0 =C2=A0 spin_lock(&nfsi->lo_lock);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0nfsi->pnfs_write_begin_pos =3D 0;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0nfsi->pnfs_write_end_pos =3D 0;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0nfsi->layoutcommit_ctx =3D NULL;
>
> These should be cleared before the spin_lock is released in
> pnfs_layoutcommit_setup.
> They should be cleared (and the layoutcommit_cxt put) upon a layoutdr=
iver
> setup_layoutcommit failure.
>
> -->Andy
I just followed the old code's logic: if a layoutdriver's
setup_layoutcommit call failed(like return -ENOMEM), we just keep
nfs_inode's everything unchanged, so maybe next time we can try again
later. However, this will lead to lock lo_lock two times as in the
above code.
Maybe I should break the code's logic...
>>
>> -
>> - =C2=A0 =C2=A0 =C2=A0 /* release lock on pnfs layoutcommit attrs */
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0spin_unlock(&nfsi->lo_lock);
>>
>> - =C2=A0 =C2=A0 =C2=A0 data->is_sync =3D sync;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0status =3D pnfs4_proc_layoutcommit(data);
>> out:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0dprintk("%s end (err:%d)\n", __func__, st=
atus);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0return status;
>> -out_unlock:
>> +out_free:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0pnfs_layoutcommit_free(data);
>> - =C2=A0 =C2=A0 =C2=A0 spin_unlock(&nfsi->lo_lock);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0goto out;
>> }
>>
>> --
>> 1.6.3.3
--=20
tao.