On Tuesday 16 October 2007 17:52, Jan Engelhardt wrote:
> On Oct 16 2007 17:47, Nick Piggin wrote:
> >Here's a quick first hack...
>
> Inline patches preferred ;-)
Thanks for reviewing it anyway ;)
> >+config BLK_DEV_BRD
> >+ tristate "RAM block device support"
> >+ ---help---
> >+ This is a new based block driver that replaces BLK_DEV_RAM.
>
> based on what? -^
RAM based.
> >+ To compile this driver as a module, choose M here: the
> >+ module will be called rd.
>
> called brd.ko.
Changed. But it will hopefully just completely replace rd.c,
so I will probably just rename it to rd.c at some point (and
change .config options to stay compatible). Unless someone
sees a problem with that?
> >+/*
> >+ * And now the modules code and kernel interface.
> >+ */
> >+static int rd_nr;
> >+static int rd_size = CONFIG_BLK_DEV_RAM_SIZE;
>
> Perhaps unsigned?
> Perhaps even long for rd_size?
I've taken most of that stuff out of rd.c in an effort to stay
back compatible. I don't know if it really matters to use long?
> >+module_param(rd_nr, int, 0);
> >+MODULE_PARM_DESC(rd_nr, "Maximum number of brd devices");
> >+module_param(rd_size, int, 0);
> >+MODULE_PARM_DESC(rd_size, "Size of each RAM disk in kbytes.");
> >+MODULE_LICENSE("GPL");
> >+MODULE_ALIAS_BLOCKDEV_MAJOR(RAMDISK_MAJOR);
> >+
> >+/* options - nonmodular */
> >+#ifndef MODULE
> >+static int __init ramdisk_size(char *str)
> >+{
> >+ rd_size = simple_strtol(str,NULL,0);
> >+ return 1;
> >+}
>
> Is this, besides for compatibility, really needed?
>
> >+static int __init ramdisk_size2(char *str)
> >+{
> >+ return ramdisk_size(str);
> >+}
> >+static int __init rd_nr(char *str)
>
> Err! Overlapping symbols! The rd_nr() function collides with the rd_nr
> variable.
Thanks... %s gone awry. Fixed to the expected names.
> It also does not seem needed, since it did not exist before.
> It should go, you can set the variable with brd.rd_nr=XXX (same
> goes for ramdisk_size).
But only if it's a module?
> What's the point of ramdisk_size2()?
Back compat. When rewriting the internals, I want to try avoid
changing any externals if possible. Whether this is the Right
Way to do it or not, I don't know :P
> >+{
> >+ rd_nr = simple_strtol(str, NULL, 0);
> >+ return 1;
> >+}
> >+__setup("ramdisk=", ramdisk_size);
> >+__setup("ramdisk_size=", ramdisk_size2);
>
> __setup("ramdisk_size=", ramdisk_size); maybe, or does not that work?
Didn't try it, but the rd.c code does the same thing so I guess it
doesn't (or didn't, when it was written).
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
