Hi Alexander (and all others who are reading this),
I've got a new loop patch which should be a lot cleaner when
you access a file via the loop device. It uses the file ops of
the file to access it. (It applies to 2.1.129, why see below)
Still some things don't work:
First of all, I found out that the "create_missing_block" function
doesn't seem to work, at least it didn't work on my system.
(If you tried to create a file system with 1024 blocks (1meg) on
a file which was only 1 Byte long it didn't work...)
Since I think that the way the patch does it should make
create_missing_block obsolete, because the code does really
the same, I deleted the function completely.
But the handling of sparse files still doesn't work correctly
(I will have a further look...)
Then I found out that access to your harddisk slows down to a crawl,
if you mount a block device via the loop device (unencrypted)
(for example
losetup /dev/loop0 /dev/hda1
mount /dev/loop0 /mnt)
)
and run "bonnie" on it (bonnie is a program for measuring the
performance of filesystem accesses). It happens when bonnie
starts its "rewriting" phase.
Obviously this is a buffer/cache issue, somehow something doesn't
work as expected...
The code snippet used for accessing block devices is:
(This is executed for each block which is transferred.
I hope the code explains itself ?)
----------------------
bh = getblk(lo->lo_device, block, blksize);
if (!bh) {
printk(KERN_ERR "loop: device %s: getblk(-, %d, %d) returned NULL",
kdevname(lo->lo_device),block, blksize);
goto error_out_lock;
}
if (!buffer_uptodate(bh) && ((current_request->cmd == READ) ||
(offset || (len < blksize)))) {
ll_rw_block(READ, 1, &bh);
wait_on_buffer(bh);
if (!buffer_uptodate(bh)) {
brelse(bh);
goto error_out_lock;
}
}
if ((lo->transfer)(lo, current_request->cmd, bh->b_data + offset,
dest_addr, size)) {
printk(KERN_ERR "loop: transfer error block %d\n", block);
brelse(bh);
goto error_out_lock;
}
if (current_request->cmd == WRITE) {
mark_buffer_uptodate(bh, 1);
mark_buffer_dirty(bh, 1);
}
brelse(bh);
----------------------------
Can someone of the buffer/cache gurus give me a hint how to improve
that behaviour ?
If you mount a file with the loop device than there is of course also
a performance loss, but it doesn't slow down to a crawl.
(I think blocks get cached twice, first when some upper layer accesses
the loop device via ll_rw_block, and then the second time when
the loop device accesses the real block device. How can you avoid
this double caching ?)
Why is this patch against 2.1.129 ?
The reason is that 2.1.130 contains an update to pass the
block number to the transfer function (which is a good thing),
but doesn't make sure that blocks are really only accessed as a
whole. (If you try to start to read a block in the middle of it
you won't be able to decrypt it if the whole block is encrypted
with CBC mode).
A safe (and efficient) way to do it would be to pass the _sector_
number, because as far as I can tell an access to a block device
never transfers partial sectors.
(I'm not sure if this is also true for blocks. If it is true
then it would be possible to make the code simpler because all
the partial sector reading stuff could be deleted.)
so long
Ingo
--V0207lvV8h4k8FAm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="loop.patch"
--- loop.c Mon Nov 30 23:56:02 1998
+++ loop_new.c Thu Dec 3 02:00:46 1998
@@ -59,10 +59,6 @@
#define FALSE 0
#define TRUE (!FALSE)
-/* Forward declaration of function to create missing blocks in the
- backing file (can happen if the backing file is sparse) */
-static int create_missing_block(struct loop_device *lo, int block, int blksize);
-
/*
* Transfer functions
*/
@@ -148,12 +144,16 @@
static void do_lo_request(void)
{
- int real_block, block, offset, len, blksize, size;
- char *dest_addr;
+ int block, offset, len, blksize, size;
+ char *dest_addr,block_buf[4096];
struct loop_device *lo;
struct buffer_head *bh;
struct request *current_request;
- int block_present;
+ struct file* file;
+ int retval;
+ loff_t f_offset;
+ mm_segment_t old_fs;
+ struct inode *inode;
repeat:
INIT_REQUEST;
@@ -203,31 +203,64 @@
if (size > len)
size = len;
- real_block = block;
- block_present = TRUE;
-
- if (lo->lo_flags & LO_FLAGS_DO_BMAP) {
- real_block = bmap(lo->lo_dentry->d_inode, block);
- if (!real_block) {
-
- /* The backing file is a sparse file and this block
- doesn't exist. If reading, return zeros. If
- writing, force the underlying FS to create
- the block */
- if (current_request->cmd == READ) {
- memset(dest_addr, 0, size);
- block_present = FALSE;
- } else {
- if (!create_missing_block(lo, block, blksize)) {
- goto error_out_lock;
- }
+ if (lo->lo_backing_file) { /* If we access a file use file ops to */
+ /* access the data */
+ f_offset = block * blksize + offset;
+ file = lo->lo_backing_file;
+
+ if (file->f_op == NULL) {
+ printk(KERN_WARNING "loop: no file ops\n");
+ goto error_out_lock;
+ }
+
+ if (file->f_op->llseek != NULL) {
+ file->f_op->llseek(file, f_offset, 0);
+ }
+ else {
+ /* Do what the default llseek() code would have done */
+ file->f_pos = f_offset;
+ file->f_reada = 0;
+ file->f_version = ++event; /* What's this ??? */
+ }
+
+ if (current_request->cmd==READ) {
+ old_fs = get_fs();
+ set_fs(get_ds());
+
+ retval=file->f_op->read(file,block_buf,size,&file->f_pos);
+
+ set_fs(old_fs);
+
+ if (retval < 0) {
+ printk(KERN_ERR "loop: cannot read block!");
+ goto error_out_lock;
}
-
}
- }
-
- if (block_present) {
- bh = getblk(lo->lo_device, real_block, blksize);
+
+ if ((lo->transfer)(lo, current_request->cmd, block_buf,
+ dest_addr, size)) {
+ printk(KERN_ERR "loop: transfer error block %d\n", block);
+ goto error_out_lock;
+ }
+
+ if (current_request->cmd==WRITE) {
+ old_fs = get_fs();
+ set_fs(get_ds());
+ inode = file->f_dentry->d_inode;
+ down(&inode->i_sem);
+
+ retval = file->f_op->write(file, block_buf, size, &file->f_pos);
+
+ up(&inode->i_sem);
+ set_fs(old_fs);
+
+ if (retval < 0) {
+ printk(KERN_ERR "loop: cannot write block!");
+ goto error_out_lock;
+ }
+ }
+ } else { /* It is a block device, so use block driver routines */
+ bh = getblk(lo->lo_device, block, blksize);
if (!bh) {
printk(KERN_ERR "loop: device %s: getblk(-, %d, %d) returned NULL",
kdevname(lo->lo_device),
@@ -274,61 +307,6 @@
CURRENT=current_request;
end_request(0);
goto repeat;
-}
-
-static int create_missing_block(struct loop_device *lo, int block, int blksize)
-{
- struct file *file;
- loff_t new_offset;
- char zero_buf[1] = { 0 };
- ssize_t retval;
- mm_segment_t old_fs;
- struct inode *inode;
-
- file = lo->lo_backing_file;
- if (file == NULL) {
- printk(KERN_WARNING "loop: cannot create block - no backing file\n");
- return FALSE;
- }
-
- if (file->f_op == NULL) {
- printk(KERN_WARNING "loop: cannot create block - no file ops\n");
- return FALSE;
- }
-
- new_offset = block * blksize;
-
- if (file->f_op->llseek != NULL) {
- file->f_op->llseek(file, new_offset, 0);
- } else {
- /* Do what the default llseek() code would have done */
- file->f_pos = new_offset;
- file->f_reada = 0;
- file->f_version = ++event;
- }
-
- if (file->f_op->write == NULL) {
- printk(KERN_WARNING "loop: cannot create block - file not writeable\n");
- return FALSE;
- }
-
- old_fs = get_fs();
- set_fs(get_ds());
-
- inode = file->f_dentry->d_inode;
- down(&inode->i_sem);
- retval = file->f_op->write(file, zero_buf, 1, &file->f_pos);
- up(&inode->i_sem);
-
- set_fs(old_fs);
-
- if (retval < 0) {
- printk(KERN_WARNING "loop: cannot create block - FS write failed: code %d\n",
- retval);
- return FALSE;
- } else {
- return TRUE;
- }
}
static int loop_set_fd(struct loop_device *lo, kdev_t dev, unsigned int arg)
--V0207lvV8h4k8FAm--
-
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/