Hello Joseph, On 09/14/2016 04:13 PM, Joseph Qi wrote: > Hi Eric, > > On 2016/9/14 15:57, Eric Ren wrote: >> Hello Joseph, >> >> Thanks for fixing up this. >> >> On 09/14/2016 12:15 PM, Joseph Qi wrote: >>> If ocfs2_reserve_cluster_bitmap_bits fails with ENOSPC, it will try to >>> free truncate log and then retry. Since ocfs2_try_to_free_truncate_log >>> will lock/unlock global bitmap inode, we have to unlock it before >>> calling this function. But when retry reserve and it fails with no >> You mean the retry succeeds by "retry reserve", right? I fail to understand >> in which situation >> the retry will fail to get global bitmap inode lock. Because I didn't see >> this problem when I >> tested my patch, could you explain a bit more? >> >> Eric > Before retry it has inode unlocked, but ac inode is still valid. And > if inode lock fails this time, it will goto bail and do inode unlock > again. Yeah, I see this point, thanks. I am also wondering when we will fail to lock the global bitmap inode?
BTW, I'm guessing you mean "retry deserves" by "retry reserve"? Eric > > Thanks, > Joseph > >>> global bitmap inode lock taken, it will unlock again in error handling >>> branch and BUG. >>> This issue also exists if no need retry and then ocfs2_inode_lock fails. >>> So fix it. >>> >>> Fixes: 2070ad1aebff ("ocfs2: retry on ENOSPC if sufficient space in >>> truncate log" >>> Signed-off-by: Jospeh Qi <joseph...@huawei.com> >>> Signed-off-by: Jiufei Xue <xuejiu...@huawei.com> >>> --- >>> fs/ocfs2/suballoc.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c >>> index ea47120..041453b 100644 >>> --- a/fs/ocfs2/suballoc.c >>> +++ b/fs/ocfs2/suballoc.c >>> @@ -1199,14 +1199,23 @@ retry: >>> inode_unlock((*ac)->ac_inode); >>> >>> ret = ocfs2_try_to_free_truncate_log(osb, bits_wanted); >>> - if (ret == 1) >>> + if (ret == 1) { >>> + iput((*ac)->ac_inode); >>> + (*ac)->ac_inode = NULL; >>> goto retry; >>> + } >>> >>> if (ret < 0) >>> mlog_errno(ret); >>> >>> inode_lock((*ac)->ac_inode); >>> - ocfs2_inode_lock((*ac)->ac_inode, NULL, 1); >>> + status = ocfs2_inode_lock((*ac)->ac_inode, NULL, 1); >>> + if (status < 0) { >>> + inode_unlock((*ac)->ac_inode); >>> + iput((*ac)->ac_inode); >>> + (*ac)->ac_inode = NULL; >>> + goto bail; >>> + } >>> } >>> if (status < 0) { >>> if (status != -ENOSPC) >> >> >> . >> > > _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel