Re: Linux 2.4.10-pre11

[Andrea Arcangeli]

On Fri, Sep 21, 2001 at 06:46:21AM +0200, Andrea Arcangeli wrote:
> On Fri, Sep 21, 2001 at 06:06:25AM +0200, Andrea Arcangeli wrote:
> > On Fri, Sep 21, 2001 at 05:47:49AM +0200, Andrea Arcangeli wrote:
> > > write and I cannot zero it out. It is getting harder to fix this one
> > > just inside the ->make_request callback... Hints?
> > 
> > I think the best fix is to have the ramdisk using the same aops of ramfs
> > and replace "Secure" with "Uptodate". We need to trap the security issue
> > at the higher layer and also this will avoid us having to map useless
> > bh, so it should be an improvement, only the filesystem will end
> > triggering the ->make_request callback of the ramdisk. Then if the fs
> > does I/O on stuff out of the physical address space we'll just clear it
> > out.
> 
> ok, here it is, this obsoletes the previous patch (so again against
> 2.4.10pre12), initrd works again.

rediffed against pre13 (I'm passing the bd_inode to the ->open/release,
so we can modify the a_ops of the physical address space, ramdisk needs
that in rd_open())

diff -urN ramdisk-ref/drivers/block/rd.c ramdisk/drivers/block/rd.c
--- ramdisk-ref/drivers/block/rd.c	Fri Sep 21 08:28:04 2001
+++ ramdisk/drivers/block/rd.c	Fri Sep 21 08:06:02 2001
@@ -186,17 +186,77 @@
 
 #endif
 
+/*
+ * Copyright (C) 2000 Linus Torvalds.
+ *               2000 Transmeta Corp.
+ * aops copied from ramfs.
+ */
+static int ramdisk_readpage(struct file *file, struct page * page)
+{
+	if (!Page_Uptodate(page)) {
+		memset(kmap(page), 0, PAGE_CACHE_SIZE);
+		kunmap(page);
+		flush_dcache_page(page);
+		SetPageUptodate(page);
+	}
+	UnlockPage(page);
+	return 0;
+}
+
+/*
+ * Writing: just make sure the page gets marked dirty, so that
+ * the page stealer won't grab it.
+ */
+static int ramdisk_writepage(struct page *page)
+{
+	SetPageDirty(page);
+	UnlockPage(page);
+	return 0;
+}
+
+static int ramdisk_prepare_write(struct file *file, struct page *page, unsigned offset, unsigned to)
+{
+	void *addr = kmap(page);
+	if (!Page_Uptodate(page)) {
+		memset(addr, 0, PAGE_CACHE_SIZE);
+		flush_dcache_page(page);
+		SetPageUptodate(page);
+	}
+	SetPageDirty(page);
+	return 0;
+}
+
+static int ramdisk_commit_write(struct file *file, struct page *page, unsigned offset, unsigned to)
+{
+	kunmap(page);
+	return 0;
+}
+
+struct address_space_operations ramdisk_aops = {
+	readpage: ramdisk_readpage,
+	writepage: ramdisk_writepage,
+	prepare_write: ramdisk_prepare_write,
+	commit_write: ramdisk_commit_write,
+};
+
 static int rd_blkdev_pagecache_IO(int rw, struct buffer_head * sbh, int minor)
 {
-	struct address_space * mapping = rd_inode[minor]->i_mapping;
+	struct address_space * mapping;
 	unsigned long index;
-	int offset, size, err = 0;
+	int offset, size, err;
 
-	if (sbh->b_page->mapping == mapping) {
-		if (rw != READ)
-			SetPageDirty(sbh->b_page);
+	err = -EIO;
+	/*
+	 * If we did BLKFLSBUF just before doing read/write,
+	 * we could see a rd_inode before the opener had a chance
+	 * to return from rd_open(), that's ok, as soon as we
+	 * see the inode we can use its i_mapping, and the inode
+	 * cannot go away from under us.
+	 */
+	if (!rd_inode[minor])
 		goto out;
-	}
+	err = 0;
+	mapping = rd_inode[minor]->i_mapping;
 
 	index = sbh->b_rsector >> (PAGE_CACHE_SHIFT - 9);
 	offset = (sbh->b_rsector << 9) & ~PAGE_CACHE_MASK;
@@ -206,8 +266,7 @@
 		int count;
 		struct page ** hash;
 		struct page * page;
-		const char * src;
-		char * dst;
+		char * src, * dst;
 		int unlock = 0;
 
 		count = PAGE_CACHE_SIZE - offset;
@@ -217,20 +276,24 @@
 
 		hash = page_hash(mapping, index);
 		page = __find_get_page(mapping, index, hash);
-		if (!page && rw != READ) {
+		if (!page) {
 			page = grab_cache_page(mapping, index);
 			err = -ENOMEM;
 			if (!page)
 				goto out;
 			err = 0;
+
+			if (!Page_Uptodate(page)) {
+				memset(kmap(page), 0, PAGE_CACHE_SIZE);
+				kunmap(page);
+				flush_dcache_page(page);
+				SetPageUptodate(page);
+			}
+
 			unlock = 1;
 		}
 
 		index++;
-		if (!page) {
-			offset = 0;
-			continue;
-		}
 
 		if (rw == READ) {
 			src = kmap(page);
@@ -321,6 +384,13 @@
 				struct block_device * bdev = inode->i_bdev;
 
 				down(&bdev->bd_sem);
+				/*
+				 * We're the only users here, protected by the
+				 * bd_sem, but first check if we didn't just
+				 * flushed the ramdisk.
+				 */
+				if (!rd_inode[minor])
+					goto unlock;
 				if (bdev->bd_openers > 2) {
 					up(&bdev->bd_sem);
 					return -EBUSY;
@@ -328,8 +398,16 @@
 				bdev->bd_openers--;
 				bdev->bd_cache_openers--;
 				iput(rd_inode[minor]);
+				/*
+				 * Make sure the cache is flushed from here
+				 * and not from close(2), somebody
+				 * could reopen the device before we have a
+				 * chance to close it ourself.
+				 */
+				truncate_inode_pages(rd_inode[minor]->i_mapping, 0);
 				rd_inode[minor] = NULL;
 				rd_blocksizes[minor] = rd_blocksize;
+			unlock:
 				up(&bdev->bd_sem);
 			}
 			break;
@@ -420,6 +498,8 @@
 			/* bdev->bd_sem is held by caller */
 			bdev->bd_openers++;
 			bdev->bd_cache_openers++;
+
+			bdev->bd_inode->i_mapping->a_ops = &ramdisk_aops;
 		}
 	}
 
diff -urN ramdisk-ref/fs/block_dev.c ramdisk/fs/block_dev.c
--- ramdisk-ref/fs/block_dev.c	Fri Sep 21 07:48:12 2001
+++ ramdisk/fs/block_dev.c	Fri Sep 21 08:25:29 2001
@@ -736,7 +736,7 @@
 	if (bdev->bd_op) {
 		ret = 0;
 		if (bdev->bd_op->open)
-			ret = bdev->bd_op->open(inode,filp);
+			ret = bdev->bd_op->open(bdev->bd_inode,filp);
 		if (!ret) {
 			bdev->bd_openers++;
 			bdev->bd_cache_openers++;
@@ -835,7 +835,7 @@
 
 	/* release the device driver */
 	if (bdev->bd_op->release)
-		ret = bdev->bd_op->release(inode, NULL);
+		ret = bdev->bd_op->release(bd_inode, NULL);
 	if (!--bdev->bd_openers) {
 		bdev->bd_op = NULL;
 		bdev->bd_inode = NULL;

Andrea