Re: Kernel is unstable

[torvalds@transmeta.com (Linus Torvalds)]

In article <20010301193017.E15051@athlon.random>,
Andrea Arcangeli  <andrea@suse.de> wrote:
>On Thu, Mar 01, 2001 at 06:20:49PM +0000, Alan Cox wrote:
>> > It's not broken, it's not there any longer as somebody dropped it between test7
>> > and 2.4.2, may I ask why?
>> 
>> Linus took it out because it was breaking things.
>
>If it happened to be buggy it didn't looked unfixable from a design standpoint
>and I think it was a very worthwhile feature, not just for memory but also to
>avoid growing the size of the avl that we would have to pay later all the time
>at each page fault.

The locking order was rather nasty in it (mapping->i_shared_lock and
mm->page_table_lock), and made a lot of the code much less readable than
it should have been.  And because none of the callers could know whether
the vma existed after being merged, they ended up doing strange things
that simply aren't necessary with the much simpler version. 

This, coupled with the fact that many merges could be done trivially by
hand (much faster), made me drop it.  There were a few places where it
was used where I couldn't make myself be sure that the locking was
right: I could not prove that it was buggy, but I couldn't convince
myself that it wasn't, either. 

Note how do_brk() does the merging itself (see the comment "Can we just
expand an old anonymous mapping?"), and that it's basically free when
done that way, with no worries about locking etc. The same could be done
fairly trivially in mmap too, but I never saw any real usage patterns
that made it look all that worthwhile (*). Handling the mmap case the
same way do_brk() does it would fix the behaviour of this pathological
example too..

Also note that the merging tests were not free, so at least under my set
of normal load the non-merging code is actually _faster_ than the clever
optimized merging. That was what clinched it for me: I absolutely hate
to see complexity that doesn't really buy you anything noticeable.

			Linus

(*) The only "testing" I did was really running normal applications and
then checking how many merges could be done on /proc/*/maps. Under
normal load I did not see very many at all - I had something like six
missed merges while running my normal set of applications (X, KDE etc).
Others can obviously have very different usage patterns.