David Sterba wrote on 2016/05/17 15:20 +0200:
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.


Right, the code is bad, just as Josef pointed out.

In fact this behavior really hides a lot of ENOSPC problem and shows quite a lot original metadata reserve limitation of current code.
(Mostly hidden by the default 128M max extent size)

We are actively debugging and testing new root fix for the problem.

Thanks,
Qu


--
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

Reply via email to