Messages in this thread | | | Date | Mon, 11 Dec 2023 11:42:11 -0500 | From | Gregory Price <> | Subject | Re: [PATCH v2 00/11] mempolicy2, mbind2, and weighted interleave |
| |
On Mon, Dec 11, 2023 at 01:53:40PM +0800, Huang, Ying wrote: > Hi, Gregory, > > Thanks for updated version! > > Gregory Price <gourry.memverge@gmail.com> writes: > > > v2: > > changes / adds: > > - flattened weight matrix to an array at requested of Ying Huang > > - Updated ABI docs per Davidlohr Bueso request > > - change uapi structure to use aligned/fixed-length members as > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > > - Implemented weight fetch logic in get_mempolicy2 > > - mbind2 was changed to take (iovec,len) as function arguments > > rather than add them to the uapi structure, since they describe > > where to apply the mempolicy - as opposed to being part of it. > > > > The sysfs structure is designed as follows. > > > > $ tree /sys/kernel/mm/mempolicy/ > > /sys/kernel/mm/mempolicy/ > > ├── possible_nodes > > └── weighted_interleave > > ├── nodeN > > │ └── weight > > └── nodeN+X > > └── weight > > > > 'mempolicy' is added to '/sys/kernel/mm/' as a control group for > > the mempolicy subsystem. > > Is it good to add 'mempolicy' in '/sys/kernel/mm/numa'? The advantage > is that 'mempolicy' here is in fact "NUMA mempolicy". The disadvantage > is one more directory nesting. I have no strong opinion here. >
i don't have a strong opinion here.
> > 'possible_nodes' is added to 'mm/mempolicy' to help describe the > > expected structures under mempolicy directorys. For example, > > possible_nodes describes what nodeN directories wille exist under > > the weighted_interleave directory. > > We have '/sys/devices/system/node/possible' already. Is this just a > duplication? If so, why? And, the possible nodes can be gotten via > contents of 'weighted_interleave' too. >
I'll remove it
> And it appears not necessary to make 'weighted_interleave/nodeN' > directory. Why not just make it a file. >
Originally I wasn't sure whether there would be more attributes, but this is probably fine. I'll change it.
> And, can we add a way to reset weight to the default value? For example > `echo > nodeN/weight` or `echo > nodeN`. >
Seems reasonable.
> > ===================================================================== > > (Patches 7-10) set_mempolicy2, get_mempolicy2, mbind2 > > > > These interfaces are the 'extended' counterpart to their relatives. > > They use the userland 'struct mpol_args' structure to communicate a > > complete mempolicy configuration to the kernel. This structure > > looks very much like the kernel-internal 'struct mempolicy_args': > > > > struct mpol_args { > > /* Basic mempolicy settings */ > > __u16 mode; > > __u16 mode_flags; > > __s32 home_node; > > __aligned_u64 pol_nodes; > > __u64 pol_maxnodes; > > __u64 addr; > > __s32 policy_node; > > __s32 addr_node; > > __aligned_u64 *il_weights; /* of size pol_maxnodes */ > > }; > > This looks unnecessarily complex. I don't think that it's a good idea > to use exact same parameter for all 3 syscalls. >
It is exactly as complex as mempolicy is. Everything here is already described in the existing interfaces (except il_weights).
> For example, can we use something as below? > > long set_mempolicy2(int mode, const unsigned long *nodemask, unsigned int *il_weights, > unsigned long maxnode, unsigned long home_node, > unsigned long flags); > > long mbind2(unsigned long start, unsigned long len, > int mode, const unsigned long *nodemask, unsigned int *il_weights, > unsigned long maxnode, unsigned long home_node, > unsigned long flags); >
Your definition of mbind2 is impossible.
Neither of these interfaces solve the extensibility issue. If a new policy which requires a new format of data arrives, we can look forward to set_mempolicy3 and mbind3.
> A struct may be defined to hold mempolicy iteself. > > struct mpol { > int mode; > unsigned int home_node; > const unsigned long *nodemask; > unsigned int *il_weights; > unsigned int maxnode; > }; >
addr could be pulled out for get_mempolicy2, so i will do that
'addr_node' and 'policy_node' are warts that came from the original get_mempolicy. Removing them increases the complexity of handling arguments in the common get_mempolicy code.
I could probably just drop support for retrieving the addr_node from get_mempolicy2, since it's already possible with get_mempolicy. So I will do that.
~Gregory
| |