Re: problems in commit 2d4dc890b5c8 (block: add helpers to run flush_dcache_page() against a bio and a request's pages)

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2009-12-10 at 02:03 +0300, Ilya Loginov wrote:
> On Wed, 09 Dec 2009 16:39:06 -0600
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > Then this extra helpers added to block: rq_flush_dcache_pages() loops
> > over every segment of the request calling flush_dcache_page() for the
> > page.  If you read the mtd code, it acts on a bio at a time so a request
> > with N bios flushes every page in the request N times (i'e N^2 flushes).
> > I'm assuming there's something somewhere I'm missing that restricts mtd
> > to single bio requests, which makes this all OK?
> 
> Sorry. But why do you think that do_blktrans_request is dealing with bio?
> May be i don't know something...

I was basing it on the fact that req->data is bio_data() which is
initialised per bio.  What mtd does can't really work unless it's doing
a bio at a time (or has small limits so that all requests are bios).

> > The pio read case is the problematic one, because you dirty the kernel
> > alias by writing the read data to it and have to flush that before it's
> > made visible to the user alias view.  The API for doing this is
> > flush_kernel_dcache_page() ... it *only* flushes the kernel view, not
> > the user view.  The reason for this is that if the arch has to protect
> > the user aliases against speculative movein, that's done in the DMA API
> > before the request is completed.
> > 
> > So for this:
> > 
> > >	case READ:
> > >		for (; nsect > 0; nsect--, block++, buf += tr->blksize)
> > >			if (tr->readsect(dev, block, buf))
> > >				return -EIO;
> > >+		rq_flush_dcache_pages(req);
> > >		return 0;
> > 
> > Actually all you need to do is loop over the pages and call
> > flush_kernel_dcache_page().
> 
> I don't think so. Please reread our discussion.

As I said previously, I seem to have missed the discussion.

>  I have this bug on system where
> icache don't look for code in dcache. And I need flush dcache exactly in
> physical layer.

icache and dcache are usually separate.  On almost every architecture
you have to flush the dcache to RAM before the icache can pull it in.
However, think about where the data is in the pio read case:  it's in
the cache above the kernel alias.  Once you flush that cache to ram, the
icache can pick it out ... you don't have to reflush all the untouched
user aliases, you just use flush_kernel_dcache_page() which places it in
the ram.

James