On 05/07/2014 11:21 AM, Liu Bo wrote:
On Sun, Feb 09, 2014 at 11:45:12PM +0000, Filipe David Borba Manana wrote:
This is an extension to my previous commit titled:
"Btrfs: faster file extent item replace operations"
(hash 1acae57b161ef1282f565ef907f72aeed0eb71d9)
Instead of inserting the new file extent item if we deleted existing
file extent items covering our target file range, also allow to insert
the new file extent item if we didn't find any existing items to delete
and replace_extent != 0, since in this case our caller would do another
tree search to insert the new file extent item anyway, therefore just
combine the two tree searches into a single one, saving cpu time, reducing
lock contention and reducing btree node/leaf COW operations.
This covers the case where applications keep doing tail append writes to
files, which for example is the case of Apache CouchDB (its database and
view index files are always open with O_APPEND).
(I'm tracking a bug which is very hard to reproduce and the stack seems to
locate on this area.)
Even I know that this has been merged, I still have to say that this just
makes the code nearly hard-to-maintained.
__btrfs_drop_extents() has already been one of the most complex function since
it was written, but now it's become more and more complex!
I'm not sure whether the gained performance number deserves that kind of
complexity, man, to be honest, try to ask yourself how much time you'll spend in
re-understanding the code and all the details.
It's just a complex operation anyway, so really it's going to suck no
matter what. What I would like to see is some sanity tests committed
that test the various corner cases of btrfs_drop_extents so when we make
these sort of changes we can be sure we're not breaking anything.
So in fact that's the new requirement, whoever wants to touch
btrfs_drop_extents next has to make sanity tests for it first, and then
they can do what they want, this includes cleaning it up. Thanks,
Josef
--
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
