copy_one_pte()

copy_one_pte()

[Zoltan Menyhart]

I had a look at copy_one_pte().
I cannot see any ioproc_update_page() call, not even for the COW pages.
Is it intentional?

We can live with a COW page for a considerably long time.
How could the IO-PROC. know that a process-ID / user virt. addr. pair
refers to the same page?

The comment above ioproc_update_page() says that every time when a PTE
is created / modified...

Thanks,

Zoltan Menyhart

Re: copy_one_pte()

[Christoph Hellwig]

On Tue, Mar 13, 2007 at 08:15:25PM +0100, Zoltan Menyhart wrote:
> I had a look at copy_one_pte().
> I cannot see any ioproc_update_page() call, not even for the COW pages.
> Is it intentional?

There is no such thing as ioproc_update_page in any mainline tree.
You must be looking at some vendor tree with braindead patches applied.

Re: copy_one_pte()

[Matt Keenan]

Christoph Hellwig wrote:
> On Tue, Mar 13, 2007 at 08:15:25PM +0100, Zoltan Menyhart wrote:
>   
>> I had a look at copy_one_pte().
>> I cannot see any ioproc_update_page() call, not even for the COW pages.
>> Is it intentional?
>>     
>
> There is no such thing as ioproc_update_page in any mainline tree.
> You must be looking at some vendor tree with braindead patches applied.
>
>   
It looks like this function exists as a part of patches to support 
Quadrics NICs / RDMA (HPC platforms).  The patches are there so the 
driver doesn't need to pin pages, it can be informed of page updates 
directly.  A patch was submitted to l-k sometime in 2005.

Matt

Re: copy_one_pte()

[Andrew Morton]

(cc restored.  Please always do reply-to-all).

> On Wed, 14 Mar 2007 08:35:07 +0000 Matt Keenan <tank.en.mate@gmail.com> wrote:
> Christoph Hellwig wrote:
> > On Tue, Mar 13, 2007 at 08:15:25PM +0100, Zoltan Menyhart wrote:
> >   
> >> I had a look at copy_one_pte().
> >> I cannot see any ioproc_update_page() call, not even for the COW pages.
> >> Is it intentional?
> >>     
> >
> > There is no such thing as ioproc_update_page in any mainline tree.
> > You must be looking at some vendor tree with braindead patches applied.
> >
> >   
> It looks like this function exists as a part of patches to support 
> Quadrics NICs / RDMA (HPC platforms).  The patches are there so the 
> driver doesn't need to pin pages, it can be informed of page updates 
> directly.  A patch was submitted to l-k sometime in 2005.

Oh Dear.

Which vendor's kernel are we talking about here?

Re: copy_one_pte()

[Matt Keenan]

Andrew Morton wrote:
> (cc restored.  Please always do reply-to-all).
>
>   
>> On Wed, 14 Mar 2007 08:35:07 +0000 Matt Keenan <tank.en.mate@gmail.com> wrote:
>> Christoph Hellwig wrote:
>>     
>>> On Tue, Mar 13, 2007 at 08:15:25PM +0100, Zoltan Menyhart wrote:
>>>   
>>>       
>>>> I had a look at copy_one_pte().
>>>> I cannot see any ioproc_update_page() call, not even for the COW pages.
>>>> Is it intentional?
>>>>     
>>>>         
>>> There is no such thing as ioproc_update_page in any mainline tree.
>>> You must be looking at some vendor tree with braindead patches applied.
>>>
>>>   
>>>       
>> It looks like this function exists as a part of patches to support 
>> Quadrics NICs / RDMA (HPC platforms).  The patches are there so the 
>> driver doesn't need to pin pages, it can be informed of page updates 
>> directly.  A patch was submitted to l-k sometime in 2005.
>>     
>
> Oh Dear.
>
> Which vendor's kernel are we talking about here?
>   
I don't know of any vendor's kernels that support this (but then I run 
vanilla kernels on Debian).  I just grep'ed for the patch because it 
sounded interesting.  There was a posting for it to l-k on 26th April 
2005 from David Addison of Quadrics Ltd xref 
http://lkml.org/lkml/2005/4/26/198  According to David, you (Andrew) and 
Andrea Arcangeli asked for it to be posted for some feedback, the main 
feedback was on whitespace issues and COWs w.r.t. fork().  Brice Goglin 
made an interesting comment about using a similar method but tracking 
VMAs rather than address spaces.  By the looks of things it never went 
into the mainline kernel.  I lurk a bit (I sometimes miss things) on l-k 
but I hadn't noticed any other methods for dynamic DMA direct to user 
space (other than pinning pages), is there anything planned?

