* [PATCH 0/2] null_blk: fix wrong use of nr_online_nodes
@ 2022-05-06 1:57 Peng Liu
2022-05-06 1:58 ` [PATCH 1/2] include/linux/nodemask.h: create node_available() helper Peng Liu
2022-05-06 1:58 ` [PATCH 2/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
0 siblings, 2 replies; 4+ messages in thread
From: Peng Liu @ 2022-05-06 1:57 UTC (permalink / raw)
To: bhelgaas, tglx, mingo, bp, dave.hansen, x86, hpa,
lorenzo.pieralisi, guohanjun, sudeep.holla, rafael, lenb, akpm,
logang, martin.oliveira, thunder.leizhen, axboe, kch, ming.lei,
shinichiro.kawasaki, mcgrof, jiangguoqing, jpittman, dave,
liupeng256, wangkefeng.wang, linux-block, linux-ia64,
linux-kernel, linux-pci, linux-acpi, linux-arm-kernel, linux-mm
Helper node_available is introduced to judge whether a node can be
used, and it is used to fix the wrong use of nr_online_nodes when
numa node is sparse.
Peng Liu (2):
include/linux/nodemask.h: create node_available() helper
null_blk: fix wrong use of nr_online_nodes
arch/ia64/hp/common/sba_iommu.c | 2 +-
arch/x86/pci/acpi.c | 2 +-
drivers/acpi/arm64/iort.c | 2 +-
drivers/block/null_blk/main.c | 45 ++++++++++++++++++++-------------
drivers/pci/pci-sysfs.c | 2 +-
include/linux/nodemask.h | 3 +++
mm/mempolicy.c | 2 +-
7 files changed, 36 insertions(+), 22 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] include/linux/nodemask.h: create node_available() helper
2022-05-06 1:57 [PATCH 0/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
@ 2022-05-06 1:58 ` Peng Liu
2022-05-06 17:23 ` Bjorn Helgaas
2022-05-06 1:58 ` [PATCH 2/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
1 sibling, 1 reply; 4+ messages in thread
From: Peng Liu @ 2022-05-06 1:58 UTC (permalink / raw)
To: bhelgaas, tglx, mingo, bp, dave.hansen, x86, hpa,
lorenzo.pieralisi, guohanjun, sudeep.holla, rafael, lenb, akpm,
logang, martin.oliveira, thunder.leizhen, axboe, kch, ming.lei,
shinichiro.kawasaki, mcgrof, jiangguoqing, jpittman, dave,
liupeng256, wangkefeng.wang, linux-block, linux-ia64,
linux-kernel, linux-pci, linux-acpi, linux-arm-kernel, linux-mm
Lots of code dose
node != NUMA_NO_NODE && !node_online(node)
or
node == NUMA_NO_NODE || node_online(node)
so create node_available to do this to simplify code.
Signed-off-by: Peng Liu <liupeng256@huawei.com>
---
arch/ia64/hp/common/sba_iommu.c | 2 +-
arch/x86/pci/acpi.c | 2 +-
drivers/acpi/arm64/iort.c | 2 +-
drivers/pci/pci-sysfs.c | 2 +-
include/linux/nodemask.h | 3 +++
mm/mempolicy.c | 2 +-
6 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 8ad6946521d8..da3a010bb5fb 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1969,7 +1969,7 @@ sba_map_ioc_to_node(struct ioc *ioc, acpi_handle handle)
unsigned int node;
node = acpi_get_node(handle);
- if (node != NUMA_NO_NODE && !node_online(node))
+ if (!node_available(node))
node = NUMA_NO_NODE;
ioc->node = node;
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 052f1d78a562..d4909daa44d9 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -254,7 +254,7 @@ static int pci_acpi_root_get_node(struct acpi_pci_root *root)
dev_info(&device->dev, FW_BUG "no _PXM; falling back to node %d from hardware (may be inconsistent with ACPI node numbers)\n",
node);
}
- if (node != NUMA_NO_NODE && !node_online(node))
+ if (!node_available(node))
node = NUMA_NO_NODE;
return node;
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index f2f8f05662de..bdf690010b97 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1251,7 +1251,7 @@ static int __init arm_smmu_v3_set_proximity(struct device *dev,
if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
int dev_node = pxm_to_node(smmu->pxm);
- if (dev_node != NUMA_NO_NODE && !node_online(dev_node))
+ if (!node_available(dev_node))
return -EINVAL;
set_dev_node(dev, dev_node);
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c263ffc5884a..502490d26c1d 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -344,7 +344,7 @@ static ssize_t numa_node_store(struct device *dev,
if ((node < 0 && node != NUMA_NO_NODE) || node >= MAX_NUMNODES)
return -EINVAL;
- if (node != NUMA_NO_NODE && !node_online(node))
+ if (!node_available(node))
return -EINVAL;
add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
index 567c3ddba2c4..591201a1e646 100644
--- a/include/linux/nodemask.h
+++ b/include/linux/nodemask.h
@@ -70,6 +70,7 @@
*
* int node_online(node) Is some node online?
* int node_possible(node) Is some node possible?
+ * int node_available(node) Is some node available(online or NUMA_NO_NODE)?
*
* node_set_online(node) set bit 'node' in node_online_map
* node_set_offline(node) clear bit 'node' in node_online_map
@@ -510,6 +511,8 @@ static inline int node_random(const nodemask_t *mask)
#define num_possible_nodes() num_node_state(N_POSSIBLE)
#define node_online(node) node_state((node), N_ONLINE)
#define node_possible(node) node_state((node), N_POSSIBLE)
+#define node_available(node) \
+ ((node) == NUMA_NO_NODE || node_state((node), N_ONLINE))
#define for_each_node(node) for_each_node_state(node, N_POSSIBLE)
#define for_each_online_node(node) for_each_node_state(node, N_ONLINE)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 8c74107a2b15..7a31b006c5e8 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -141,7 +141,7 @@ int numa_map_to_online_node(int node)
{
int min_dist = INT_MAX, dist, n, min_node;
- if (node == NUMA_NO_NODE || node_online(node))
+ if (node_available(node))
return node;
min_node = node;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] null_blk: fix wrong use of nr_online_nodes
2022-05-06 1:57 [PATCH 0/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
2022-05-06 1:58 ` [PATCH 1/2] include/linux/nodemask.h: create node_available() helper Peng Liu
@ 2022-05-06 1:58 ` Peng Liu
1 sibling, 0 replies; 4+ messages in thread
From: Peng Liu @ 2022-05-06 1:58 UTC (permalink / raw)
To: bhelgaas, tglx, mingo, bp, dave.hansen, x86, hpa,
lorenzo.pieralisi, guohanjun, sudeep.holla, rafael, lenb, akpm,
logang, martin.oliveira, thunder.leizhen, axboe, kch, ming.lei,
shinichiro.kawasaki, mcgrof, jiangguoqing, jpittman, dave,
liupeng256, wangkefeng.wang, linux-block, linux-ia64,
linux-kernel, linux-pci, linux-acpi, linux-arm-kernel, linux-mm
Certain systems are designed to have sparse/discontiguous nodes,
a valid node may be greater than nr_online_nodes. So, the use of
"nid >= nr_online_nodes" to judge if a node is online is wrong.
Node id is a basic parameter of the system, a user-configured node
must be checked as early as possible. Otherwise, it may cause panic
when calling some vulnerable functions such as node_online which
will cause panic if a very big node is received.
Check g_home_node once users config it, and use node_available to
make node-checking compatible with sparse/discontiguous nodes.
Fixes: 7ff684a683d7 ("null_blk: prevent crash from bad home_node value")
Signed-off-by: Peng Liu <liupeng256@huawei.com>
Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/block/null_blk/main.c | 45 ++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 05b1120e6623..995348d6e7e7 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -97,7 +97,33 @@ module_param_named(poll_queues, g_poll_queues, int, 0444);
MODULE_PARM_DESC(poll_queues, "Number of IOPOLL submission queues");
static int g_home_node = NUMA_NO_NODE;
-module_param_named(home_node, g_home_node, int, 0444);
+
+static int null_param_store_val(const char *str, int *val, int min, int max)
+{
+ int ret, new_val;
+
+ ret = kstrtoint(str, 10, &new_val);
+ if (ret)
+ return -EINVAL;
+
+ if (new_val < min || new_val > max)
+ return -EINVAL;
+
+ *val = new_val;
+ return 0;
+}
+
+static int null_set_home_node(const char *str, const struct kernel_param *kp)
+{
+ return null_param_store_val(str, &g_home_node, 0, MAX_NUMNODES - 1);
+}
+
+static const struct kernel_param_ops null_home_node_param_ops = {
+ .set = null_set_home_node,
+ .get = param_get_int,
+};
+
+device_param_cb(home_node, &null_home_node_param_ops, &g_home_node, 0444);
MODULE_PARM_DESC(home_node, "Home node for the device");
#ifdef CONFIG_BLK_DEV_NULL_BLK_FAULT_INJECTION
@@ -120,21 +146,6 @@ MODULE_PARM_DESC(init_hctx, "Fault injection to fail hctx init. init_hctx=<inter
static int g_queue_mode = NULL_Q_MQ;
-static int null_param_store_val(const char *str, int *val, int min, int max)
-{
- int ret, new_val;
-
- ret = kstrtoint(str, 10, &new_val);
- if (ret)
- return -EINVAL;
-
- if (new_val < min || new_val > max)
- return -EINVAL;
-
- *val = new_val;
- return 0;
-}
-
static int null_set_queue_mode(const char *str, const struct kernel_param *kp)
{
return null_param_store_val(str, &g_queue_mode, NULL_Q_BIO, NULL_Q_MQ);
@@ -2107,7 +2118,7 @@ static int __init null_init(void)
g_max_sectors = BLK_DEF_MAX_SECTORS;
}
- if (g_home_node != NUMA_NO_NODE && g_home_node >= nr_online_nodes) {
+ if (!node_available(g_home_node)) {
pr_err("invalid home_node value\n");
g_home_node = NUMA_NO_NODE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] include/linux/nodemask.h: create node_available() helper
2022-05-06 1:58 ` [PATCH 1/2] include/linux/nodemask.h: create node_available() helper Peng Liu
@ 2022-05-06 17:23 ` Bjorn Helgaas
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2022-05-06 17:23 UTC (permalink / raw)
To: Peng Liu
Cc: bhelgaas, tglx, mingo, bp, dave.hansen, x86, hpa,
lorenzo.pieralisi, guohanjun, sudeep.holla, rafael, lenb, akpm,
logang, martin.oliveira, thunder.leizhen, axboe, kch, ming.lei,
shinichiro.kawasaki, mcgrof, jiangguoqing, jpittman, dave,
wangkefeng.wang, linux-block, linux-ia64, linux-kernel,
linux-pci, linux-acpi, linux-arm-kernel, linux-mm
Subject line convention looks like "numa: ..."
On Fri, May 06, 2022 at 01:58:00AM +0000, Peng Liu wrote:
> Lots of code dose
does
> node != NUMA_NO_NODE && !node_online(node)
> or
> node == NUMA_NO_NODE || node_online(node)
> so create node_available to do this to simplify code.
node_available()
I'm not really sure what meaning "node_available" conveys, though.
Probably just because I don't understand NUMA.
Should the test for NUMA_NO_NODE be folded into node_state() or
node_online() directly instead of adding a new node_available()
interface?
NUMA_NO_NODE is -1. It's not clear to me that node_state()/
node_isset()/test_bit() would do the right thing given -1. I doubt
all node_online() callers ensure they don't pass NUMA_NO_NODE.
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -70,6 +70,7 @@
> *
> * int node_online(node) Is some node online?
> * int node_possible(node) Is some node possible?
> + * int node_available(node) Is some node available(online or NUMA_NO_NODE)?
Existing file generally fits in 80 columns; follow that lead unless
you have a really good reason. E.g., maybe this?
+ * int node_available(node) Node online or NUMA_NO_NODE
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-06 17:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-06 1:57 [PATCH 0/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
2022-05-06 1:58 ` [PATCH 1/2] include/linux/nodemask.h: create node_available() helper Peng Liu
2022-05-06 17:23 ` Bjorn Helgaas
2022-05-06 1:58 ` [PATCH 2/2] null_blk: fix wrong use of nr_online_nodes Peng Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).