Messages in this thread | | | Date | Fri, 22 Aug 2008 22:15:35 +0200 | From | Rene Herman <> | Subject | Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes. |
| |
On 22-08-08 21:08, Venki Pallipadi wrote:
> On Thu, Aug 21, 2008 at 09:15:44PM -0700, Ingo Molnar wrote:
>> Venki, Suresh, Shaohua, Dave, Arjan - any observations about this >> line of action? > > The concern I have here is that the coalescing is not guaranteed to > work. We may still end up having horrible worst case latency, even > though this improves the normal case (boot the system, start X, exit > X, reboot the system). It depends on how pages are allocated and how > much memory is there in the system and what else is running etc.
Yes, I agree. Independent of the current trigger PAT wants a more scalable approach and yes, worst case is still single page entries.
That worst case is the guaranteed case now though, so I do feel it's a generic fix. After all, there wouldn't seem to be a reason to _not_ coalesce in set_memory_array_{uc,wb}().
> Here on my test system, without this coalescing change I see > > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l > 19528 > > With the coalescing change I see > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l > 135 > > quit and restart X > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l > 985
[ constantly growing number of entries ]
Yes, absolutely right, PAT definitely needs something other than the simple linked list. I do believe we also want the coalescing change though - it seems to make sense regardless of trigger and it's only little code.
> I think this as a good workaround for now. But, for long run we still need to > look at other ways of eliminating this overhead (like using page struct > that Suresh mentioned in the other thread). > > > Also, there seems to be a bug in the error path of the patch. Below should > fix it.
Ah, yes, thanks, just sent out a final version with this fixed as well.
Rene.
| |