Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

From: Rene Herman
Date: Fri Aug 22 2008 - 16:14:33 EST


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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/