Re: [PATCH] direct IO breaks root filesystem

[GOTO Masanori <gotom@debian.org>]

At Mon, 10 Dec 2001 09:10:24 -0800 (PST),
Linus Torvalds <torvalds@transmeta.com> wrote:
> On Mon, 10 Dec 2001, GOTO Masanori wrote:
> > >
> > > "generic_direct_IO()" should just get the device from "bh->b_dev (which is
> > > filled in correctly by "get_block()".
> >
> > Oh, that's right, the patch becomes more simple (and works well).
> > Is this patch OK?

Linus, Andrea, thank you for your comments!

> This looks fine to me. At some point we _may_ end up with filesystems that
> understand about multiple devices, so it's possible in theory that
> "get_block()" might return different devices for different blocks, but
> that does not happen currently, and I'm not really sure it ever will.

I don't test for the multiple device with direct IO under current
kernel..., I keep it in mind.

> Marcelo, I do believe this should go into 2.4.x too..

Thanks Marcelo for including my patch :)

I, however, found another problem.
Accessing with inode size unit (== 4096 byte) is ok, but if I accessed
with block size unit, generic_direct_IO() returns error.  The reason
is that blocksize is designated as inode->i_blkbits, and its value is
not disk minimal block size (512), but inode's unit size (4096).

Patch is here. If inode is S_ISBLK and is not mounted, then blocksize
is set as hardsect_size. This implementation needs some computational
cost, but I think it can be ignored. This patch works well for me.

--- linux-2.4.17-pre8.vanilla/mm/filemap.c	Tue Dec 11 15:54:57 2001
+++ linux-2.4.17-pre8/mm/filemap.c	Tue Dec 11 16:21:40 2001
@@ -1508,8 +1508,22 @@
 		new_iobuf = 1;
 	}
 
-	blocksize = 1 << inode->i_blkbits;
-	blocksize_bits = inode->i_blkbits;
+	/*
+	 * If inode is BLK and it is not already mounted,
+	 * blocksize change hardsect_size.
+	 */
+	if (S_ISBLK(inode->i_mode) && !is_mounted(inode->i_rdev)) {
+		int size;
+
+		size = get_hardsect_size(inode->i_rdev);
+		blocksize = size;
+		for (blocksize_bits = 0; !(size & 1); )
+			size >>= 1, blocksize_bits++;
+	} else {
+		blocksize = 1 << inode->i_blkbits;
+		blocksize_bits = inode->i_blkbits;
+	}
+
 	blocksize_mask = blocksize - 1;
 	chunk_size = KIO_MAX_ATOMIC_IO << 10;


The current problem is that is_mounted() checking is worked for
/dev/sda1 (partition area) only, not for /dev/sda (total disk area),
if /dev/sda1 is mounted. This problem is also existed in /dev/raw
interface, but I think it's not important issue (should I fix it?)

Thanks,
-- gotom