static void unregister_memory(struct memory_block *memory)
@@ -681,6 +692,11 @@ static void unregister_memory(struct memory_block *memory)
WARN_ON(xa_erase(&memory_blocks, memory->dev.id) == NULL);
+ if (memory->group) {
+ refcount_dec(&memory->group->refcount);
+ memory->group = NULL;
Who freed the memory for the group?
+static int register_memory_group(struct memory_group group)
+{
+ struct memory_group *new_group;
+ uint32_t mgid;
+ int ret;
+
+ if (!node_possible(group.nid))
+ return -EINVAL;
+
+ new_group = kzalloc(sizeof(group), GFP_KERNEL);
+ if (!new_group)
+ return -ENOMEM;
+ *new_group = group;
You burried a memcpy here, why? Please be explicit as this is now a
dynamic structure.
+ refcount_set(&new_group->refcount, 1);
Why not just use a kref? You seem to be treating it as a kref would
work, right?
+
+ ret = xa_alloc(&memory_groups, &mgid, new_group, xa_limit_31b,
+ GFP_KERNEL);
+ if (ret)
+ kfree(new_group);
+ return ret ? ret : mgid;
I hate ?: please spell this out:
if (ret)
return ret;
return mgid;
There, more obvious and you can read it in 10 years when you have to go
fix it up...
+}
+
+int register_static_memory_group(int nid, unsigned long max_pages)
+{
+ struct memory_group group = {
+ .nid = nid,
+ .s = {
+ .max_pages = max_pages,
+ },
+ };
+
+ if (!max_pages)
+ return -EINVAL;
+ return register_memory_group(group);
+}
+EXPORT_SYMBOL_GPL(register_static_memory_group);
Let's make our global namespace a bit nicer:
memory_group_register_static()
memory_group_register_dynamic()
and so on. Use prefixes please, not suffixes.
+
+int register_dynamic_memory_group(int nid, unsigned long unit_pages)
+{
+ struct memory_group group = {
+ .nid = nid,
+ .is_dynamic = true,
+ .d = {
+ .unit_pages = unit_pages,
+ },
+ };
+
+ if (!unit_pages || !is_power_of_2(unit_pages) ||
+ unit_pages < PHYS_PFN(memory_block_size_bytes()))
+ return -EINVAL;
+ return register_memory_group(group);
+}
+EXPORT_SYMBOL_GPL(register_dynamic_memory_group);
+
+int unregister_memory_group(int mgid)
+{
+ struct memory_group *group;
+
+ if (mgid < 0)
+ return -EINVAL;
+
+ group = xa_load(&memory_groups, mgid);
+ if (!group || refcount_read(&group->refcount) > 1)
+ return -EINVAL;
+
+ xa_erase(&memory_groups, mgid);
+ kfree(group);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(unregister_memory_group);
memory_group_unregister()
+
+struct memory_group *get_memory_group(int mgid)
+{
+ return xa_load(&memory_groups, mgid);
+}
Global function?
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 97e92e8b556a..6e20a6174fe5 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -23,6 +23,42 @@
#define MIN_MEMORY_BLOCK_SIZE (1UL << SECTION_SIZE_BITS)
+struct memory_group {
+ /* Nid the whole group belongs to. */
+ int nid;
What is a "nid"?
+ /* References from memory blocks + 1. */
Blank line above this?
And put the structure comments in proper kernel doc so that others can
read them and we can verify it is correct over time.
+ refcount_t refcount;
+ /*
+ * Memory group type: static vs. dynamic.
+ *
+ * Static: All memory in the group belongs to a single unit, such as,
+ * a DIMM. All memory belonging to the group will be added in
+ * one go and removed in one go -- it's static.
+ *
+ * Dynamic: Memory within the group is added/removed dynamically in
+ * units of the specified granularity of at least one memory block.
+ */
+ bool is_dynamic;
+
+ union {
+ struct {
+ /*
+ * Maximum number of pages we'll have in this static
+ * memory group.
+ */
+ unsigned long max_pages;
+ } s;
+ struct {
+ /*
+ * Unit in pages in which memory is added/removed in
+ * this dynamic memory group. This granularity defines
+ * the alignment of a unit in physical address space.
+ */
+ unsigned long unit_pages;
+ } d;
so is_dynamic determines which to use here? Please be explicit.