On Fri, Feb 6, 2015 at 9:45 AM, Peng Tao <tao.peng@xxxxxxxxxxxxxxx> wrote:
> On Fri, Feb 6, 2015 at 6:37 AM, Trond Myklebust
> <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>> If we have to do a return-on-close in the delegreturn code, then
>> we must ensure that the inode and super block remain referenced.
>>
ah, a second thought. I looked for call sites of nfs_sb_active() and
it gets called at five places in current tree:
alloc_nfs_open_context, nfs4_opendata_alloc, nfs4_do_close,
nfs_do_call_unlink, nfs_do_call_unlink
So it appears that sb is activated while any file keeps opened and
between unlink calls. Then it looks that we are allowed to keep
delegations after sb is released? Maybe the best way to fix the sb
reference part is to pin sb when getting the first delegation.
Cheers,
Tao
> looks good. One nit is that maybe it's better to reuse the two helpers
> in your 2ed patch.
>
> Reviewed-by: Peng Tao <tao.peng@xxxxxxxxxxxxxxx>
>> Cc: Peng Tao <tao.peng@xxxxxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx # 3.17.x
>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>> ---
>> fs/nfs/nfs4proc.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index cd4295d84d54..b803c1d363e7 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5175,9 +5175,16 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
>> static void nfs4_delegreturn_release(void *calldata)
>> {
>> struct nfs4_delegreturndata *data = calldata;
>> + struct inode *inode = data->inode;
>> +
>> + if (inode) {
>> + struct super_block *sb = inode->i_sb;
>>
>> - if (data->roc)
>> - pnfs_roc_release(data->inode);
>> + if (data->roc)
>> + pnfs_roc_release(inode);
>> + iput(inode);
>> + nfs_sb_deactive(sb);
>> + }
>> kfree(calldata);
>> }
>>
>> @@ -5234,9 +5241,11 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co
>> nfs_fattr_init(data->res.fattr);
>> data->timestamp = jiffies;
>> data->rpc_status = 0;
>> - data->inode = inode;
>> - data->roc = list_empty(&NFS_I(inode)->open_files) ?
>> - pnfs_roc(inode) : false;
>> + data->inode = igrab(inode);
>> + if (data->inode) {
>> + nfs_sb_active(inode->i_sb);
>> + data->roc = nfs4_roc(inode);
>> + }
>>
>> task_setup_data.callback_data = data;
>> msg.rpc_argp = &data->args;
>> --
>> 2.1.0
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
