On Wed, 2009-06-24 at 23:52 +0100, Alan Jenkins wrote: > On 6/24/09, Andreas Robinson <andr345@xxxxxxxxx> wrote: > > This is just to let you know that I haven't dropped the ball on the soft > > dependency command. It isn't ready to be reviewed, much less merged, but > > it might serve as a few minutes idle reading. > > > > http://github.com/andr345/module-init-tools/commits/modprobe-softdep/ > > > > Cheers, > > Andreas > > Cool. I happen to be rather idle. > > > The following changes since commit > > 9ded2725bf85c782cdceb078dd61b31f2cecb5c7: > > Jon Masters (1): > > doc: link depmod.d to depmod.conf also > > > > are available in the git repository at: > > > > http://github.com/andr345/module-init-tools.git modprobe-softdep > > > > Andreas Robinson (6): > > modprobe: add forked do_modprobe() wrapper > > I hope you find a way to survive softdep errors without forking, as > the changelog says this is a workaround :-). It should be ok to exit > on ENOMEM, and modprobe -a already knows how to continue on errors. Well, it's not so much a workaround as a way to paper over fundamental flaws. :) I mean, modprobe wasn't written to ever free memory, close files and exit cleanly. And, it's all too convenient to put in a fatal() call when you need one. But forking is really cheap on Linux, isn't it? The kernel will only allocate a task structure and copy a page table, AFAIK. The rest is copy-on-write. We still save the time spent launching a shell, parsing the command line and so on, and get a (crude) way of detecting dependency loops. > > modprobe: reduce nesting in conf parser > > modprobe: trivial whitespace/indentation fixes in conf parser > > > modprobe: put configuration objects in a struct > > Neat! I think this idea is a great cleanup in it's own right. Thankee sai. > I noticed this > > + struct modprobe_conf conf; > + memset(&conf, 0, sizeof(struct modprobe_conf)); > > - personally I'm rigid and would always put a blank line between > declarations and code :-). In case you didn't know, there is a > shorter equivalent which you might like > > struct modprobe_conf conf = {}; Ooooh, pretty! Will use. > > > modprobe: implement softdep conf command > > modprobe: add softdep loop detector > > AFAICS softdeps aren't triggered by aliases in this implementation. > So they won't apply to modules loaded during udev startup ("coldplug" > aka udevadm trigger). They aren't triggered by normal deps either. > lt looks like they're not a strict replacement for install commands > which run modprobe - and they seem largely pointless. > > Do you have some examples where softdeps would be useful even with > these limitations? The idea is to replace existing install commands > (or avoid the need for new ones), right? Damn, I hoped you wouldn't notice. ;-) You're absolutely right. The current logic is something I hacked up without much thought to make the rest testable, hoping it would sort itself out eventually. I've been fretting over the top-level structural additions and changes that need to be done because I don't want to break anything or introduce subtle bugs. So, some design decisions have repeatedly been put aside for "later". (Which is now, apparently.) For example: since the various commands are used in different places (c.f alias and install) it would be logical and nice to separate the configuration-file parsing and per-module lookup. (This is how softdep should be handled at any rate, that fnmatch() call inside the parser was an afterthought.) However, the include command should be ripped out at the same time, but I'm not sure downstream has had enough time to move away from using it yet. If include has to stay in for now, I'd have to design something ugly to encode the tree formed by the include files. That "something" would be obsolete before it's ever used and then be removed again in a few months. It's things like this that I have trouble getting past ... -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