Matt


p.s. I Cc'ed Brice Goglin and David Addison

Re: copy_one_pte()

[Daniel J Blueman]

On 13 Mar, 20:20, Zoltan Menyhart <Zoltan.Menyh...@free.fr> wrote:
> I had a look at copy_one_pte().
> I cannot see any ioproc_update_page() call, not even for the COW pages.
> Is it intentional?

There could be an ioproc_update_range() call in
memory.c:copy_pte_range(), after the pte_unmap_unlock(), and this
would be an optimisation, since explicitly updating the mappings in
the Quadrics RDMA NIC avoids the need for the NIC to trap and update
it's MMU later.

The code which implements [1] this takes the pagetable locks with
pte_offset_map_lock(), and uses one of the kmap_atomic slots, so has
to be after the pagetables are unlocked. The
ioproc_invalidate_page/range() calls are different and can't live with
this race, so have to be used with the pagetable locks held.

Daniel

--- [1]

http://lwn.net/Articles/133627/
http://www.quadrics.com/linux

> We can live with a COW page for a considerably long time.
> How could the IO-PROC. know that a process-ID / user virt. addr. pair
> refers to the same page?
>
> The comment above ioproc_update_page() says that every time when a PTE
> is created / modified...
>
> Thanks,
>
> ZoltanMenyhart
-- 
Daniel J Blueman

Re: copy_one_pte()

[Daniel J Blueman]

On 03/04/07, Zoltan Menyhart <Zoltan.Menyhart@bull.net> wrote:
> Daniel J Blueman wrote:
> > On 13 Mar, 20:20, Zoltan Menyhart <Zoltan.Menyh...@free.fr> wrote:
> >
> >> I had a look at copy_one_pte().
> >> I cannot see any ioproc_update_page() call, not even for the COW pages.
> >> Is it intentional?
> >
> > There could be an ioproc_update_range() call in
> > memory.c:copy_pte_range(), after the pte_unmap_unlock(), and this
> > would be an optimisation, since explicitly updating the mappings in
> > the Quadrics RDMA NIC avoids the need for the NIC to trap and update
> > it's MMU later.
>
> I can agree that this optimization is a good idea.
>
> Can you please confirm that cp->update_range() understands the concept of
> the COW? (I.e. the card should not write anything into the COW pages.)
>
> > The code which implements [1] this takes the pagetable locks with
> > pte_offset_map_lock(), and uses one of the kmap_atomic slots, so has
> > to be after the pagetables are unlocked. The
> > ioproc_invalidate_page/range() calls are different and can't live with
> > this race, so have to be used with the pagetable locks held.
>
> I can see two issues here:
>
> - Performance problem: page_table_lock is more expensive than a split lock,
>   and taking page_table_lock after the split lock released is even worse.

I have introduced an ioproc_update_page_locked() call for this and
related paths, and updated the kernel patches; this avoids a race
condition I was seeing in multi-threaded processes taking concurrent
page faults under the split locks. The split lock is now acquired
once, and the changes allowed me to do other some optimisations on
this path too.

> - Synchronization problem: do_wp_page() is protected by the appropriate
>   split lock, i.e. the COW page can be broken up while you are inside
>   cp->update_range() under the protection of page_table_lock.

The split locks were used previously via pte_offset_map_lock(), so
this race wasn't possible previously.

> I think the PTE modification and the cp->update_xxx() should be protected
> by the very same lock, without releasing it between the two operations.
> I think the PTE modification and the cp->update_xxx() should be an
> atomic operation with respect to the VMM activity.

This does make most sense and is now the case, although atomiticity
wasn't a problem, since it's fine to use ioproc_update_page/range()
speculatively.

Dan

> Thanks,
>
> Zoltan
-- 
Daniel J Blueman