[PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Rik van Riel]

Hi,

Almost the same patch as before, except this one has had
a few hours of testing by Andrew Morton and two bugs have
been ironed out, most notably the truncate_complete_page()
race.  This patch is probably safe since Andrew got bored
when no new bugs showed up ...

If you have some time left this weekend and feel brave,
please test the patch which can be found at:

	http://surriel.com/patches/2.5/2.5.25-rmap-akpmtested

This patch is based on Craig Kulesa's minimal rmap patch
for 2.5.24, with a few changes:
- removed a few unrelated changes
- updated armv/rmap.h for new pagetable layout of linux/arm
- dropped per-zone pte_chain freelists, we want to make per-cpu
  ones for SMP scalability
- ported to 2.5.25 (PF_NOWARN instead of PF_RADIX_TREE)
- drop spelling and whitespace fixes (should be merged separately)
- fix truncate_complete_page race condition (akpm)

It should be mostly ready for being integrated into the 2.5 tree,
with the note that pte-highmem support still needs to be implemented
(some IBMers have been volunteered for this task, this functionality
can easily be added afterwards).

Right now this patch needs testing and careful scrutiny. If you can
find anything wrong with it, please let me know.

kind regards,

Rik
-- 
Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Andrew Morton]

Rik van Riel wrote:
> 
> Hi,
> 
> Almost the same patch as before, except this one has had
> a few hours of testing by Andrew Morton and two bugs have
> been ironed out, most notably the truncate_complete_page()
> race.  This patch is probably safe since Andrew got bored
> when no new bugs showed up ...
> 

The box died, but not due to rmap.  We have a lock ranking
bug:

        do_exit
        ->mmput
          ->exit_mmap                           page_table_lock
            ->removed_shared_vm_struct
              ->lock_vma_mappings               i_shared_lock

versus

        do_truncate
        ->notify_change
          ->inode_setattr
            ->vmtruncate                        i_shared_lock
              ->vmtruncate_list
                ->zap_page_range                page_table_lock

It seems that in 2.5.16, a call to remove_shared_vm_struct() was
added to exit_mmap(), inside mm->page_table_lock.

That ranking conflicts with truncate.

-
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Linus Torvalds]

On Fri, 5 Jul 2002, Andrew Morton wrote:
>
> The box died, but not due to rmap.  We have a lock ranking
> bug:
>
>         do_exit
>         ->mmput
>           ->exit_mmap                           page_table_lock
>             ->removed_shared_vm_struct
>               ->lock_vma_mappings               i_shared_lock

I _think_ we should just move the remove_shared_vm_struct() down into the
case where we're closing the mapping, ie something like the appended.

That way we _only_ do the actual page table stuff under the page table
lock, and do all the generic VM/FS stuff outside the lock.

Comments?

		Linus

-------------------------------
--- 1.34/mm/mmap.c	Thu Jun 27 00:35:55 2002
+++ edited/mm/mmap.c	Sat Jul  6 12:06:02 2002
@@ -1121,7 +1121,6 @@
 		unsigned long end = mpnt->vm_end;

 		mm->map_count--;
-		remove_shared_vm_struct(mpnt);
 		unmap_page_range(tlb, mpnt, start, end);
 		mpnt = mpnt->vm_next;
 	}
@@ -1148,6 +1147,7 @@
 	 */
 	while (mpnt) {
 		struct vm_area_struct * next = mpnt->vm_next;
+		remove_shared_vm_struct(mpnt);
 		if (mpnt->vm_ops) {
 			if (mpnt->vm_ops->close)
 				mpnt->vm_ops->close(mpnt);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Andrew Morton]

Linus Torvalds wrote:
> 
> On Fri, 5 Jul 2002, Andrew Morton wrote:
> >
> > The box died, but not due to rmap.  We have a lock ranking
> > bug:
> >
> >         do_exit
> >         ->mmput
> >           ->exit_mmap                           page_table_lock
> >             ->removed_shared_vm_struct
> >               ->lock_vma_mappings               i_shared_lock
> 
> I _think_ we should just move the remove_shared_vm_struct() down into the
> case where we're closing the mapping, ie something like the appended.
> 
> That way we _only_ do the actual page table stuff under the page table
> lock, and do all the generic VM/FS stuff outside the lock.
> 
> Comments?

That is basically what do_munmap() does.  But I'm quite unfamiliar
with the locking in there.

-
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Linus Torvalds]

