flush_dcache_page does too much?
anfei.zhou at gmail.com
Tue Jan 19 08:05:47 EST 2010
On Tue, Jan 19, 2010 at 08:16:36AM +0800, anfei wrote:
> On Mon, Jan 18, 2010 at 03:01:52PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Jan 18, 2010 at 10:57:31PM +0800, anfei wrote:
> > > On Mon, Jan 18, 2010 at 02:44:18PM +0000, Russell King - ARM Linux wrote:
> > > > On Mon, Jan 18, 2010 at 10:15:30PM +0800, anfei wrote:
> > > > > On Mon, Jan 18, 2010 at 02:00:05PM +0000, Russell King - ARM Linux wrote:
> > > > > > On Mon, Jan 18, 2010 at 09:54:31PM +0800, anfei wrote:
> > > > > > > Do you mean this implementation can ensure the coherence between write
> > > > > > > and shared mmapings? But it's easy to reproduce the alias problem by
> > > > > > > this simple testcase (w/o error handler) on omap2430 with VIPT cache:
> > > > > >
> > > > > > Your program doesn't do anything to identify any problem. You don't
> > > > > > even say _what_ problem you see with this program.
> > > > > >
> > > > > Sorry for that.
> > > > >
> > > > > > If you have a specific case which fails, please show the problem, please
> > > > > > describe exactly the behaviour that you see, and what you expect to see.
> > > >
> > > > Are you using a write allocate cache?
> > >
> > > I guess not, because this line is neccessary to reproduce the issue:
> > > tmp = *(addr+0);
> > > If it's write allocate, this line may not be neccessary, since it's just
> > > a read (and cache the data).
> > It makes no sense then - without write allocate, writes will go straight
> > through to the underlying page, bypassing the cache.
> Because of the read, the write is cache hitted too even on read allocate:
> *(addr+0) = 0x44444444; <- bypass the cache
> tmp = *(addr+0); <- read allocate
> *(addr+1) = 0x77777777; <- same cacheline, cache hitted
> So the two write values are cached, then the sequence in sys_write
> cannot guarantee the coherence:
> copy to page;
> It should call flush_dcache_page() at the beginning too in order to
> flush the shared mapping. Actually, I think it's better to split this
> function into two, such as:
> copy to page;
> And this patch seems to fix it, any other fs doesn't call it need to add
> that too.
Do you think this is a bug and I can send the patch, or it's the problem
of the test case?
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 96ac6b0..07056fb 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2196,6 +2196,9 @@ again:
> if (unlikely(status))
> + if (mapping_writably_mapped(mapping))
> + flush_dcache_page(page);
> copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes);
More information about the linux-arm-kernel