Re: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release

[Peng Tao <bergwolf@gmail.com>]

On Sat, Sep 10, 2011 at 1:46 AM, Vitaliy Gusev
<gusev.vitaliy@nexenta.com> wrote:
>>>> Reported-by: Vitaliy Gusev<gusev.vitaliy@nexenta.com>
>>>> Signed-off-by: Peng Tao<peng_tao@emc.com>
>>>> ---
>>>>  fs/nfs/nfs4proc.c |    2 ++
>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>> index 8c77039..da7c20c 100644
>>>> --- a/fs/nfs/nfs4proc.c
>>>> +++ b/fs/nfs/nfs4proc.c
>>>> @@ -5964,6 +5964,7 @@ static void nfs4_layoutcommit_release(void
>>>> *calldata)
>>>>        struct pnfs_layout_segment *lseg, *tmp;
>>>>
>>>>        pnfs_cleanup_layoutcommit(data);
>>>> +       spin_lock(&data->args.inode->i_lock);
>>>
>>> I think lock over list_del_init(&lseg->pls_lc_list) is enough, because
>>
>> I put the spinlock outside of the loop because the critical section is
>> short enough and it should be faster than grabbing/dropping the inode
>> lock for every entry in the list, agree?
>
> Yes, you are right.
>
> Looked again I saw that issue is not so bad as seems.
> Really, what is result of this issue? Only that lseg is in data->lseg_list,
> but without set NFS_LSEG_LAYOUTCOMMIT. The put_lseg is called correctly at
> nfs4_layoutcommit_release. So there is no any bug.
Yes, you are right. In that case, we will have one lseg in the lc_list
list w/o NFS_LSEG_LAYOUTCOMMIT set but it doesn't hurt anyone. The
above patch of mine is not really necessary.

Thanks,
Tao