Re: [patch][rfc] rewrite ramdisk

[Jan Engelhardt]

On Oct 16 2007 17:47, Nick Piggin wrote:
>
>Here's a quick first hack...

Inline patches preferred ;-)

>+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?         -^

>+	  To compile this driver as a module, choose M here: the
>+	  module will be called rd.

called brd.ko.

>+/*
>+ * 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?

>+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. 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). What's the point of ramdisk_size2()?

>+{
>+	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?

>+__setup("rd_nr=", rd_nr);
>+#endif
>+
>+
>+static struct brd_device *brd_alloc(int i)
>+{
>+	struct brd_device *brd;
>+	struct gendisk *disk;
>+
>+	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
>+	if (!brd)
>+		goto out;
>+	brd->brd_number		= i;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/