On Tue, May 17, 2016 at 03:20:16PM +0200, David Sterba wrote: > On Fri, Apr 01, 2016 at 02:35:01PM +0800, Qu Wenruo wrote: > > @@ -5815,6 +5817,23 @@ out_fail: > > } > > if (delalloc_lock) > > mutex_unlock(&BTRFS_I(inode)->delalloc_mutex); > > + /* > > + * The number of metadata bytes is calculated by the difference > > + * between outstanding_extents and reserved_extents. Sometimes though > > + * reserve_metadata_bytes() fails to reserve the wanted metadata bytes, > > + * indeed it has already done some work to reclaim metadata space, hence > > + * both outstanding_extents and reserved_extents would have changed and > > + * the bytes we try to reserve would also has changed(may be smaller). > > + * So here we try to reserve again. This is much useful for online > > + * dedupe, which will easily eat almost all meta space. > > + * > > + * XXX: Indeed here 3 is arbitrarily choosed, it's a good workaround for > > + * online dedupe, later we should find a better method to avoid dedupe > > + * enospc issue. > > + */ > > + if (unlikely(ret == -ENOSPC && loops++ < 3)) > > + goto again; > > + > > This does not seem right and needs to be addressed properly before I > consider adding the patchset to for-next. I don't have idea how to fix > it. Agreed, and this sort of issue is a reason why I strongly feel we don't want to merge this series piecemeal until we know that after everything is complete, we can end up with a fully baked in-band dedupe implementation. Luckily Qu says he's on it so if he posts a workable fix here my whole point can become moot. Until then though this is exactly the type of 'fix later' coding we need to be avoiding. --Mark -- Mark Fasheh -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
