headers
James Bottomley | 4 Apr 09:23
Favicon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

On Mon, 2006-04-03 at 15:21 +0200, Miklos Szeredi wrote:
>   flush_dcache_page():
>      - The kernel address is flushed regardless whether the page is
>        anonymous or not

Not quite ... if the page is file backed but has no user mappings, on
the page dirty bit will be set (the kernel view won't be flushed).

>      - If the page is file backed, then all user addresses refering to
>        the page are flushed

Yes, that's what it does.

> Why this discrepancy between anonymous and file backed pages?
> Wounldn't it be enough for file backed pages too to flush only one
> user address?

Not necessarily ... you're getting deep into how VIPT and VIVT caches
work.  VIVT and non-CAM based VIPT caches need every alias flushed
(well, that's not congruent, anyway ... we try to keep congruence in
parisc, but it's not always possible).  Usually a page is either file
backed or anonymous, so for an anonymous page, we wouldn't know the user
address to flush even if it were congruent.

> Added to the mix are copy_to/from_user_page() which already seem to do
> the above, and are used in combination with get_user_pages() which
> results in multiple redundant cache flushes.  Not too clean, is it?

I don't see that they do.  If flush_dcache_page() also does anon pages,
then the arch implementation of flush_anon_page() will be empty.  If it
(Continue reading)

Permalink | Reply |
headers
Miklos Szeredi | 4 Apr 10:50
Picon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

> >   flush_dcache_page():
> >      - The kernel address is flushed regardless whether the page is
> >        anonymous or not
> 
> Not quite ... if the page is file backed but has no user mappings, on
> the page dirty bit will be set (the kernel view won't be flushed).

Yes, but in this case (page obtained from get_user_pages()) it
obviously does have a user mapping.

> >      - If the page is file backed, then all user addresses refering to
> >        the page are flushed
> 
> Yes, that's what it does.
> 
> > Why this discrepancy between anonymous and file backed pages?
> > Wounldn't it be enough for file backed pages too to flush only one
> > user address?
> 
> Not necessarily ... you're getting deep into how VIPT and VIVT caches
> work.  VIVT and non-CAM based VIPT caches need every alias flushed
> (well, that's not congruent, anyway ... we try to keep congruence in
> parisc, but it's not always possible).  Usually a page is either file
> backed or anonymous, so for an anonymous page, we wouldn't know the user
> address to flush even if it were congruent.

I still don't see _why_ you need all aliases flushed from
get_user_pages() when you are only accessing the page through a single
address.
(Continue reading)

Permalink | Reply |
headers
James Bottomley | 4 Apr 17:40
Favicon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

On Tue, 2006-04-04 at 10:50 +0200, Miklos Szeredi wrote:
> I still don't see _why_ you need all aliases flushed from
> get_user_pages() when you are only accessing the page through a single
> address.

Because the aliases may contain dirty cache lines.  Even if they only
contained clean cache lines, those lines would then obscure the changed
data is anyone accessed them.  What you're trying to do is to alter the
user's view of a page by modifying the kernel's view of it.  This is
what gets you into the caching problems in the first place.

> Think of it this way: 
> 
> get_user_pages(... write=0 ...) + memcpy() is equivalent to
> copy_from_user()
> 
> get_user_pages(... write=1 ...) + memcpy() + flush_kernel_dcache_page()
> is equivalent to copy_to_user()
> 
> copy_from_user() and copy_to_user() don't care about aliases, do they?

No, because if you look at the implementation, you'll see that
copy_to/from_user() copy straight into the user view (i.e. via the user
cache lines).

> > > Added to the mix are copy_to/from_user_page() which already seem to do
> > > the above, and are used in combination with get_user_pages() which
> > > results in multiple redundant cache flushes.  Not too clean, is it?
> > 
> > I don't see that they do.  If flush_dcache_page() also does anon pages,
(Continue reading)

Permalink | Reply |
headers
Miklos Szeredi | 4 Apr 18:07
Picon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

> > I still don't see _why_ you need all aliases flushed from
> > get_user_pages() when you are only accessing the page through a single
> > address.
> 
> Because the aliases may contain dirty cache lines.  Even if they only
> contained clean cache lines, those lines would then obscure the changed
> data is anyone accessed them.  What you're trying to do is to alter the
> user's view of a page by modifying the kernel's view of it.  This is
> what gets you into the caching problems in the first place.

Understood.

> > Think of it this way: 
> > 
> > get_user_pages(... write=0 ...) + memcpy() is equivalent to
> > copy_from_user()
> > 
> > get_user_pages(... write=1 ...) + memcpy() + flush_kernel_dcache_page()
> > is equivalent to copy_to_user()
> > 
> > copy_from_user() and copy_to_user() don't care about aliases, do they?
> 
> No, because if you look at the implementation, you'll see that
> copy_to/from_user() copy straight into the user view (i.e. via the user
> cache lines).

Yes.  But how will this ensure that the above problems (dirty/clean
cache lines in aliases) won't cause any problems?

What is the difference between
(Continue reading)

Permalink | Reply |
headers
James Bottomley | 6 Apr 02:07
Favicon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

On Tue, 2006-04-04 at 18:07 +0200, Miklos Szeredi wrote:
> > No, because if you look at the implementation, you'll see that
> > copy_to/from_user() copy straight into the user view (i.e. via the user
> > cache lines).
> 
> Yes.  But how will this ensure that the above problems (dirty/clean
> cache lines in aliases) won't cause any problems?

It doesn't.  The object is to write into the user cache as though the
actual user process had done it (i.e. disregarding all aliasing).

> What is the difference between
> 
>  - updating cached data through the user view
> 
>  - flushing the cache lines for the user view, then updating data
>    through the kernel view and finally flushing the cache lines for
>    the kernel view

In the former, the data usually ends up in dirty user cache lines.  In
the latter the cache lines are entirely clean after the procedure.

> I don't see in either case that the cache lines for any other aliases
> are touched.  Is there still some difference in behavior?

Yes, in the former, only a single alias (the user view) is affected.  In
the latter, all views are accounted for.

James
(Continue reading)

Permalink | Reply |
headers
Miklos Szeredi | 6 Apr 08:53
Picon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

> > > No, because if you look at the implementation, you'll see that
> > > copy_to/from_user() copy straight into the user view (i.e. via the user
> > > cache lines).
> > 
> > Yes.  But how will this ensure that the above problems (dirty/clean
> > cache lines in aliases) won't cause any problems?
> 
> It doesn't.  The object is to write into the user cache as though the
> actual user process had done it (i.e. disregarding all aliasing).

Which is also what the object of get_user_pages() + memcpy() is.  The
fact that in the process some part of the cache is flushed is
irrelevant.

get_user_pages() is used as a _substitute_ for copy_*_user() if the
memory copying needs to be done from a different VM context.

> > What is the difference between
> > 
> >  - updating cached data through the user view
> > 
> >  - flushing the cache lines for the user view, then updating data
> >    through the kernel view and finally flushing the cache lines for
> >    the kernel view
> 
> In the former, the data usually ends up in dirty user cache lines.  In
> the latter the cache lines are entirely clean after the procedure.
> 
> > I don't see in either case that the cache lines for any other aliases
> > are touched.  Is there still some difference in behavior?
(Continue reading)

Permalink | Reply |
headers
James Bottomley | 6 Apr 13:30
Favicon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

On Thu, 2006-04-06 at 08:53 +0200, Miklos Szeredi wrote:
> How about this?

It would double flush the page on a lot of architectures

James

-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
Permalink | Reply |
headers
Miklos Szeredi | 6 Apr 13:56
Picon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

> > How about this?
> 
> It would double flush the page on a lot of architectures

Which?

Miklos

-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
Permalink | Reply |
headers
James Bottomley | 7 Apr 14:25
Favicon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

On Thu, 2006-04-06 at 13:56 +0200, Miklos Szeredi wrote:
> > > How about this?
> > 
> > It would double flush the page on a lot of architectures
> 
> Which?

Well ... all of them.  Anonymous pages coming into get_user_pages() is
pretty much the exception.  You do this for every page.  But anyway,
even if you fixed that, Dave Miller vetoed an implementation based on
flush_cache_page() on the grounds that the implementation is nop'd on
quite a few architectures because of the way it's used ... it was the
first fix I tried to get past linux-arch.

James

-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
Permalink | Reply |
headers
Miklos Szeredi | 7 Apr 21:11
Picon

Re: Re: [parisc-linux] [PATCH] Fixs to work on ARM and PARIC platforms.

> > > > How about this?
> > > 
> > > It would double flush the page on a lot of architectures
> > 
> > Which?
> 
> Well ... all of them.  Anonymous pages coming into get_user_pages() is
> pretty much the exception.

I don't think that holds, since I would imagine most user pages _are_
anonymous.

Ptrace is one of the more notable users of get_user_pages() so I
checked what happens if I run 'strace /bin/true' on an instrumented
kernel.

Anon pages won by 214 to 26.

> You do this for every page.  But anyway, even if you fixed that,
> Dave Miller vetoed an implementation based on flush_cache_page() on
> the grounds that the implementation is nop'd on quite a few
> architectures because of the way it's used

Sounds like another case of a crappy interface.

But I don't care overmuch.  I'll submit a patch for ARM and let this
issue rest in peace.

Anyway, thanks for the fast course on CPU caches :)
(Continue reading)

Permalink | Reply |

Gmane