2004-06-01 17:03:58

by Russell King

[permalink] [raw]
Subject: [PATCH] Fix loop device cache handling

Back in the distant LKML past, davem hinted that the loop block device
driver was missing some cache handling for aliasing caches:

http://lkml.org/lkml/2003/5/29/19

It appears the loop driver has had one flush_dcache_page() call
added for the case where it writes to the backing device page
cache pages.

However, it seems to be missing the call where it writes to its
own page cache pages. This patch allows us to clearly demonstrate
the problem on ARM:

> @@ -85,9 +85,10 @@ static int transfer_none(struct loop_dev
> char *raw_buf = kmap_atomic(raw_page, KM_USER0) + raw_off;
> char *loop_buf = kmap_atomic(loop_page, KM_USER1) + loop_off;
>
> - if (cmd == READ)
> + if (cmd == READ) {
> +int i; for (i = 0; i < PAGE_SIZE; i+=32) ((volatile unsigned char *)loop_buf)[i];
> memcpy(loop_buf, raw_buf, size);
> - else
> + } else
> memcpy(raw_buf, loop_buf, size);
>
> kunmap_atomic(raw_buf, KM_USER0);

The effect of this is that we pull the loop device page into cache
to exhasibate the problem, since newly allocated pages are _not_
guaranteed to be completely clean of cache lines in their kernel
space mapping.

Note that other drivers need to be audited to ensure that any CPU
writes to page cache pages have a flush_dcache_page() call.

This patch adds the necessary missing flush:

--- orig/drivers/block/loop.c Sun May 30 10:32:32 2004
+++ linux/drivers/block/loop.c Tue Jun 1 17:46:47 2004
@@ -308,7 +308,9 @@ lo_read_actor(read_descriptor_t *desc, s
page->index);
desc->error = -EINVAL;
}
-
+
+ flush_dcache_page(p->page);
+
desc->count = count - size;
desc->written += size;
p->offset += size;


--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core


2004-06-02 04:57:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] Fix loop device cache handling

On Tue, 1 Jun 2004 18:03:36 +0100
Russell King <[email protected]> wrote:

> The effect of this is that we pull the loop device page into cache
> to exhasibate the problem, since newly allocated pages are _not_
> guaranteed to be completely clean of cache lines in their kernel
> space mapping.
>
> Note that other drivers need to be audited to ensure that any CPU
> writes to page cache pages have a flush_dcache_page() call.
>
> This patch adds the necessary missing flush:

I %100 agree with this patch.