linux-kernel - Re: [PATCH v2 2/4] mm/mempolicy: Support memory hotplug in weighted interleave

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9MGdwrgX8dYeHha@gourry-fedora-PF4VCD3F>
Date: Thu, 13 Mar 2025 12:23:19 -0400
From: Gregory Price <gourry@...rry.net>
To: Rakie Kim <rakie.kim@...com>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
	joshua.hahnjy@...il.com, dan.j.williams@...el.com,
	ying.huang@...ux.alibaba.com, kernel_team@...ynix.com,
	honggyu.kim@...com, yunjeong.mun@...com, david@...hat.com
Subject: Re: [PATCH v2 2/4] mm/mempolicy: Support memory hotplug in weighted
 interleave

On Thu, Mar 13, 2025 at 03:33:37PM +0900, Rakie Kim wrote:
> > I'm fairly certain this logic is wrong.  If I add two memory blocks and
> > then remove one, would this logic not remove the sysfs entries despite
> > there being a block remaining?
> 
> Regarding the assumption about node configuration:
> Are you assuming that a node has two memory blocks and that
> MEM_OFFLINE is triggered when one of them is offlined? If so, then
> you are correct that this logic would need modification.
> 
> I performed a simple test by offlining a single memory block:
> # echo 0 > /sys/devices/system/node/node2/memory100/online
> 
> In this case, MEM_OFFLINE was not triggered. However, I need to
> conduct further analysis to confirm this behavior under different
> conditions. I will review this in more detail and share my
> findings, including the test methodology and results.
>

+David - might have a quick answer to this.  I would have expected a
single memory block going offline to cause a notification.


I think the logic we care about is here:

static void node_states_check_changes_online(unsigned long nr_pages,
        struct zone *zone, struct memory_notify *arg)
{
        int nid = zone_to_nid(zone);

        arg->status_change_nid = NUMA_NO_NODE;
        arg->status_change_nid_normal = NUMA_NO_NODE;

        if (!node_state(nid, N_MEMORY))
                arg->status_change_nid = nid;
        if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY))
                arg->status_change_nid_normal = nid;
}

static void node_states_set_node(int node, struct memory_notify *arg)
{
        if (arg->status_change_nid_normal >= 0)
                node_set_state(node, N_NORMAL_MEMORY);

        if (arg->status_change_nid >= 0)
                node_set_state(node, N_MEMORY);
}

int online_pages(unsigned long pfn, unsigned long nr_pages,
                       struct zone *zone, struct memory_group *group)
{
	...
        node_states_check_changes_online(nr_pages, zone, &arg);
	...
        node_states_set_node(nid, &arg);
	...
        memory_notify(MEM_ONLINE, &arg);
}

In the callback i think you want to check whether N_MEMORY is set

+	case MEM_OFFLINE:
++              if (node is !N_MEMORY)
++			sysfs_wi_node_release(node_attrs[nid], wi_kobj);
+		break;
+	}

Similar with online (don't want to double-add).


also from what I can tell, N_MEMORY implies N_ONLINE because N_ONLINE
occurs when memory blocks are added (regardless of state).

~Gregory