On Tue, Mar 29, 2022 at 07:31:12AM -0700, Dave Hansen wrote: > On 3/29/22 04:52, Jagdish Gediya wrote: > > The current implementation to identify the demotion > > targets limits some of the opportunities to share > > the demotion targets between multiple source nodes. > > This changelog is a bit unsatisfying. It basically says: the current > code isn't working, throw some more code at the problem. The current code works but it doesn't utilize all the possibilities for the demotion. One of the comment in the current code says that 'used_targets will become unavailable in future passes. This limits some opportunities for multiple source nodes to share a destination.' This patch removes those limitation. The current limitation is single used_target nodemask sharing between all the nodes, This patch prepares the used_targets from scratch for each individual node and then tries to avoid the looping based on what demotion targets are not possible for that particular node based on the data we accumulate in newly defined src_nodes array. For example, with below numa topology where node 0,1 & 2 are cpu + dram node and node 3 is slow memory node, node 0 1 2 3 0: 10 20 20 40 1: 20 10 20 40 2: 20 20 10 40 3: 40 40 40 10 once node 3 is utilized for node 0, it can not be shared for node 1 & 2 because in current code, used_targets will have that set even when we find demotion targets for 1 & 2. > I'd love to see some more information about *why* the current code > doesn't work. Is it purely a bug or was it mis-designed? I think the design seems to be intended because as per the comment in the current code, it was known that there are limits to the node sharing as a demotion target. > I actually wrote it intending for it to handle cases like you describe > while not leaving lots of nodes without demotion targets. if you see the examples I have given in the commit message, the current code misses many opportunities for the demotion, so current implementation to avoid the looping is not the best as I have explained above, that is where this patch is required.