On Sat, Sep 10, 2011 at 1:46 AM, Vitaliy Gusev <gusev.vitaliy@xxxxxxxxxxx> wrote: >>>> Reported-by: Vitaliy Gusev<gusev.vitaliy@xxxxxxxxxxx> >>>> Signed-off-by: Peng Tao<peng_tao@xxxxxxx> >>>> --- >>>> 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 -- 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