Re: [PATCH] direct IO breaks root filesystem

GOTO Masanori (gotom@debian.org)
Tue, 11 Dec 2001 18:03:08 +0900


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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/