| Message ID | x2vac8f92701003280113k2458fcb6mefe6c04875c9b3a3@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jing, If you're wondering why I'm taking a long time to respond to your patches, it is because they are very ill-formed. They don't conform to the submitting patches guidelines, the patch comments don't adequately explain the why the patch is needed, and what testing has been done, and you tend to throw in patches that aren't correctly submitted in the middle of comments, and in some cases it's not clear whether this patch is suppose to be in addition to the previous patch, and combined into a separate commit, or kept as two separate patches, etc. As a result, it takes, much, MUCH, MUCH longer for me to review the patches for correctness. I will eventually get to them, but I may end up working on other patches which are better formed and easier for me to evaluate for correctness and quality. If you do submit new patches, especially in this thread where you have already submitted so many different patches, I would appreicate it if you could explicitly state that a particular patch has been superceded by another, or has been withdrawn. I will try to keep score on the patchwork web site (for example, I rejected your bb_free_cache patch), but you've been so prolific with patches, some of which haven't been very well explained, that I may have lost track of all of your submissions. Please bear with me. Best regards, - Ted On Sun, Mar 28, 2010 at 04:13:51PM +0800, jing zhang wrote: > 2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>: > > On Tue, 23 Mar 2010 20:47:39 +0800, jing zhang <zj.barak@gmail.com> wrote: > >> 2010/3/22, tytso@mit.edu <tytso@mit.edu>: > >> > On Sun, Mar 21, 2010 at 10:01:06PM +0800, jing zhang wrote: > >> >> From: Jing Zhang <zj.barak@gmail.com> > >> >> > >> >> Date: Sun Mar 21 21:59:35 2010 > >> >> > >> >> Cc: Theodore Ts'o <tytso@mit.edu> > >> >> Cc: Andreas Dilger <adilger@sun.com> > >> >> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com> > >> >> Signed-off-by: Jing Zhang <zj.barak@gmail.com> > >> >> > >> >> --- > >> >> > >> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800 > >> >> +++ ext4_mm_leak/mballoc3.c 2010-03-21 21:37:18.000000000 +0800 > >> >> @@ -2360,6 +2360,24 @@ err_freesgi: > >> >> return -ENOMEM; > >> >> } > >> >> > >> >> +static void ext4_mb_destroy_backend(struct super_block *sb) > >> >> +{ > >> >> + ext4_group_t ngroups = ext4_get_groups_count(sb); > >> >> + ext4_group_t i; > >> >> + struct ext4_sb_info *sbi = EXT4_SB(sb); > >> >> + int j; > >> >> + int num_meta_group_infos = (ngroups + EXT4_DESC_PER_BLOCK(sb) -1) > >> >> + >> EXT4_DESC_PER_BLOCK_BITS(sb); > >> >> + for (i = 0; i < ngroups; i++) > >> >> + kfree(ext4_get_group_info(sb, i)); > >> >> + > >> >> + for (j = 0; j < num_meta_group_infos; j++) > >> >> + kfree(sbi->s_group_info[j]); > >> >> + > >> >> + kfree(sbi->s_group_info); > >> >> + iput(sbi->s_buddy_cache); > >> >> +} > >> >> + > >> > > >> > It would be better if this could be done by making ext4_mb_release() > >> > more flexible, and then calling ext4_mb_release() if there is an error > >> > setting up the data structures in ext4_mb_init(). > >> > > >> > - Ted > >> > > >> > >> Yeah, Ted, going through ext4_mb_release() is clearer. > >> > >> --- > >> > >> diff --git a/linux-2.6.32/fs/ext4/mballoc.c b/ext4_mm_leak/mballoc3.c > >> index bba1282..99ca2de 100644 > >> --- a/linux-2.6.32/fs/ext4/mballoc.c > >> +++ b/ext4_mm_leak/mballoc3.c > >> @@ -2417,8 +2417,7 @@ int ext4_mb_init(struct super_block *sb, int > >> needs_recovery) > >> > >> sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group); > >> if (sbi->s_locality_groups == NULL) { > >> - kfree(sbi->s_mb_offsets); > >> - kfree(sbi->s_mb_maxs); > >> + ext4_mb_release(sb); > > > > We may want to make sure that we can safely call ext4_mb_release that > > early. what i would suggest is to move s_locality_group allocation > > before ext4_mb_init. that makes error handling easy > > > > cool idea, Aneesh, and I got it. > > --- > > --- old-linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800 > +++ ext4_mm_leak/mballoc3-2.c 2010-03-28 16:13:20.000000000 +0800 > @@ -2397,11 +2397,27 @@ int ext4_mb_init(struct super_block *sb, > i++; > } while (i <= sb->s_blocksize_bits + 1); > > + sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group); > + if (sbi->s_locality_groups == NULL) { > + kfree(sbi->s_mb_offsets); > + kfree(sbi->s_mb_maxs); > + return -ENOMEM; > + } > + for_each_possible_cpu(i) { > + struct ext4_locality_group *lg; > + lg = per_cpu_ptr(sbi->s_locality_groups, i); > + mutex_init(&lg->lg_mutex); > + for (j = 0; j < PREALLOC_TB_SIZE; j++) > + INIT_LIST_HEAD(&lg->lg_prealloc_list[j]); > + spin_lock_init(&lg->lg_prealloc_lock); > + } > + > /* init file for buddy data */ > ret = ext4_mb_init_backend(sb); > if (ret != 0) { > kfree(sbi->s_mb_offsets); > kfree(sbi->s_mb_maxs); > + free_percpu(sbi->s_locality_groups); > return ret; > } > > @@ -2415,20 +2431,6 @@ int ext4_mb_init(struct super_block *sb, > sbi->s_mb_order2_reqs = MB_DEFAULT_ORDER2_REQS; > sbi->s_mb_group_prealloc = MB_DEFAULT_GROUP_PREALLOC; > > - sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group); > - if (sbi->s_locality_groups == NULL) { > - kfree(sbi->s_mb_offsets); > - kfree(sbi->s_mb_maxs); > - return -ENOMEM; > - } > - for_each_possible_cpu(i) { > - struct ext4_locality_group *lg; > - lg = per_cpu_ptr(sbi->s_locality_groups, i); > - mutex_init(&lg->lg_mutex); > - for (j = 0; j < PREALLOC_TB_SIZE; j++) > - INIT_LIST_HEAD(&lg->lg_prealloc_list[j]); > - spin_lock_init(&lg->lg_prealloc_lock); > - } > > if (sbi->s_proc) > proc_create_data("mb_groups", S_IRUGO, sbi->s_proc, > > > > > > > >> return -ENOMEM; > >> } > >> for_each_possible_cpu(i) { > >> @@ -2511,7 +2510,8 @@ int ext4_mb_release(struct super_block *sb) > >> atomic_read(&sbi->s_mb_discarded)); > >> } > >> > >> - free_percpu(sbi->s_locality_groups); > >> + if (sbi->s_locality_groups) > >> + free_percpu(sbi->s_locality_groups); > >> if (sbi->s_proc) > >> remove_proc_entry("mb_groups", sbi->s_proc); > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -aneesh > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/4, tytso@mit.edu <tytso@mit.edu>: > Hi Jing, > > If you're wondering why I'm taking a long time to respond to your > patches, it is because they are very ill-formed. They don't conform > to the submitting patches guidelines, the patch comments don't > adequately explain the why the patch is needed, and what testing has > been done, and you tend to throw in patches that aren't correctly > submitted in the middle of comments, and in some cases it's not clear > whether this patch is suppose to be in addition to the previous patch, > and combined into a separate commit, or kept as two separate patches, > etc. > Sorry again, Ted. There are so much for a newbie to learn to do cool work, especially to meet your requirements. I am happy, any way, to patch ext4 under your direction. > As a result, it takes, much, MUCH, MUCH longer for me to review the > patches for correctness. I will eventually get to them, but I may end > up working on other patches which are better formed and easier for me > to evaluate for correctness and quality. > > If you do submit new patches, especially in this thread where you have > already submitted so many different patches, I would appreicate it if > you could explicitly state that a particular patch has been superceded Without the cool git, though I am learning how to take advantage of it, I could not manage all the patches delivered. In fact, I dig the patches with UltraEdit for modifying the C code, Cygwin for git and diff -Npu, and virtual machine for compiling. My kid, 11 years old boy, has to share the HP notebook with me playing games. And please laugh at me, I am not living in stone age. I resolve conflicts and dependencies between patches in the way that they are carried out by independent guys, since I am told that git is cool enough. But indeed I created so many hard work for you, sorry. Is it possible for me to patch not based upon the stock version I downloaded at kernel.org, but upon the patched version, say the latest git tree? > by another, or has been withdrawn. I will try to keep score on the > patchwork web site (for example, I rejected your bb_free_cache patch), > but you've been so prolific with patches, some of which haven't been > very well explained, that I may have lost track of all of your > submissions. > > Please bear with me. No problem. I like ext4, maybe still the native file system of GNU Linux, and the cool guys such as Alen Cox, Andy Clin, Ingo Molnar, Rusty Russell, Jens Axboe, Alexey N. Kuznetsov (where r u now, ANK?), Mike Haertel, Avi Kivity, Ted, Reiser, and their cool works which help many newbies to understand Linux, to apply the cool ideas and methods they learned in Linux to their everyday work, to earn bread and salt in this hard time. One of the amazing cools of Linux, I believe, besides free, is to change ideas and lives of individuals, and to change what should be changed in the society in which individuals live. I am being changed to do patch in appreciated way. Thanks - zj > > Best regards, > > - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Apr 04, 2010 at 09:05:14AM +0800, jing zhang wrote: > > Without the cool git, though I am learning how to take advantage of > it, I could not manage all the patches delivered. In fact, I dig the > patches with UltraEdit for modifying the C code, Cygwin for git and > diff -Npu, and virtual machine for compiling. My kid, 11 years old > boy, has to share the HP notebook with me playing games. How much testing are you doing before submitting patches, out of curiosity? > I resolve conflicts and dependencies between patches in the way that > they are carried out by independent guys, since I am told that git is > cool enough. But indeed I created so many hard work for you, sorry. Having independent patches is actually better --- but I think you're misunderstanding what I was complaining about before. Patches should that are accepted into mainline should do one and only one thing. So if someone suggests that you make changes to your submitted patch, ideally what you should do is to resubmit the patch with the fixes --- and not submit a patch which is a delta to the previous one. This is especially true if the original patch is buggy; one of the things we try very hard to maintain is that the kernel tree compile cleanly, and pass the regression test suite, between every single commit. In other words, we try to avoid knowingly introducing a regression in a patch and fixing it in a subsequent patch. This allows things like "git bisect" to work, and it also makes it easier for people to look at the commit history to understand why certain changes were made, and especially when trying to find how a bug was introduced into ext4. Ultimately, this is about keeping the kernel source code easily maintainable. This means that incrased code complexity has to be justified, and code and code changes have to be meticulously documented. Best regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, tytso@mit.edu <tytso@mit.edu>: > On Sun, Apr 04, 2010 at 09:05:14AM +0800, jing zhang wrote: > > How much testing are you doing before submitting patches, out of > curiosity? Yes, Ted, it is curiosity that drives me to do hard works, including patch ext4. I already told you that I am happy to patch ext4. I also like to share/deliver my patches to the developers and maintainers of ext4, simply because I find something not correct or perfect in the version I got at kernel.org, and because I am not 100% sure. And review is necessary, as described in Andi Kleen's "On submitting kernel patches", which I read last night. And I got that on cmdline, git --stat --summary a/x.c b/x.c > x-01.diff may get nice output, correct? It is hard work for me, a newbie, though curious, to do perfect patch, and I try my best to describe the patch in English, which is not my native language but one of the most beautiful languages on earth. What is behind my curiosity is the belief that I will be freed both by Jesus in West and by Buddha in East, today or tomorrow, if I deliver what I can to those who need. And after operations on cmdline, I compile the modified, modprobe, dd, and rmmod with virtual machine. It is not hard. > Having independent patches is actually better --- but I think you're > misunderstanding what I was complaining about before. Patches should Whatever you complain, I try to be a good listener every time:) GNU Linux is free, is MIT free? And I am free to change, to be responsible for what I did, or maybe freed by you. > that are accepted into mainline should do one and only one thing. So > if someone suggests that you make changes to your submitted patch, > ideally what you should do is to resubmit the patch with the fixes --- > and not submit a patch which is a delta to the previous one. > > This is especially true if the original patch is buggy; one of the > things we try very hard to maintain is that the kernel tree compile > cleanly, and pass the regression test suite, between every single > commit. In other words, we try to avoid knowingly introducing a > regression in a patch and fixing it in a subsequent patch. This > allows things like "git bisect" to work, and it also makes it easier > for people to look at the commit history to understand why certain > changes were made, and especially when trying to find how a bug was > introduced into ext4. Ultimately, this is about keeping the kernel > source code easily maintainable. This means that incrased code > complexity has to be justified, and code and code changes have to be > meticulously documented. I think what is called ext4 will change, and more will be freed by the change, since it is under your maintenance, at least currently, a model of maintainer, especially of strict requirement and kind patience to patches received. Good weekend, and please review my new patches next week. Thank you, Ted. - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
jing zhang wrote: > 2010/4/5, tytso@mit.edu <tytso@mit.edu>: >> On Sun, Apr 04, 2010 at 09:05:14AM +0800, jing zhang wrote: >> >> How much testing are you doing before submitting patches, out of >> curiosity? > > Yes, Ted, it is curiosity that drives me to do hard works, including patch ext4. It is the language barrier that is making some of this difficult, but I'm not complaining - you speak English much better than I speak any second language. :) Ted meant that -he- was curious about how much testing you were doing. ... > And after operations on cmdline, I compile the modified, modprobe, dd, > and rmmod with virtual machine. It is not hard. More testing than this would be good; dd is very minimal. One of our new standard tests for et4 is the xfstests test suite from http://git.kernel.org/?p=fs/xfs/xfstests-dev.git;a=summary It is a collection of many tests developed for xfs, but many tests are generic and can run on ext4 as well. I would suggest that after you have several patches ready, you should at least run through the tests in this collection. It won't catch every mistake but it runs a large variety of tests, much more stressful than dd. Thanks for your email, and thanks for clearly spending time looking for ways to improve ext4. I think that with practice, you will be a good contributor. Ted can certainly be a patient maintainer - read his suggestions and the kernel patch submission guidelines, and I think you will get better at this. Do your best to explain the reasons for your patches, and any testing you have done, and describe any test which can show a bug that you find - and we can help to clarify changelogs if they need it. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, Eric Sandeen <sandeen@redhat.com>: > jing zhang wrote: >> 2010/4/5, tytso@mit.edu <tytso@mit.edu>: >>> On Sun, Apr 04, 2010 at 09:05:14AM +0800, jing zhang wrote: >>> >>> How much testing are you doing before submitting patches, out of >>> curiosity? >> >> Yes, Ted, it is curiosity that drives me to do hard works, including patch >> ext4. > > It is the language barrier that is making some of this difficult, > but I'm not complaining - you speak English much better than I speak any You are good guy:) Is English your native language? And, I am curious, what is the second language you are able to speak, Eric? > second language. :) > > Ted meant that -he- was curious about how much testing you were doing. How do know what Ted meant, by iphone? > > ... > >> And after operations on cmdline, I compile the modified, modprobe, dd, >> and rmmod with virtual machine. It is not hard. > > More testing than this would be good; dd is very minimal. > > One of our new standard tests for et4 is the xfstests test suite from > http://git.kernel.org/?p=fs/xfs/xfstests-dev.git;a=summary > > It is a collection of many tests developed for xfs, but many tests are > generic and can run on ext4 as well. I would suggest that after you have > several patches ready, you should at least run through the tests in this > collection. It won't catch every mistake but it runs a large variety of > tests, much more stressful than dd. Thanks for your good advice. > > Thanks for your email, and thanks for clearly spending time looking for > ways to improve ext4. I think that with practice, you will be a good > contributor. > > Ted can certainly be a patient maintainer - read his suggestions and the > kernel patch submission guidelines, and I think you will get better at this. > > Do your best to explain the reasons for your patches, and any testing you > have done, and describe any test which can show a bug that you find - > and we can help to clarify changelogs if they need it. > I am not good at testing, partially because it is hard to setup the required environment, sometimes several hard disks are needed, maybe a few boxes, but I try to analyse the C code while reading and understanding the works by great maintainers and developers of Linux kernel. - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
jing zhang wrote: > 2010/4/5, Eric Sandeen <sandeen@redhat.com>: >> jing zhang wrote: >>> 2010/4/5, tytso@mit.edu <tytso@mit.edu>: >>>> On Sun, Apr 04, 2010 at 09:05:14AM +0800, jing zhang wrote: >>>> >>>> How much testing are you doing before submitting patches, out of >>>> curiosity? >>> Yes, Ted, it is curiosity that drives me to do hard works, including patch >>> ext4. >> It is the language barrier that is making some of this difficult, >> but I'm not complaining - you speak English much better than I speak any > > You are good guy:) Is English your native language? And, I am curious, > what is the second language you are able to speak, Eric? Only a little German. >> second language. :) >> >> Ted meant that -he- was curious about how much testing you were doing. > > How do know what Ted meant, by iphone? No, because I am a native English speaker and I understood the figure of speech. ("out of curiosity") > I am not good at testing, partially because it is hard to setup the > required environment, sometimes several hard disks are needed, maybe a > few boxes, but I try to analyse the C code while reading and > understanding the works by great maintainers and developers of Linux > kernel. Testing really is critical to development; some things can be done by inspection, but if you don't test it is hard to know if you made a mistake. You can always test inside a vm, or on a loopback file, on a single box. Without testing, you are asking others to do testing for you (unless the change is so obvious that it can be trusted) -Eric > - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, Eric Sandeen <sandeen@redhat.com>: > Testing really is critical to development; some things can be done by > inspection, but if you don't test it is hard to know if you made a > mistake. > > You can always test inside a vm, or on a loopback file, on a single box. > > Without testing, you are asking others to do testing for you > (unless the change is so obvious that it can be trusted) > Before linux-2.6.32 is released, would you like tell me, how is ext4 tested? Is tough testing able to catch all bugs? - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, Eric Sandeen <sandeen@redhat.com>: >>> >>> Ted meant that -he- was curious about how much testing you were doing. >> >> How do know what Ted meant, by iphone? > > No, because I am a native English speaker and I understood the > figure of speech. ("out of curiosity") > And I am also curious about what educational degree did you win, Eric, on the language of English, at Harvard University? - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 05, 2010 at 01:08:58PM +0800, jing zhang wrote: > > Before linux-2.6.32 is released, would you like tell me, how is ext4 tested? > Is tough testing able to catch all bugs? For one thing, we use the XFSQA (aka xfstests) test suite. I usually run the "quick" set of tests between each commit, and I run a much longer "auto" test suite before submitting a set of patches to Linus. For specific patches, we also like to be able to test it specifically. So often we'll create a small filesystem image and then see what the file system does with and without the patch. For example, if we're worried that a specific file system corruption will cause a BUG_ON, we'll create a small file system, corrupt it using the debugfs tool, and then demonstrate that without the patch, we get the BUG_ON. With the patch applied, the BUG_ON should go away. So the patch-specific tests are there so we can make sure the patch does what we think it should, and it fixes the problem it claims to fix. The XFSQA test suite is to there to make sure that we don't accidentally introduce bugs while trying to improve the file system. And of course, the final safeguard is patch review by others. This is where decent comments and making sure the code is maintainable is critically important. Since this takes other developer's time, especially senior developers like Eric and myself's time, which is rather valuable and hard to find, it's why patch submitters need shoulder more of the work in terms of refining patches and resubmitting new versions of the patches. - Ted P.S. Yes, Eric was absolutely correct. I wanted to know how much testing you had been doing on your patches before submitting them. At least few of your patches have caused me to wonder whether the patches had gone through sufficient testing. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 05, 2010 at 01:18:58PM +0800, jing zhang wrote: > 2010/4/5, Eric Sandeen <sandeen@redhat.com>: > >>> > >>> Ted meant that -he- was curious about how much testing you were doing. > >> > >> How do know what Ted meant, by iphone? > > > > No, because I am a native English speaker and I understood the > > figure of speech. ("out of curiosity") > > > > And I am also curious about what educational degree did you win, Eric, > on the language of English, at Harvard University? Jing, That's uncalled for. Please be polite. Eric is a native speaker of English, and he quite correctly interpreted my question. Thanks, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, tytso@mit.edu <tytso@mit.edu>: > On Mon, Apr 05, 2010 at 01:08:58PM +0800, jing zhang wrote: >> >> Before linux-2.6.32 is released, would you like tell me, how is ext4 >> tested? >> Is tough testing able to catch all bugs? > > For one thing, we use the XFSQA (aka xfstests) test suite. I usually > run the "quick" set of tests between each commit, and I run a much > longer "auto" test suite before submitting a set of patches to Linus. > > For specific patches, we also like to be able to test it specifically. > So often we'll create a small filesystem image and then see what the > file system does with and without the patch. For example, if we're > worried that a specific file system corruption will cause a BUG_ON, > we'll create a small file system, corrupt it using the debugfs tool, > and then demonstrate that without the patch, we get the BUG_ON. With > the patch applied, the BUG_ON should go away. > > So the patch-specific tests are there so we can make sure the patch > does what we think it should, and it fixes the problem it claims to > fix. The XFSQA test suite is to there to make sure that we don't > accidentally introduce bugs while trying to improve the file system. > > And of course, the final safeguard is patch review by others. This is First, Mr. Theodore Ts'o, your reply is a great honor to me. And I learn more about ext4 and its maintainers and developers, specially that testing is important to ext4. Thanks. In my view, one of the important parts of a patch is that the patcher is really concerning what is patched, another is whether the questions listed in the patch do exist, and whether the solution, if provided by the patcher, is correct. Even if the provide solution is ok, I think, it is the privilege, responsibility and duty of maintainers to execute the final submitting, and therefore it is not the duty of patchers to make sure that the solution is 100% correct and perfect, testing is good or not. I like to be a maintainer of some subsystem of GNU Linux, since it is my shame that end users experience panic, crash, bug, bug_on caused by what is under my maintenance, that end users are treated as rats and monkeys in laboratory even though I am not paid by any end user, and the nice reputations of distributors, say red hat, of service providers, say IBM. To be a maintainer, I will apologize on my homepage once a patch received, if the questions listed in the patch do exist, simply because I abuse my duty and time. > where decent comments and making sure the code is maintainable is > critically important. Since this takes other developer's time, > especially senior developers like Eric and myself's time, which is I still concern how to correctly measure the lost, in the next half of 2010, of time and value of end users, say 100, caused by what is buggy in ext4. Are you sure no buggy, Eric and Ted? > rather valuable and hard to find, it's why patch submitters need > shoulder more of the work in terms of refining patches and > resubmitting new versions of the patches. > > - Ted > > P.S. Yes, Eric was absolutely correct. I wanted to know how much Do you like opera, Mr. Theodore Ts'o? If you like, I invite you Beijing Opera and tea, and I pay the air ticket, double trip, from Los Angeles to Beijing. And please info me your time schedule if you accept my invitation. > testing you had been doing on your patches before submitting them. At > least few of your patches have caused me to wonder whether the patches > had gone through sufficient testing. > If end users of ext4 can not be treated, for any reason, as rats and monkeys, lets go ahead, please, focusing upon patch. - zj -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 06, 2010 at 09:43:58PM +0800, jing zhang wrote: > > In my view, one of the important parts of a patch is that the patcher > is really concerning what is patched, another is whether the questions > listed in the patch do exist, and whether the solution, if provided by > the patcher, is correct. > > Even if the provide solution is ok, I think, it is the privilege, > responsibility and duty of maintainers to execute the final > submitting, and therefore it is not the duty of patchers to make sure > that the solution is 100% correct and perfect, testing is good or not. Well, the problem is there are many more patch submitters than there are maintainers, and so your proposal simply doesn't scale. Consider that for some maintainers, there may be 10 or 20 or 30 or more patch submitters in their subsystem. With that kind of submitter-to-maintainer ratio, the patch submitter simply has to do much more of the work, since otherwise the subsystem maintainer simply can't keep up. Most maintainers actually spend much less time dealing with patch submitters than I have invested with you. They simply send a NACK, and maybe 2-3 sentences explaning of what still needs fixing, and then it's up to the patch submitter to resubmit the patch. Some patches are resubmitted 5, 6 times before the maintainer finally accepts it. I happen to believe that we need to encourage newcomers to the kernel developer community, and so I spend more time mentoring people who are new to the process. But at the end of the day, I have only so many hours that I can spend on newcomers, and so after a while, I will start serving other patch submitters who are more capable of providing patches that are easy to review and integrate. > I like to be a maintainer of some subsystem of GNU Linux, since it is > my shame that end users experience panic, crash, bug, bug_on caused by > what is under my maintenance, that end users are treated as rats and > monkeys in laboratory even though I am not paid by any end user, and > the nice reputations of distributors, say red hat, of service > providers, say IBM. Sure, but that means the patches which are submitted has to be high quality. If I get half-tested patches, or patches where you can't give me a reproduction case that demonstrates the BUG_ON, and it's not obvious whether or not the patch is safe, I have to worry about the possibility that applying your patch may make the the file system more likely to crash, not less. Ext4 is actually quite stable at this point. Very large numbers of people are using it, and most users are quite happy. So at this point, I have to weigh the risk that a patch might introduce a bug against the claim that it fixes the bug --- especially if the patch submitter can't explain how the BUG_ON might have been triggered without their patch, and can't explain to me how much testing he or she has done before sumitting the patch to me. As far as your concern about end users getting treated as "rats and monkeys", they are certainly getting paid; they get to use very high quality software for free! The tradeoff is that they won't get as much support compared to someone who is a paying customer of Red Hat or IBM. For the right customer, when I worked at IBM, I would fly on site to a bank in New York City and fix their bug. And I'm sure it's the same for Eric, if someone has a critical bug and they are a big Red Hat customer. Those who pay our salaries, get the best support. End users who aren't paying support to a big Linux company still get support, but it won't be as good, and that's just life. End users who want to use the latest code, are in fact guinea pigs. If they don't want to guinea pigs, they can use Fedora kernels or OpenSuSE kernels. > > where decent comments and making sure the code is maintainable is > > critically important. Since this takes other developer's time, > > especially senior developers like Eric and myself's time, which is > > I still concern how to correctly measure the lost, in the next half of > 2010, of time and value of end users, say 100, caused by what is buggy > in ext4. Are you sure no buggy, Eric and Ted? There is no such thing as code which is not buggy. For any non-trivial program, it's almost certain there are bugs. The only question is how easy it is for the someone to trigger the bugs, and what are the consequences if the bugs get triggered. During ext4 development, we've found bugs that were around in for over a decade in ext3, but it was simply something that no one had ever tripped over. (It required a certain race condition getting hit, plus a power failure or other unclean shutdown very shortly after the race condition, and it was simply something which no one had ever noticed, or if they did trip over it, they assumed it was caused by a hardware problem.) Ext4 is not exempt from these fundamental laws of software engienering. "Code is always buggy until the last user of the program dies". As far as the time value of users, remember that people who get free code, get what they pay for. We all have our salaries paid by someone, and at the end of the day, our priorities are driven by our employers. So I will track down a bug report from an end user because (a) that bug might affect the machines that I am paid to support, and (b) a larger ext4 user base is good for the community in general, and there are secondary benefits to my employer that accrue from that, and (c) Google is just a great company. :-) (As is Red Hat, SuSE, Oracle, etc. :-) But please don't get fooled that the resources we have to work on problems from end users, or time that I can spend mentoring newcomers to the kernel developer community, is infinite. Because it isn't. I'll spend some of my own time, and work late at nights, to help end users who are getting linux "for free", especially if they run into problems with ext4, because I like to help users as much as I can. And I will help out newcomers like you because it's personally important to me. But I do need to sleep, and I do have other priorities, like family and friends. And at the end of the day, because I like food with my meals, the needs of my $DAYJOB, are going to get priority, at least during work hours. Finally, keep in mind that the maxim that code which is not buggy also applies to your patches. At least some of your patches are definitely buggy, and which brings us back to the question whether things will be made better or worse if we were to apply all of your patches. And in the meantime, there are patches from other patch submitters which are proven themselves to be much more likely to be correct, and easier to review and integrate. If you were a maintainer, faced with limited time and resources, and someone who floods you with a large number of patches that take extra time to review and comprehend, and that person refuses to help you make life easier by reworking their patches so that they are easier to review and comprehend, what would _you_ do? Best regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/6, tytso@mit.edu <tytso@mit.edu>: > On Tue, Apr 06, 2010 at 09:43:58PM +0800, jing zhang wrote: >> >> In my view, one of the important parts of a patch is that the patcher >> is really concerning what is patched, another is whether the questions >> listed in the patch do exist, and whether the solution, if provided by >> the patcher, is correct. >> >> Even if the provide solution is ok, I think, it is the privilege, >> responsibility and duty of maintainers to execute the final >> submitting, and therefore it is not the duty of patchers to make sure >> that the solution is 100% correct and perfect, testing is good or not. > > Well, the problem is there are many more patch submitters than there > are maintainers, and so your proposal simply doesn't scale. Consider > that for some maintainers, there may be 10 or 20 or 30 or more patch > submitters in their subsystem. With that kind of > submitter-to-maintainer ratio, the patch submitter simply has to do > much more of the work, since otherwise the subsystem maintainer simply > can't keep up. > If you are not able to deal with all patches received, the simple way seems that you should share your burden with those maintainers who like. > Most maintainers actually spend much less time dealing with patch > submitters than I have invested with you. They simply send a NACK, > and maybe 2-3 sentences explaning of what still needs fixing, and then > it's up to the patch submitter to resubmit the patch. Some patches > are resubmitted 5, 6 times before the maintainer finally accepts it. This seems one of the cool reasons that some subsystem, say in stable stock kernel, is buggy, and end users have to face panic. > > I happen to believe that we need to encourage newcomers to the kernel > developer community, and so I spend more time mentoring people who are > new to the process. But at the end of the day, I have only so many > hours that I can spend on newcomers, and so after a while, I will > start serving other patch submitters who are more capable of providing > patches that are easy to review and integrate. > Newcomers, I think, are the cool gifts sent by Jesus and Buddha to what is called GUN Linux, and the accepted patches do not remove the buggy in the unaccepted patches, if the buggy does exist. >> I like to be a maintainer of some subsystem of GNU Linux, since it is >> my shame that end users experience panic, crash, bug, bug_on caused by >> what is under my maintenance, that end users are treated as rats and >> monkeys in laboratory even though I am not paid by any end user, and >> the nice reputations of distributors, say red hat, of service >> providers, say IBM. > > Sure, but that means the patches which are submitted has to be high > quality. If I get half-tested patches, or patches where you can't > give me a reproduction case that demonstrates the BUG_ON, and it's not The first action on the received patch to what is under my maintenance is to check whether there is really buggy. If yes, I apologize to the patcher as soon as possible. Second the correctness of the solution, if provided in patch, is checked as tough as I can, simply because that no case of reproduction can not tell me it is not buggy. Third it is my duty to be responsible for the stability of the subsystem in what is called stable stock kernel, with little to do with the safety of any patch, and I would share my ideas with the original patcher. > obvious whether or not the patch is safe, I have to worry about the > possibility that applying your patch may make the the file system more > likely to crash, not less. > > Ext4 is actually quite stable at this point. Very large numbers of How stable is it? Would you like to show which version of ext4 is free of buggy? > people are using it, and most users are quite happy. So at this Is happy meaning ext4 is free of buggy? > point, I have to weigh the risk that a patch might introduce a bug > against the claim that it fixes the bug --- especially if the patch It seems the day to day duty to review patches for any maintainer, but I am not sure. > submitter can't explain how the BUG_ON might have been triggered > without their patch, and can't explain to me how much testing he or > she has done before sumitting the patch to me. I do not think his/her testing can ensure the safety of the patch, but review is necessary. > > As far as your concern about end users getting treated as "rats and > monkeys", they are certainly getting paid; they get to use very high > quality software for free! The tradeoff is that they won't get as End users do get privilege for free from GNU, other than to be treated as rats and monkeys for any reason, I think. If you like to provide free ext4 as stable, first above all, you should be sure it is free of buggy, or at least you have to info end users, as much as you can, how buggy it is. In other words, if you like to provide free bread, and claim the bread is safe, you must ensure the bread is free of poison, or you have to info, as much as you can, those who take your bread that the bread is not free of poison, how danger the bread is. Is it murder with no info at all? > much support compared to someone who is a paying customer of Red Hat > or IBM. For the right customer, when I worked at IBM, I would fly on > site to a bank in New York City and fix their bug. And I'm sure it's > the same for Eric, if someone has a critical bug and they are a big > Red Hat customer. Those who pay our salaries, get the best support. Those right customers seems pay too much for what is unnecessary, and IBM is lucky enough, unlike Toyota. Indeed I first hear that, a cool story:) I would share it with my friends. Thanks. > End users who aren't paying support to a big Linux company still get > support, but it won't be as good, and that's just life. End users who So these end users who aren't paying do get treated as rats and monkeys by you? > want to use the latest code, are in fact guinea pigs. If they don't No, no and no, in any way, end users are not pigs in Guinea but users of GNU Linux, I believe. Jesus and Buddha, please, rescue me. > want to guinea pigs, they can use Fedora kernels or OpenSuSE kernels. > >> > where decent comments and making sure the code is maintainable is >> > critically important. Since this takes other developer's time, >> > especially senior developers like Eric and myself's time, which is >> >> I still concern how to correctly measure the lost, in the next half of >> 2010, of time and value of end users, say 100, caused by what is buggy >> in ext4. Are you sure no buggy, Eric and Ted? > > There is no such thing as code which is not buggy. For any > non-trivial program, it's almost certain there are bugs. The only So it is clear ext4 is not free of buggy but in stable linux-2.6.32. > question is how easy it is for the someone to trigger the bugs, and > what are the consequences if the bugs get triggered. During ext4 > development, we've found bugs that were around in for over a decade in > ext3, but it was simply something that no one had ever tripped over. And you achieved a lot, but I think ext4 is another ext3. > (It required a certain race condition getting hit, plus a power > failure or other unclean shutdown very shortly after the race > condition, and it was simply something which no one had ever noticed, > or if they did trip over it, they assumed it was caused by a hardware > problem.) > > Ext4 is not exempt from these fundamental laws of software > engienering. "Code is always buggy until the last user of the program > dies". Nobody want to die, it is a life not so hard. > > As far as the time value of users, remember that people who get free > code, get what they pay for. We all have our salaries paid by GNU is free, Linux is also free, it sounds cool, for any user at least today. > someone, and at the end of the day, our priorities are driven by our > employers. So I will track down a bug report from an end user because > (a) that bug might affect the machines that I am paid to support, and > (b) a larger ext4 user base is good for the community in general, and > there are secondary benefits to my employer that accrue from that, and > (c) Google is just a great company. :-) > > (As is Red Hat, SuSE, Oracle, etc. :-) > > But please don't get fooled that the resources we have to work on > problems from end users, or time that I can spend mentoring newcomers > to the kernel developer community, is infinite. Because it isn't. > I'll spend some of my own time, and work late at nights, to help end > users who are getting linux "for free", especially if they run into > problems with ext4, because I like to help users as much as I can. Nice words. And How about those users in Guinea? > And I will help out newcomers like you because it's personally > important to me. But I do need to sleep, and I do have other > priorities, like family and friends. And at the end of the day, > because I like food with my meals, the needs of my $DAYJOB, are going > to get priority, at least during work hours. > > Finally, keep in mind that the maxim that code which is not buggy also > applies to your patches. At least some of your patches are definitely > buggy, and which brings us back to the question whether things will be My buggy is still not in the stable kernel, right? > made better or worse if we were to apply all of your patches. And in > the meantime, there are patches from other patch submitters which are > proven themselves to be much more likely to be correct, and easier to > review and integrate. > > If you were a maintainer, faced with limited time and resources, and My mind is changed, I should not be a maintainer like you who are treating users differently. > someone who floods you with a large number of patches that take extra > time to review and comprehend, and that person refuses to help you > make life easier by reworking their patches so that they are easier to > review and comprehend, what would _you_ do? My answer is simple, I would apologize to all patchers first. good night - zj > > Best regards, > > - Ted > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- old-linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800 +++ ext4_mm_leak/mballoc3-2.c 2010-03-28 16:13:20.000000000 +0800 @@ -2397,11 +2397,27 @@ int ext4_mb_init(struct super_block *sb, i++; } while (i <= sb->s_blocksize_bits + 1); + sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group); + if (sbi->s_locality_groups == NULL) { + kfree(sbi->s_mb_offsets); + kfree(sbi->s_mb_maxs); + return -ENOMEM; + } + for_each_possible_cpu(i) { + struct ext4_locality_group *lg; + lg = per_cpu_ptr(sbi->s_locality_groups, i); + mutex_init(&lg->lg_mutex); + for (j = 0; j < PREALLOC_TB_SIZE; j++) + INIT_LIST_HEAD(&lg->lg_prealloc_list[j]); + spin_lock_init(&lg->lg_prealloc_lock); + } + /* init file for buddy data */ ret = ext4_mb_init_backend(sb); if (ret != 0) { kfree(sbi->s_mb_offsets); kfree(sbi->s_mb_maxs); + free_percpu(sbi->s_locality_groups); return ret; } @@ -2415,20 +2431,6 @@ int ext4_mb_init(struct super_block *sb, sbi->s_mb_order2_reqs = MB_DEFAULT_ORDER2_REQS; sbi->s_mb_group_prealloc = MB_DEFAULT_GROUP_PREALLOC; - sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group); - if (sbi->s_locality_groups == NULL) { - kfree(sbi->s_mb_offsets); - kfree(sbi->s_mb_maxs); - return -ENOMEM; - } - for_each_possible_cpu(i) { - struct ext4_locality_group *lg; - lg = per_cpu_ptr(sbi->s_locality_groups, i); - mutex_init(&lg->lg_mutex); - for (j = 0; j < PREALLOC_TB_SIZE; j++) - INIT_LIST_HEAD(&lg->lg_prealloc_list[j]); - spin_lock_init(&lg->lg_prealloc_lock); - } if (sbi->s_proc) proc_create_data("mb_groups", S_IRUGO, sbi->s_proc,