2007-05-01 11:40:11

by Nick Piggin

[permalink] [raw]
Subject: Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path

Rohit Seth wrote:
>
>
> -----Original Message-----
> From: Nick Piggin [mailto:[email protected]]
> Sent: Friday, April 27, 2007 7:00 PM
> To: [email protected]
> Cc: Mike Stroyan; Andrew Morton; Hugh Dickins; Luck, Tony;
> [email protected]; [email protected]
> Subject: Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path
>
> Rohit Seth wrote:
>
>>
>>>You mean by user space? If so, then it is user space responsibility to
>>>do the appropriate operations (like flush icache in this case).
>
>
>>No, I mean places that set PG_arch_1. flush_dcache_page. This can happen
>>for mapped pages in write, splice, install_arg_page looks questionable,
>
> direct IO...
>
>
> If a user is requesting kernel to do (for example) write on a page that is
> already mapped with execute and write permissions then it should be treated
> as if the user space is doing modifications to that page. There is no
> change in protections so lazy_prot_mmu_update shouldn't be called even
> though PG_arch_1 is (I think) set. Does it answer your concern?

I'm not sure that I would agree. For direct modifications of memory via
a passed in user virtual address, perhaps. For operations on pagecache,
we may not even have a handle to issue the flush cache instruction on (ie.
a user virtual address), let alone know whether anyone else is mapping
the page.

>>What if you were to say remove all the PG_arch_1 code, and do
>>something really simple like flush icache in
>>flush_dcache_page? Would performance suffer horribly?
>
>
> On Itanium, I think it will have some performance penalty (horrible or not I
> don't know) as you will be invalidating the caches more often. And they
> alsways look for last 0.1% performance that they can get.

Sure, but if we _only_ flushed when page_mapcount was raised, then we
should have most of the benefits of lazy flushing, and the main places
were we do extra flushes are those where aliases could potentially occur
under the old scheme.

--
SUSE Labs, Novell Inc.


2007-05-02 00:36:43

by Rohit Seth

[permalink] [raw]
Subject: Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path

On Tue, 2007-05-01 at 21:39 +1000, Nick Piggin wrote:
> Rohit Seth wrote:

> >
> > If a user is requesting kernel to do (for example) write on a page that is
> > already mapped with execute and write permissions then it should be treated
> > as if the user space is doing modifications to that page. There is no
> > change in protections so lazy_prot_mmu_update shouldn't be called even
> > though PG_arch_1 is (I think) set. Does it answer your concern?
>
> I'm not sure that I would agree. For direct modifications of memory via
> a passed in user virtual address, perhaps. For operations on pagecache,
> we may not even have a handle to issue the flush cache instruction on (ie.
> a user virtual address), let alone know whether anyone else is mapping
> the page.
>

Can you please describe the page cache scenario in more detail? IMO, if
a page is user mapped with at least one execute and write permission
then the responsibility of update caches lies with user.

> >>What if you were to say remove all the PG_arch_1 code, and do
> >>something really simple like flush icache in
> >>flush_dcache_page? Would performance suffer horribly?
> >
> >
> > On Itanium, I think it will have some performance penalty (horrible or not I
> > don't know) as you will be invalidating the caches more often. And they
> > alsways look for last 0.1% performance that they can get.
>
> Sure, but if we _only_ flushed when page_mapcount was raised,

You will need this every time there is change in protection (e.g.
mprotect) not only when page_mapcount is raised.

-rohit


2007-05-02 01:57:52

by Nick Piggin

[permalink] [raw]
Subject: Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path

Rohit Seth wrote:
> On Tue, 2007-05-01 at 21:39 +1000, Nick Piggin wrote:
>
>>Rohit Seth wrote:
>
>
>>>If a user is requesting kernel to do (for example) write on a page that is
>>>already mapped with execute and write permissions then it should be treated
>>>as if the user space is doing modifications to that page. There is no
>>>change in protections so lazy_prot_mmu_update shouldn't be called even
>>>though PG_arch_1 is (I think) set. Does it answer your concern?
>>
>>I'm not sure that I would agree. For direct modifications of memory via
>>a passed in user virtual address, perhaps. For operations on pagecache,
>>we may not even have a handle to issue the flush cache instruction on (ie.
>>a user virtual address), let alone know whether anyone else is mapping
>>the page.
>>
>
>
> Can you please describe the page cache scenario in more detail? IMO, if
> a page is user mapped with at least one execute and write permission
> then the responsibility of update caches lies with user.

What if a different user write(2)s the underlying page?


>>>>What if you were to say remove all the PG_arch_1 code, and do
>>>>something really simple like flush icache in
>>>>flush_dcache_page? Would performance suffer horribly?
>>>
>>>
>>>On Itanium, I think it will have some performance penalty (horrible or not I
>>>don't know) as you will be invalidating the caches more often. And they
>>>alsways look for last 0.1% performance that they can get.
>>
>>Sure, but if we _only_ flushed when page_mapcount was raised,
>
>
> You will need this every time there is change in protection (e.g.
> mprotect) not only when page_mapcount is raised.

Yeah, you would retain the flush on fault, I meant you would
introduce a flush in flush_dcache_page for when mapcount is raised.

--
SUSE Labs, Novell Inc.