Hi Miao, thank you for addressing the issue. I will try out this 5/5 patchset and let you know what I see. I will apply it on top of latest for-linus branch.
Thanks, Alex. On Wed, Jan 23, 2013 at 10:58 AM, Miao Xie <mi...@cn.fujitsu.com> wrote: > On wed, 23 Jan 2013 16:17:53 +0800, Liu Bo wrote: >> On Wed, Jan 23, 2013 at 02:33:27PM +0800, Miao Xie wrote: >>> On wed, 23 Jan 2013 14:06:21 +0800, Liu Bo wrote: >>>> On Wed, Jan 23, 2013 at 12:44:49PM +0800, Miao Xie wrote: >>>>> No, we can't. The other tasks which flush the delalloc data may remove >>>>> the inode >>>>> from the delalloc list/splice list. If we release the lock, we will meet >>>>> the race >>>>> between list traversing and list_del(). >>>> >>>> OK, then please merge patch 1 and 4 so that we can backport 1 less patch >>>> at least. >>> >>> I don't think we should merge these two patch because they do two different >>> things - one >>> is bug fix, and the other is just a improvement, and this improvement >>> changes the logic >>> of the code and might be argumentative for some developers. So 2 patches is >>> better than one, >>> I think. >> >> Sorry, this is right only when patch 1 really fixes the problem Alex >> reported. >> >> But the fact is >> >> 1) patch 1 is not enough to fix the bug, it just fixes the OOM of >> allocating 'struct btrfs_delalloc_work' while the original OOM of allocating >> requests remains. We can still get the same inode over and over again >> and then stuck in btrfs_start_delalloc_inodes() because we 'goto again' to >> make sure we flush all inodes listed in fs_info->delalloc_inodes. >> >> 2) patch 4 fixes 1)'s problems by removing 'goto again'. >> >> Am I missing something? > > In fact, there are two different problems. > One is OOM bug. it is a serious problem and also is an regression, so we > should fix it > as soon as possible. > The other one is that we may fetch the same inode again and again if someone > write data > into it endlessly. This problem is not so urgent to deal with. and perhaps > some developers > think it is not a problem, we should flush that inode since there are dirty > pages in it. > So we need split it from the patch of the 1st problem. > > Thanks > Miao -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