On Sat, 6 Jul 2002, Andrew Morton wrote:
>
> That is basically what do_munmap() does.  But I'm quite unfamiliar
> with the locking in there.

The only major user of i_shared is really vmtruncate, I think, and it's
quite ok to unmap the file before removing the mapping from the shared
list - if vmtruncate finds a unmapped area, it just won't be doing
anything (zap_page_range, but that won't do anything without any page
tables).

Together with the fact that unmap() already does it this way anyway, it
looks like the obvious fix..

		Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Sebastian Droege]

[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]

On Sat, 6 Jul 2002 02:31:38 -0300 (BRT)
Rik van Riel <riel@conectiva.com.br> wrote:

> Hi,
> 
> Almost the same patch as before, except this one has had
> a few hours of testing by Andrew Morton and two bugs have
> been ironed out, most notably the truncate_complete_page()
> race.  This patch is probably safe since Andrew got bored
> when no new bugs showed up ...
> 
> If you have some time left this weekend and feel brave,
> please test the patch which can be found at:
> 
> 	http://surriel.com/patches/2.5/2.5.25-rmap-akpmtested
> 
> This patch is based on Craig Kulesa's minimal rmap patch
> for 2.5.24, with a few changes:
> - removed a few unrelated changes
> - updated armv/rmap.h for new pagetable layout of linux/arm
> - dropped per-zone pte_chain freelists, we want to make per-cpu
>   ones for SMP scalability
> - ported to 2.5.25 (PF_NOWARN instead of PF_RADIX_TREE)
> - drop spelling and whitespace fixes (should be merged separately)
> - fix truncate_complete_page race condition (akpm)
> 
> It should be mostly ready for being integrated into the 2.5 tree,
> with the note that pte-highmem support still needs to be implemented
> (some IBMers have been volunteered for this task, this functionality
> can easily be added afterwards).
> 
> Right now this patch needs testing and careful scrutiny. If you can
> find anything wrong with it, please let me know.
> 
> kind regards,
> 
> Rik
Hi,
after running your patch some time I have to say that the old VM implementation and the full rmap patch (by Craig Kulesa) was better.
The system becomes very slow and has to swap in too much after some uptime (4 hours - 2 days) and memory intensive tasks...
Maybe this happens only to me but it's fully reproducable
If you need some more informations just ask

Bye

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Rik van Riel]

On Wed, 10 Jul 2002, Sebastian Droege wrote:
> On Sat, 6 Jul 2002 02:31:38 -0300 (BRT)
> Rik van Riel <riel@conectiva.com.br> wrote:
>
> > If you have some time left this weekend and feel brave,
> > please test the patch which can be found at:
> >
> > 	http://surriel.com/patches/2.5/2.5.25-rmap-akpmtested

> after running your patch some time I have to say that the old VM
> implementation and the full rmap patch (by Craig Kulesa) was better. The
> system becomes very slow and has to swap in too much after some uptime
> (4 hours - 2 days) and memory intensive tasks...
> Maybe this happens only to me but it's fully reproducable

It's a known problem with use-once. Users of plain 2.4.18
are complaining about it, too.

This is something to touch on after the rmap mechanism
has been merged, Linus has indicated that he wants to merge
the thing in small bits so that's what we'll be doing ;)

kind regards,

Rik
-- 
Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Daniel Phillips]

On Wednesday 10 July 2002 22:42, Rik van Riel wrote:
> On Wed, 10 Jul 2002, Sebastian Droege wrote:
> > On Sat, 6 Jul 2002 02:31:38 -0300 (BRT)
> > Rik van Riel <riel@conectiva.com.br> wrote:
> >
> > > If you have some time left this weekend and feel brave,
> > > please test the patch which can be found at:
> > >
> > > 	http://surriel.com/patches/2.5/2.5.25-rmap-akpmtested
> 
> > after running your patch some time I have to say that the old VM
> > implementation and the full rmap patch (by Craig Kulesa) was better. The
> > system becomes very slow and has to swap in too much after some uptime
> > (4 hours - 2 days) and memory intensive tasks...
> > Maybe this happens only to me but it's fully reproducable
> 
> It's a known problem with use-once. Users of plain 2.4.18
> are complaining about it, too.

Hey, thanks Rik, I know something about that :-)  And I'd be testing right
now to see if you're right, if the DAC960 driver compiled successfully.
But it doesn't, and since my test machine won't boot without it... given a
choice between diving into the driver and going back to work on directory
hashing on 2.4...

