On Mon, Feb 14, 2011 at 9:58 PM, Ted Ts'o <tytso@xxxxxxx> wrote: > -lsf-pc, -linux-fsdevel > > On Mon, Feb 14, 2011 at 09:00:58PM +0200, Amir Goldstein wrote: >> Yes, of course. Upgraders won't be the ones using snapshots. >> My intension was to state that those people installing new systems to test >> snapshots would be functioning as testers for "ext3 mode", because: >> 1. when no snapshots exists it boils down to testing "ext3 mode". >> 2. it is unlikely that snapshots will mask "ext3 mode" bugs. >> >> So my claim is that "ext3 mode" would benefit from a transition >> period in which snapshots and (extens,delalloc) are mutually >> exclusive in ext4. > > Here are the requirements that I think are critical before we do this: > > 1) We need to solve the testing matrix problem. Right now "ext3 mode" > in ext4 doesn't get enough testing as it is. Part of the solution is > (a) deciding on the modes that need testing, and (b) writing some > shell scripts so that xfstests can be automatically run in all of the > right modes. And then it will be having some number of people > (hopefully not just me) running said tests and reporting failures. > > 2) The code has to integrate in a fairly seemless and easy way. > mballoc.c is an example of code that still needs a lot of cleanup. > Coly Li has submitted some cleanups, which is great. But I suspect a > lot more is needed. > > One thing that comes to mind about your question with the > e4b->alloc_semp causing problems. If the only reason why we need it > is to protect against multiple attempts to initialize different block > groups that share the same buddy bitmap, can we solve the problem by > ditching e4b->alloc_semp entirely, and simply using lock_page() on the > buddy bitmap page to protect it? > Perhaps. I imagine there is more than one elegant way to deal with that, but using a semaphore is not one of them. I will take a shot at evaporating e4b->alloc_semp. > That's an example of the radical code cleanup and simplification that > parts of the ext4 codebase could really use. That isn't the > snapshot's code fault, and if we didn't really need to touch parts the > code in question, it's probably stable enough as it is. > Unfortunately, if you need to make changes, there's enough code debt > in some of the files that you need to change that any changes _has_ to > make things better, and not worse. So for example, checking to see if > the blocksize==page_size, and then skipping the down_read(alloc_smp) > call is an example of layering _more_ complexity and code hackery, and > not less. > Fair enough. I accept the challenge. I shall cleanup mballoc.c in the process of merging snapshots code. If you have specific things that bug you in mballoc.c, let me know. > Note what I did with patches in the ext4_da_writepages() codepath --- > about 100 lines of code removed in just 7 patches, and I expect > performance will get better as a result of the cleanup. And then > compare that to how that code looked in say, 2.6.27. We need to do > similar amounts of cleanup in other parts of ext4 --- and mballoc.c is > by no means the worse. But building on top of code which has a fair > amount of code debt, is not a receipe for long-term success; it's like > building a castle on quicksand, or in a swamp (insert obligatory Monty > Python reference here). > > - Ted > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
