On 05/23/2011 05:54 PM, Fred Isaman wrote:
> On Mon, May 23, 2011 at 12:22 AM, Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
>> On 05/22/2011 10:40 PM, Benny Halevy wrote:
>>>> +/*
>>>> + * Called by non rpc-based layout drivers
>>>> + */
>>>> +int
>>>> +pnfs_ld_write_done(struct nfs_write_data *data)
>>>> +{
>>>> + int status;
>>>> +
>>>
>>>
>>>> + if (!data->pnfs_error) {
>>>> + pnfs_set_layoutcommit(data);
>>>
>>> We need at least to set data->task.tk_status to 0
>>
>> I guess it does not hurt, but we never touched it, is it garbage?
>>
>>>
>>>> + data->mds_ops->rpc_call_done(&data->task, data);
>>>> + data->mds_ops->rpc_release(data);
>>>
>>> Where's the put_lseg you had in PATCH 10/13?
>>>
>>> Benny
>>
>> That was the bug. Please see my SQUASHME patches I explained
>> it all there.
>>
>>>
>>>> + return 0;
>>>> + }
>>>> +
>>>> + put_lseg(data->lseg);
>>>> + data->lseg = NULL;
>>
>> I'm not sure it is needed here as well.
>> Fred! please see this code
>> We know that the lseg is put in nfs_writedata_release()
>> Does the below nfs_initiate_write() retakes the ref.
>> If it does we need the put here. If it does not we
>> don't need here.
>>
>
> nfs_initiate_read/write() did not take any reference to the lseg.
> This was done immediately prior in the nfs_read/write_rpcsetup
> functions.
>
Thanks, Yes, you are right. I actually tested that and I saw the bug.
The put here in the error case needs to be removed as well.
Benny I'll send a squashme. (I inserted simulated errors to test the error path)
> Note however that the commit code takes references in the file driver code.
>
We don't do commit in objects yet, and Benny did not implement the
pnfs_ld_commit_done, yet. But we will need it eventually.
I hope that the ref taken by the file driver is balance by the driver
and not by generic code, right? If not, we might need to fix that somewhere.
> Fred
>
Thanks
>> Boaz
>>
>>>> + dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
>>>> + data->pnfs_error);
>>>> + status = nfs_initiate_write(data, NFS_CLIENT(data->inode),
>>>> + data->mds_ops, NFS_FILE_SYNC);
>>>> + return status ? : -EAGAIN;
>>>> +}
--
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
