From: Blaisorblade <bla...@ya...> - 2005-08-12 18:28:12
|
Ok, I've been working for the past two weeks learning well the Linux VM, understanding the Ingo's remap_file_pages protection support and its various weakness (due to lack of time on his part), and splitting and finishing it. Here follow a series of 39 _little_ patches against the git-commit-id 889371f61fd5bb914d0331268f12432590cf7e85, which means between 2.6.13-rc4 and -rc5. Actually, the first 7 ones are unrelated trivial cleanups which somehow get in the way on this work and that can probably be merged even now (many are just comment fixes). Since I was a VM newbie until two weeks ago, I've separated my changes into many little patches. To avoid the noise, I'm CC:ing many people only on this message, while I'm sending the full patch series only to akpm, mingo and LKML. Or actually, I'm trying - my provider seem not to like me sending so many patches. I attached an exported tarball to this mail, since it's very little. I hope these changes can be included inside -mm, but I guess that they'll probably conflict with pagefault scalability patches, and that some of them are not completely polished. Still, the patch is IMHO in better shape, in many ways, than when it was in -mm last time. I'll appreciate any comments. ============== Changes from 2.6.5-mm1/dropped version of the patches: ============== *) Actually implemented _real_ and _anal_ protection support, safe against swapout; programs get SIGSEGV *always* when they should. I've used the attached test program (an improved version of Ingo's one) to check that. I tested just until patch 25, onto UML. The subsequent ones are either patches for foreign archs or proposed *) Fixed many changes present in the patches. *) Fixed UML bits *) Added several headaches for arches ports. I've also included some patches which reduce this *) No more usage of a new syscall slot: to use the new interface, application will use the new MAP_NOINHERIT flag I've added. I've still the patches to use the old -mm ABI, if there's any reason they're needed. *) Fixed a regression wrt using mprotect() against remapped area (see patch 15) ====== Still to do: ====== *) fix mprotect VS remap_file_pages(MAP_NOINHERIT) interaction - see long discussion in patch 15 changelog *) ->populate flushes each TLB individually, instead of using mmu_gathers as it should; this was suggested even by Ingo when sending the patch, but it seems he didn't get the time to finish this. Seems rewriting the kernel locking is a quite time-consuming task! ====== Patch summaries ====== Each patch has an attached changelog, but I'm giving here a summary (sorry for using the patch numbers, but I found no other way). The first 7 are just generic cleanups (mostly for comments) which bugged me along the way, however some of them are needed for the subsequent patches to apply. 08-11 ones are arch bits for some arches (the ones I have access too). 12 is the core change for generic code, 13-17 are various changes to the syscall code, as 20, 21 and 23, 35 and 36, to review individually. Most of those changes (except #23, which is a fix for try_to_unmap_one I missed initially) are just speedups, and it should be possible to individually drop them. 18, 19, 22, 32, 33, 34 move partially the handling of protection checks from the arches' page faults handler to the generic code, by introducing VM_FAULT_SIGSEGV. In fact, the VMA protection are not reliable for VM_NONUNIFORM areas. This aspect was just begun in Ingo's code, and was the weakest area of his patch. I must now pass the *full* kind of fault to the generic code, and test it against the PTE or possibly the VMA protections. However, in these patches it's done in a kludgy way, because we check the VMA protections against VM_READ/WRITE/EXEC with no consideration of the architecture-specific dependencies between them (like READ_IMPLIES_EXEC and so on), so arches have to workaround this. This is fixed in patch 33, which is untested however. 24 and 25 are some fixes for UML code, needed to make it work even with this change. 26-31 are other arch's compile fix for the introduction of pte_to_pgoff. The last three ones (37-39) are not to apply - they are some possible changes I'm either really uncertain about, or which I'm sure are wrong in that form but express possibly correct ideas. 36 should be a fixed version of the #37 one, but I wrote it in the past few minutes. -- Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!". Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894) http://www.user-mode-linux.org/~blaisorblade |
From: David S. M. <da...@da...> - 2005-08-12 18:30:04
|
Please do not BOMB linux-kernel with 39 patches in one go, that will kill the list server. Try to consolidate your patch groups into smaller pieces, like so about 10 or 15 at a time. And send any that remain on some later date. |
From: Blaisorblade <bla...@ya...> - 2005-08-12 18:57:24
|
On Friday 12 August 2005 20:29, David S. Miller wrote: > Please do not BOMB linux-kernel with 39 patches in one > go, that will kill the list server. > Try to consolidate your patch groups into smaller pieces, > like so about 10 or 15 at a time. And send any that remain > on some later date. Whoops - some later date for me means a week unfortunately or some minutes. I'm trying for the latter. However, I sent the initial tarball containing all them, so I hope that will be useful. -- Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!". Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894) http://www.user-mode-linux.org/~blaisorblade ___________________________________ Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB http://mail.yahoo.it |
From: David S. M. <da...@da...> - 2005-08-12 19:05:23
|
From: Blaisorblade <bla...@ya...> Date: Fri, 12 Aug 2005 20:56:11 +0200 > However, I sent the initial tarball containing all them, so I hope > that will be useful. So not only did you spam the list with 40 patch postings, you sent a second copy of everything as a tarball attachment as well. That's even worse. Please do not abuse the list server in this way, it is a resource that is not just your's, but rather one which has to be shared amongst all folks doing kernel development. |
From: Ingo M. <mi...@el...> - 2005-08-14 01:59:36
|
* David S. Miller <da...@da...> wrote: > From: Blaisorblade <bla...@ya...> > Date: Fri, 12 Aug 2005 20:56:11 +0200 > > > However, I sent the initial tarball containing all them, so I hope > > that will be useful. > > So not only did you spam the list with 40 patch postings, you sent a > second copy of everything as a tarball attachment as well. That's > even worse. > > Please do not abuse the list server in this way, it is a resource that > is not just your's, but rather one which has to be shared amongst all > folks doing kernel development. while i agree that 40 patches is not common, i'd like to point out that Paolo has sent 40 very nicely split up patches instead of one unreviewable monolithic patch, which are the encouraged format for kernel changes. I havent seen any hard limit mentioned for patch-bombs on lkml before - and i've seen much larger patchbombs going to lkml as well, without any followup chastising of the submitter. E.g.: Subject: [0/48] Suspend2 2.1.9.8 for 2.6.12 So if there needs to be some limit, it might be worth defining some actual hard limit for this. But the more important point is that given how complex the VM, and in particular sys_remap_file_pages_prot() is, i'm personally much more happy about the work having been submitted in a split-up way than i am unhappy about the bombing! Paolo has actually worked alot on this, which resulted in 40 real, new patches, so i couldnt think of any better way to present this work for review. Had he posted some link it would not be individually reviewable. (nor could the patch components be picked up by search utilities in that case - i frequently search lkml for patches, but naturally i dont traverse links referenced in them.) So i think we should rather be happy about the 40-patch progress that Paolo has made to Linux, than be unhappy about this intense work's effect on our infrastructure. In other words, we should not be worried about the number of real changes submitted to lkml, and we should only hope for that number to increase, and we should encourage people to do it! Paolo did this in 2 weeks, so it's not like he has sent changes accumulated over a long time in a patch-bomb. It was real, cutting-edge work very relevant to lkml, which work i believe Paolo didnt have much choice submitting in any other sensible and reviewable form. (i think i agree that maybe the tarball should not have been sent - but even that one was within the usual size limits and other people send tarballs frequently too.) Ingo |
From: Ingo M. <mi...@el...> - 2005-08-14 01:39:04
|
* Blaisorblade <bla...@ya...> wrote: > Ok, I've been working for the past two weeks learning well the Linux > VM, understanding the Ingo's remap_file_pages protection support and > its various weakness (due to lack of time on his part), and splitting > and finishing it. > > Here follow a series of 39 _little_ patches against the git-commit-id > 889371f61fd5bb914d0331268f12432590cf7e85, which means between > 2.6.13-rc4 and -rc5. > > Actually, the first 7 ones are unrelated trivial cleanups which > somehow get in the way on this work and that can probably be merged > even now (many are just comment fixes). > > Since I was a VM newbie until two weeks ago, I've separated my changes > into many little patches. hi. Great work! I'm wondering about this comment in rfp-fix-unmap-linear.patch: > Additionally, add a missing TLB flush in both locations. However, > there'is some excess of flushes in these functions. excess TLB flushes one of the reasons of bad UML performance, so you should really review them and not do spurious TLB flushes. Ingo |
From: Blaisorblade <bla...@ya...> - 2005-08-22 16:28:00
|
On Sunday 14 August 2005 03:38, Ingo Molnar wrote: > * Blaisorblade <bla...@ya...> wrote: > > Ok, I've been working for the past two weeks learning well the Linux > > VM, understanding the Ingo's remap_file_pages protection support and > > its various weakness (due to lack of time on his part), and splitting > > and finishing it. > > Here follow a series of 39 _little_ patches against the git-commit-id > > 889371f61fd5bb914d0331268f12432590cf7e85, which means between > > 2.6.13-rc4 and -rc5. > > Actually, the first 7 ones are unrelated trivial cleanups which > > somehow get in the way on this work and that can probably be merged > > even now (many are just comment fixes). > > Since I was a VM newbie until two weeks ago, I've separated my changes > > into many little patches. > hi. Great work! I'm wondering about this comment in Thanks for your appreciation. > rfp-fix-unmap-linear.patch: > > Additionally, add a missing TLB flush in both locations. However, > > there'is some excess of flushes in these functions. > excess TLB flushes one of the reasons of bad UML performance, so you > should really review them and not do spurious TLB flushes. After a bit of thought I realized that there was no spurious flush in those function, and that I didn't need to add any, because after setting a PTE as missing and flushing the TLB, I can freely alter it if I don't make it present again (right?), without other TLB flushes. -- Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!". Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894) http://www.user-mode-linux.org/~blaisorblade ___________________________________ Yahoo! Messenger: chiamate gratuite in tutto il mondo http://it.beta.messenger.yahoo.com |