The tree that builds wins this time.

> This is something to touch on after the rmap mechanism
> has been merged, Linus has indicated that he wants to merge
> the thing in small bits so that's what we'll be doing ;)

I bet it's something a lot dumber, like a memory leak.

-- 
Daniel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Jens Axboe]

On Wed, Jul 10 2002, Daniel Phillips wrote:
> On Wednesday 10 July 2002 22:42, Rik van Riel wrote:
> > On Wed, 10 Jul 2002, Sebastian Droege wrote:
> > > On Sat, 6 Jul 2002 02:31:38 -0300 (BRT)
> > > Rik van Riel <riel@conectiva.com.br> wrote:
> > >
> > > > If you have some time left this weekend and feel brave,
> > > > please test the patch which can be found at:
> > > >
> > > > 	http://surriel.com/patches/2.5/2.5.25-rmap-akpmtested
> > 
> > > after running your patch some time I have to say that the old VM
> > > implementation and the full rmap patch (by Craig Kulesa) was better. The
> > > system becomes very slow and has to swap in too much after some uptime
> > > (4 hours - 2 days) and memory intensive tasks...
> > > Maybe this happens only to me but it's fully reproducable
> > 
> > It's a known problem with use-once. Users of plain 2.4.18
> > are complaining about it, too.
> 
> Hey, thanks Rik, I know something about that :-)  And I'd be testing right
> now to see if you're right, if the DAC960 driver compiled successfully.
> But it doesn't, and since my test machine won't boot without it... given a
> choice between diving into the driver and going back to work on directory
> hashing on 2.4...

Leonard has promised me to convert DAC960 to the "new" pci dma api for
years (or so it seems, actual date may vary, no purchase necessary). I
do have a Mylex controller here myself these days, so it's not
completely impossible that I may do it on a rainy day.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Daniel Phillips]

On Thursday 11 July 2002 08:47, Jens Axboe wrote:
> On Wed, Jul 10 2002, Daniel Phillips wrote:
> > ...I'd be testing right
> > now to see if you're right, if the DAC960 driver compiled successfully.
> > But it doesn't, and since my test machine won't boot without it... given a
> > choice between diving into the driver and going back to work on directory
> > hashing on 2.4...
> 
> Leonard has promised me to convert DAC960 to the "new" pci dma api for
> years (or so it seems, actual date may vary, no purchase necessary). I
> do have a Mylex controller here myself these days, so it's not
> completely impossible that I may do it on a rainy day.

Well, tell me what the new api is and I'll dive in there.  For the record,
what happened to the old one?  Backwards compatibility dropped recently?
Mea culpa for not knowing, but those dma api threads were just so bushy.

I wouldn't be surprised if some other little things have rotted as well.

-- 
Daniel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH][RFT](2) minimal rmap for 2.5 - akpm tested

[Jens Axboe]

On Thu, Jul 11 2002, Daniel Phillips wrote:
> On Thursday 11 July 2002 08:47, Jens Axboe wrote:
> > On Wed, Jul 10 2002, Daniel Phillips wrote:
> > > ...I'd be testing right
> > > now to see if you're right, if the DAC960 driver compiled successfully.
> > > But it doesn't, and since my test machine won't boot without it... given a
> > > choice between diving into the driver and going back to work on directory
> > > hashing on 2.4...
> > 
> > Leonard has promised me to convert DAC960 to the "new" pci dma api for
> > years (or so it seems, actual date may vary, no purchase necessary). I
> > do have a Mylex controller here myself these days, so it's not
> > completely impossible that I may do it on a rainy day.
> 
> Well, tell me what the new api is and I'll dive in there.  For the record,

Documentation/DMA-mapping.txt. Also, DAC960 initial bio conversion
happened before the interface was finalized, so it may need changes in
that regard as well. Documentation/block/biodoc.txt is your friend there
:-)

a quick make drivers/block/DAC960.o shows the following stuff needs
changing immediately:

1) q->queue_lock is a pointer to a lock, not the lock itself. Probably
add a per-controller spinlock to DAC960_Controller_T, and pass that to
blk_init_queue(). Then change DAC960_AcquireControllerLock and friends
in DAC960.h accordingly.

2) wrt DMA mapping, see DAC960_BA_WriteHardwareMailbox
   (Virtual_to_Bus64, anyone?)...

And probably lots more will unearth once you start tackling it...

> I wouldn't be surprised if some other little things have rotted as well.

Heh, not at all :-)

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/