On Sat, Sep 24, 2011 at 9:57 PM, Aapo Laine <aapo.laine@xxxxxxxxxxxxx> wrote: > > On 09/23/11 11:15, NeilBrown wrote: >> >> On Fri, 23 Sep 2011 02:09:36 -0600 Thomas Fjellstrom<thomas@xxxxxxxxxxxxx> >> wrote: >>> >>> I forgot to say, but: Thank you very much :) for the help, and your tireless >>> work on md. >>> >> >> You've very welcome .... but I felt I needed to respond to that word >> "tireless". >> The truth is that I am getting rather tired of md .... if anyone knows anyone >> who wants to get into kernel development and is wondering where to start - >> please consider whispering 'the md driver' in their ear. Plenty to do, great >> mentoring possibilities, and competent linux kernel engineers with good >> experience are unlikely to have much trouble finding a job ;-) >> >> NeilBrown > > Whoa this is shocking news! > > Firstly then, thank you so much for your excellent work up to now. Linux has what I believe to be the best software raid of all operating systems thanks to you. Excellent in both features, and reliability i.e. quality of code. > > And also the support through the list was great. I found so many problems solved already just by looking at the archives... so many people with their arse saved by you. > > I think everybody here is sorry to see you willing to go. > > Now the bad news... Regarding the MD takeover, there I think I see a problem... > The MD code is very tersely commented, compared to its complexity! > > - there is not much explanation of overall strategies, or the structure of code. Also the flow of data between the functions is not much explained most of the times. > > - It's not obvious to me what is the entry point for kernel processes related to MD arrays, how are they triggered and where do they run... E.g. in the past I tried to understand how did resync work, but I couldn't. I thought there was a kernel process controlling resync advancement, but I couldn't really find the boundaries of code inside which it was executing. > > - it's not clear what the various functions do or in what occasion they are called. Except from their own name, most of them have no comments just before the definition. > > - the algoritms within the functions are very long and complex, but only rarely they are explained by comments. I am now seeing pieces having 5 levels of if's nested one inside the other, and there are almost no comments. > > - last but not least, variables have very short names, and for most of them, it is not explained what they mean. This is mostly for local variables, but sometimes even for the structs which go into metadata e.g. in struct r1_private_data_s most members do not have an explanation. This is pretty serious, to me at least, for understanding the code. > > - ... maybe more I can't think of right now ... > > Your code is of excellent quality, as I wrote, I wish there were more programmers like you, but if you now want to leave, THEN I start to be worried! Would you please comment it (much) more before leaving? Fully understanding your code I think is going to take other people a lot of time otherwise, and you might not find a replacement easily and/or s/he might do mistakes. > > There were times in the past when I had ideas and I wanted to contribute code, but when I looked inside MD and tried to understand where should I put my changes, I realized I wasn't able to understand what current code was doing. Maybe I am not a good enough C programmer, but I was able to change things in other occasions. > > > I hope you won't get these critiques bad... > and thanks for all your efforts, really, in the name of, I think, everybody. > Aapo L. > -- Thank you Mr. Brown for the immensely useful mdadm. In its simplicity, it is mastery of craft, purely functional to the point that it attains an inherent beauty of form. Re. Aapo's comments, a GitHub repository of mdadm could be the ticket. A de-facto Github merge maintainer owns a master repo, say "mdadm-documented". People fork from it at will, creating comments for the code and / or increasing intuitive readability if need be. Maintaner merges it all, creating a mdadm version that is functionally identical to mdadm stable but sort of pedagogically smoothed :) The mdadm maintainer could then merge this into the actual mdadm head at will. If it takes off, this would take the load off Neil's shoulders – plus it's sometimes easier for fresh eyes to see things in context when the initial work of building the full mental picture is done. -kd -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
