On Tue, 7 Mar 2000, Tigran Aivazian wrote:
> On Tue, 7 Mar 2000, Mike Galbraith wrote:
> > Instead of exporting bdget(), I opted to keep the ramdisk's private
> > problem internal to that driver as in patch below. Thoughts?
>
> The main thought is that the driver is broken at the moment, irrespective
> of your latest change. Namely:
>
> a) dd a filesystem image (I used 2.88M ext2 LILO floppy but anything
> should do) to /dev/rd/0 and mount it.
>
> b) edit some text file on it
>
> c) umount /dev/rd/0
>
> d) dd /dev/rd/0 to a disk file say boot2.img
>
> e) dd /dev/zero to /dev/rd/0
>
> f) dd boot2.img to /dev/rd/0 and mount it. Behold, the text file you
> edited is full of zeroes.
>
> The kernel is 2.3.50-SMP + your patch. No rmmod/insmod-ing of rd was done
> at any time, i.e. it was only loaded once by:
>
> insmod rd rd_size=2880 rd_blocksize=1024
>
> Please let me know what info you need to assist you with fixing this
> problem.
OK, I think I have it fixed, and would _really_ appreciate it if you would
test/confirm. (would very much like to see this problem finally die..
and stay dead long enough for someone to actually use the device :)
The problem is one of aliases. I don't fully understand it, but the
only way I have ever (since page_cache was born) been able to get the
thing to act fully sane is to unconditionally copy the data to and fro.
This patch does that, keeps protection problem internal to the driver,
and adds an in kernel ramdisk_blocksize setup option. Reasoning for
adding the setup option is simply because if module has the option,
in kernel setup should also have it also.
I am going to veeeeeeery thoroughly test this bugger before sending
it off to Linus for consideration, and would appreciate it if folks
who use ramdisk and initrd would give it a trial burn.
-Mike
--- linux-2.3.50.virgin/drivers/block/rd.c Fri Mar 3 06:28:01 2000
+++ linux-2.3.49.test/drivers/block/rd.c Wed Mar 8 08:23:49 2000
@@ -99,6 +99,7 @@
static int rd_blocksizes[NUM_RAMDISKS]; /* Size of 1024 byte blocks :) */
static int rd_kbsize[NUM_RAMDISKS]; /* Size in blocks of 1024 bytes */
static devfs_handle_t devfs_handle = NULL;
+static struct block_device *rd_dev[NUM_RAMDISKS]; /* Protected devices */
/*
* Parameters for the boot-loading of the RAM disk. These are set by
@@ -169,11 +170,18 @@
return ramdisk_size(str);
}
+static int __init ramdisk_blocksize(char *str)
+{
+ rd_blocksize = simple_strtol(str,NULL,0);
+ return 1;
+}
+
__setup("ramdisk_start=", ramdisk_start_setup);
__setup("load_ramdisk=", load_ramdisk);
__setup("prompt_ramdisk=", prompt_ramdisk);
__setup("ramdisk=", ramdisk_size);
__setup("ramdisk_size=", ramdisk_size2);
+__setup("ramdisk_blocksize=", ramdisk_blocksize);
#endif
@@ -216,67 +224,17 @@
goto repeat;
}
- /*
- * This has become somewhat more complicated with the addition of
- * the page cache. The problem is that in some cases the furnished
- * buffer is "real", i.e., part of the existing ramdisk, while in
- * others it is "unreal", e.g., part of a page. In the first case
- * not much needs to be done, while in the second, some kind of
- * transfer is needed.
- *
- * The two cases are distinguished here by checking whether the
- * real buffer is already in the buffer cache, and whether it is
- * the same as the one supplied.
- *
- * There are three cases with read/write to consider:
- *
- * 1. Supplied buffer matched one in the buffer cache:
- * Read - Clear the buffer, as it wasn't already valid.
- * Write - Mark the buffer as "Protected".
- *
- * 2. Supplied buffer mismatched one in the buffer cache:
- * Read - Copy the data from the buffer cache entry.
- * Write - Copy the data to the buffer cache entry.
- *
- * 3 No buffer cache entry existed:
- * Read - Clear the supplied buffer, but do not create a real
- * one.
- * Write - Create a real buffer, copy the data to it, and mark
- * it as "Protected".
- *
- * NOTE: There seems to be some schizophrenia here - the logic
- * using "len" seems to assume arbitrary request lengths, while
- * the "protect" logic assumes a single buffer cache entry.
- * This seems to be left over from the ancient contiguous ramdisk
- * logic.
- */
-
sbh = CURRENT->bh;
- rbh = get_hash_table(sbh->b_dev, sbh->b_blocknr, sbh->b_size);
- if (sbh == rbh) {
- if (CURRENT->cmd == READ)
- memset(CURRENT->buffer, 1, len);
- } else if (rbh) {
- if (CURRENT->cmd == READ)
+ if ((rbh = getblk(sbh->b_dev, sbh->b_blocknr, sbh->b_size)) == NULL)
+ BUG();
+ if (CURRENT->cmd == READ) {
+ if (sbh != rbh)
memcpy(CURRENT->buffer, rbh->b_data, rbh->b_size);
- else
+ } else
+ if (sbh != rbh)
memcpy(rbh->b_data, CURRENT->buffer, rbh->b_size);
- } else { /* !rbh */
- if (CURRENT->cmd == READ)
- memset(sbh->b_data, 2, len);
- else {
- rbh = getblk(sbh->b_dev, sbh->b_blocknr, sbh->b_size);
- if (rbh)
- memcpy(rbh->b_data, CURRENT->buffer,
- rbh->b_size);
- else
- BUG(); /* No buffer, what to do here? */
- }
- }
- if (rbh) {
- mark_buffer_protected(rbh);
- brelse(rbh);
- }
+ mark_buffer_protected(rbh);
+ brelse(rbh);
end_request(1);
goto repeat;
@@ -372,6 +330,14 @@
if (DEVICE_NR(inode->i_rdev) >= NUM_RAMDISKS)
return -ENXIO;
+ /*
+ * Immunize device against invalidate_buffers().
+ */
+ if (rd_dev[DEVICE_NR(inode->i_rdev)] == NULL) {
+ rd_dev[DEVICE_NR(inode->i_rdev)] = inode->i_bdev;
+ atomic_inc(&rd_dev[DEVICE_NR(inode->i_rdev)]->bd_openers);
+ }
+
MOD_INC_USE_COUNT;
return 0;
@@ -389,24 +355,30 @@
ioctl: rd_ioctl,
};
+#ifdef MODULE
/* Before freeing the module, invalidate all of the protected buffers! */
static void __exit rd_cleanup (void)
{
int i;
for (i = 0 ; i < NUM_RAMDISKS; i++) {
- struct block_device *bdev;
- bdev = bdget(kdev_t_to_nr(MKDEV(MAJOR_NR,i)));
- atomic_dec(&bdev->bd_openers);
+ if (rd_dev[i]) {
+ /* withdraw invalidate_buffers() immunity */
+ atomic_dec(&rd_dev[i]->bd_openers);
+ /* remove stale pointer to module address space */
+ rd_dev[i]->bd_op = NULL;
+ }
destroy_buffers(MKDEV(MAJOR_NR, i));
}
devfs_unregister (devfs_handle);
unregister_blkdev( MAJOR_NR, "ramdisk" );
blk_cleanup_queue(BLK_DEFAULT_QUEUE(MAJOR_NR));
+ hardsect_size[MAJOR_NR] = NULL;
blksize_size[MAJOR_NR] = NULL;
blk_size[MAJOR_NR] = NULL;
}
+#endif
/* This is the registration and initialization section of the RAM disk driver */
int __init rd_init (void)
@@ -441,21 +413,16 @@
S_IFBLK | S_IRUSR | S_IWUSR, 0, 0,
&fd_fops, NULL);
- hardsect_size[MAJOR_NR] = rd_hardsec; /* Size of the RAM disk blocks */
- blksize_size[MAJOR_NR] = rd_blocksizes; /* Avoid set_blocksize() check */
-
- for (i = 0; i < NUM_RAMDISKS; i++) {
- struct block_device *bdev;
+ for (i = 0; i < NUM_RAMDISKS; i++)
register_disk(NULL, MKDEV(MAJOR_NR,i), 1, &fd_fops, rd_size<<1);
- bdev = bdget(kdev_t_to_nr(MKDEV(MAJOR_NR,i)));
- atomic_inc(&bdev->bd_openers); /* avoid invalidate_buffers() */
- }
#ifdef CONFIG_BLK_DEV_INITRD
/* We ought to separate initrd operations here */
register_disk(NULL, MKDEV(MAJOR_NR,INITRD_MINOR), 1, &fd_fops, rd_size<<1);
#endif
+ hardsect_size[MAJOR_NR] = rd_hardsec; /* Size of the RAM disk blocks */
+ blksize_size[MAJOR_NR] = rd_blocksizes; /* Avoid set_blocksize() check */
blk_size[MAJOR_NR] = rd_kbsize; /* Size of the RAM disk in kB */
/* rd_size is given in kB */
@@ -468,8 +435,8 @@
#ifdef MODULE
module_init(rd_init);
-#endif
module_exit(rd_cleanup);
+#endif
/* loadable module support */
MODULE_PARM (rd_size, "1i");
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Mar 15 2000 - 21:00:13 EST