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? thanks, liubo -- 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
