Re: [libvirt] RFC: APIs to manage KSM

On Fri, Aug 31, 2012 at 04:56:01PM +0800, Osier Yang wrote: > Triggered by the requirement to control the new sys knob > /sys/kernel/mm/ksm/merge_nodes for a NUMA aware host by > patch: > > http://permalink.gmane.org/gmane.linux.kernel.mm/81497 > > It shows there is no methods to manage the KSM in > libvirt. This proposes the new APIs like following, thanks > for suggestions/idea in advance. > > == The macros and enums for the params' fields > > /* virNodeKSMRunMode: > * > * Represents the modes of /sys/kernel/mm/ksm/run. > */ > typedef enum { > /* Stop ksmd from running but keep merged pages */ > VIR_NODE_STOP_KSM_KEEP_MERGED_PAGES = 0, > > /* Start ksmd */ > VIR_NODE_START_KSM = 1, > > /* Stop ksmd and and unmerge all pages currently merged */ > VIR_NODE_STOP_KSM_UNMERGE_PAGES = 2, > } virNodeKSMRunMode; See my next comment about this... > > /* > * VIR_NODE_KSM_PAGES_TO_SCAN: > * > * Macro for typed parameter that represents how many present pages > * to scan before ksmd goes to sleep. > */ > # define VIR_NODE_KSM_PAGES_TO_SCAN "pages_to_scan" > > /* > * VIR_NODE_KSM_SLEEP_MILLISECS: > * > * Macro for typed parameter that represents how many milliseconds > * ksmd should sleep before next scan. > */ > # define VIR_NODE_KSM_SLEEP_MILLISECS "sleep_millisecs" > > /* > * VIR_NODE_KSM_RUN: > * > * Macro for typed parameter that is used to control ksmd's state, as > * an int containing a virNodeKSMRunMode value. > */ > # define VIR_NODE_KSM_RUN "run" The other constants here are all tunable parameters. This one however is an action. IMHO we should not use a {Get,Set}Parameters style APIs for actions. We should have an explicit API for invoking them.

> /* > * VIR_NODE_KSM_PAGES_SHARED: > * > * Macro for typed parameter that represents how many ksm shared pages > * are being used. > */ > # define VIR_NODE_KSM_PAGES_SHARED "pages_shared" > > /* > * VIR_NODE_KSM_PAGES_SHARING: > * > * Macro for typed parameter that represents how many sites are > * sharing the pages i.e. how much saved. > */ > # define VIR_NODE_KSM_PAGES_SHARING "pages_sharing" > > /* VIR_NODE_KSM_PAGES_UNSHARED: > * > * Macro for typed parameter that represents how many pages unique > * but repeatedly checked for merging. > */ > # define VIR_NODE_KSM_PAGES_UNSHARED "pages_unshared" > > /* VIR_NODE_KSM_PAGES_VOLATILE: > * > * Macro for typed parameter that represents how many pages changing > * too fast to be placed in a tree. > */ > # define VIR_NODE_KSM_PAGES_VOLATILE "pages_volatile" > > /* VIR_NODE_KSM_FULL_SCAN: > * > * Macro for typed parameter that represents how many times all > * mergeable areas have been scanned. > */ > # define VIR_NODE_KSM_FULL_SCAN "full_scan" > > == API to Get KSM parameters. > > /* > * virNodeGetKSMParameters: > * @conn: pointer to the hypervisor connection > * @params: pointer to memory parameter object > * (return value, allocated by the caller) > * @nparams: pointer to number of memory parameters; input and output > * @flags: extra flags; not used yet, so callers should always pass 0 > * > * ... Detailed comments ommitted ... > * > * Returns 0 in case of success, and -1 in case of failure. > */ > int > virNodeGetKSMParameters(virConnectPtr conn, > virTypedParameterPtr params, > int *nparams, > unsigned int flags); > > == API to set KSM parameters. > > Currently only "run", "pages_to_scan", and "sleep_millisecs" > are writable. The new boolearn sys knob "merge_nodes" could > be added once it's acceptted by kernel upstream. > > /* > * virNodeSetKSMParameters: > * @conn: pointer to the hypervisor connection > * @params: pointer to scheduler parameter objects > * @naprams: number of scheduler parameter objects > * (this value can be the same or less than the returned > * value nparams of virDomainGetSchedulerType) > * @flags: extra flags; not used yet, so callers should always pass 0 > * > * ... Detailed comments ommitted ... > * > * Returns 0 in case of success, -1 in case of failure. > */ > int > virNodeSetKSMParameters(virConnectPtr conn, > virTypedParameterPtr params, > int nparams, > unsigned int flags); > > BTW, the other thing libvirt misses is to tell ksmtuned to retune > when a domain is started or destroyed. Which is outlined in > the Red Hat Virtualization Administration Guide doc, but it's > not true. So either we have a hole in libvirt, or lacks a fix > for the document. As a general point, we should not use the term 'KSM' in the API names, since that is an implementation specific name. We want some more generic name. eg

virNodeSetMemorySharingParameters