From: "Uladzislau Rezki (Sony)" <[email protected]>
The kvfree_rcu() macro's single-argument form is deprecated. Therefore
switch to the new kvfree_rcu_mightsleep() variant. The goal is to
avoid accidental use of the single-argument forms, which can introduce
functionality bugs in atomic contexts and latency bugs in non-atomic
contexts.
Cc: Jens Axboe <[email protected]>
Cc: Philipp Reisner <[email protected]>
Cc: Lars Ellenberg <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
drivers/block/drbd/drbd_nl.c | 6 +++---
drivers/block/drbd/drbd_receiver.c | 4 ++--
drivers/block/drbd/drbd_state.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 60757ac31701..f49f2a5282e1 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1615,7 +1615,7 @@ int drbd_adm_disk_opts(struct sk_buff *skb, struct genl_info *info)
drbd_send_sync_param(peer_device);
}
- kvfree_rcu(old_disk_conf);
+ kvfree_rcu_mightsleep(old_disk_conf);
kfree(old_plan);
mod_timer(&device->request_timer, jiffies + HZ);
goto success;
@@ -2446,7 +2446,7 @@ int drbd_adm_net_opts(struct sk_buff *skb, struct genl_info *info)
mutex_unlock(&connection->resource->conf_update);
mutex_unlock(&connection->data.mutex);
- kvfree_rcu(old_net_conf);
+ kvfree_rcu_mightsleep(old_net_conf);
if (connection->cstate >= C_WF_REPORT_PARAMS) {
struct drbd_peer_device *peer_device;
@@ -2860,7 +2860,7 @@ int drbd_adm_resize(struct sk_buff *skb, struct genl_info *info)
new_disk_conf->disk_size = (sector_t)rs.resize_size;
rcu_assign_pointer(device->ldev->disk_conf, new_disk_conf);
mutex_unlock(&device->resource->conf_update);
- kvfree_rcu(old_disk_conf);
+ kvfree_rcu_mightsleep(old_disk_conf);
new_disk_conf = NULL;
}
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 757f4692b5bd..e197b2a465d2 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3759,7 +3759,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
drbd_info(connection, "peer data-integrity-alg: %s\n",
integrity_alg[0] ? integrity_alg : "(none)");
- kvfree_rcu(old_net_conf);
+ kvfree_rcu_mightsleep(old_net_conf);
return 0;
disconnect_rcu_unlock:
@@ -4127,7 +4127,7 @@ static int receive_sizes(struct drbd_connection *connection, struct packet_info
rcu_assign_pointer(device->ldev->disk_conf, new_disk_conf);
mutex_unlock(&connection->resource->conf_update);
- kvfree_rcu(old_disk_conf);
+ kvfree_rcu_mightsleep(old_disk_conf);
drbd_info(device, "Peer sets u_size to %lu sectors (old: %lu)\n",
(unsigned long)p_usize, (unsigned long)my_usize);
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 75d13ea0024f..2aeea295fa28 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -2071,7 +2071,7 @@ static int w_after_conn_state_ch(struct drbd_work *w, int unused)
conn_free_crypto(connection);
mutex_unlock(&connection->resource->conf_update);
- kvfree_rcu(old_conf);
+ kvfree_rcu_mightsleep(old_conf);
}
if (ns_max.susp_fen) {
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kvfree_rcu() macro's single-argument form is deprecated. Therefore
switch to the new kvfree_rcu_mightsleep() variant. The goal is to
avoid accidental use of the single-argument forms, which can introduce
functionality bugs in atomic contexts and latency bugs in non-atomic
contexts.
Cc: Bryan Tan <[email protected]>
Cc: Vishnu Dasa <[email protected]>
Reviewed-by: Vishnu Dasa <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
drivers/misc/vmw_vmci/vmci_context.c | 2 +-
drivers/misc/vmw_vmci/vmci_event.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/vmw_vmci/vmci_context.c b/drivers/misc/vmw_vmci/vmci_context.c
index 172696abce31..f22b44827e92 100644
--- a/drivers/misc/vmw_vmci/vmci_context.c
+++ b/drivers/misc/vmw_vmci/vmci_context.c
@@ -687,7 +687,7 @@ int vmci_ctx_remove_notification(u32 context_id, u32 remote_cid)
spin_unlock(&context->lock);
if (notifier)
- kvfree_rcu(notifier);
+ kvfree_rcu_mightsleep(notifier);
vmci_ctx_put(context);
diff --git a/drivers/misc/vmw_vmci/vmci_event.c b/drivers/misc/vmw_vmci/vmci_event.c
index 2100297c94ad..5d7ac07623c2 100644
--- a/drivers/misc/vmw_vmci/vmci_event.c
+++ b/drivers/misc/vmw_vmci/vmci_event.c
@@ -209,7 +209,7 @@ int vmci_event_unsubscribe(u32 sub_id)
if (!s)
return VMCI_ERROR_NOT_FOUND;
- kvfree_rcu(s);
+ kvfree_rcu_mightsleep(s);
return VMCI_SUCCESS;
}
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kvfree_rcu() macro's single-argument form is deprecated. Therefore
switch to the new kvfree_rcu_mightsleep() variant. The goal is to
avoid accidental use of the single-argument forms, which can introduce
functionality bugs in atomic contexts and latency bugs in non-atomic
contexts.
Cc: Steven Rostedt (VMware) <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/trace/trace_osnoise.c | 2 +-
kernel/trace/trace_probe.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index 04f0fdae19a1..f68ca1e6460f 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -159,7 +159,7 @@ static void osnoise_unregister_instance(struct trace_array *tr)
if (!found)
return;
- kvfree_rcu(inst);
+ kvfree_rcu_mightsleep(inst);
}
/*
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 20d0c4a97633..2d2616678295 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1172,7 +1172,7 @@ int trace_probe_remove_file(struct trace_probe *tp,
return -ENOENT;
list_del_rcu(&link->list);
- kvfree_rcu(link);
+ kvfree_rcu_mightsleep(link);
if (list_empty(&tp->event->files))
trace_probe_clear_flag(tp, TP_FLAG_TRACE);
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kvfree_rcu() macro's single-argument form is deprecated. Therefore
switch to the new kvfree_rcu_mightsleep() variant. The goal is to
avoid accidental use of the single-argument forms, which can introduce
functionality bugs in atomic contexts and latency bugs in non-atomic
contexts.
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
lib/test_vmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
index de4ee0d50906..cd2bdba6d3ed 100644
--- a/lib/test_vmalloc.c
+++ b/lib/test_vmalloc.c
@@ -334,7 +334,7 @@ kvfree_rcu_1_arg_vmalloc_test(void)
return -1;
p->array[0] = 'a';
- kvfree_rcu(p);
+ kvfree_rcu_mightsleep(p);
}
return 0;
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
of the single-argument forms, which can introduce functionality bugs in
atomic contexts and latency bugs in non-atomic contexts.
Cc: Eric Dumazet <[email protected]>
Cc: David S. Miller <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
net/core/sysctl_net_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 74842b453407..782273bb93c2 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -177,7 +177,7 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
if (orig_sock_table) {
static_branch_dec(&rps_needed);
static_branch_dec(&rfs_needed);
- kvfree_rcu(orig_sock_table);
+ kvfree_rcu_mightsleep(orig_sock_table);
}
}
}
@@ -215,7 +215,7 @@ static int flow_limit_cpu_sysctl(struct ctl_table *table, int write,
lockdep_is_held(&flow_limit_update_mutex));
if (cur && !cpumask_test_cpu(i, mask)) {
RCU_INIT_POINTER(sd->flow_limit, NULL);
- kfree_rcu(cur);
+ kfree_rcu_mightsleep(cur);
} else if (!cur && cpumask_test_cpu(i, mask)) {
cur = kzalloc_node(len, GFP_KERNEL,
cpu_to_node(i));
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
of the single-argument forms, which can introduce functionality bugs in
atomic contexts and latency bugs in non-atomic contexts.
Cc: Ariel Levkovich <[email protected]>
Cc: Saeed Mahameed <[email protected]>
Cc: Vlad Buslov <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
index ca834bbcb44f..8afcec0c5d3c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
@@ -242,7 +242,7 @@ mlx5e_int_port_remove(struct mlx5e_tc_int_port_priv *priv,
mlx5_del_flow_rules(int_port->rx_rule);
mapping_remove(ctx, int_port->mapping);
mlx5e_int_port_metadata_free(priv, int_port->match_metadata);
- kfree_rcu(int_port);
+ kfree_rcu_mightsleep(int_port);
priv->num_ports--;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
index 08d0929e8260..b811dad7370a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
@@ -670,7 +670,7 @@ static int mlx5e_macsec_del_txsa(struct macsec_context *ctx)
mlx5e_macsec_cleanup_sa(macsec, tx_sa, true);
mlx5_destroy_encryption_key(macsec->mdev, tx_sa->enc_key_id);
- kfree_rcu(tx_sa);
+ kfree_rcu_mightsleep(tx_sa);
macsec_device->tx_sa[assoc_num] = NULL;
out:
@@ -849,7 +849,7 @@ static void macsec_del_rxsc_ctx(struct mlx5e_macsec *macsec, struct mlx5e_macsec
xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
metadata_dst_free(rx_sc->md_dst);
kfree(rx_sc->sc_xarray_element);
- kfree_rcu(rx_sc);
+ kfree_rcu_mightsleep(rx_sc);
}
static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
of the single-argument forms, which can introduce functionality bugs in
atomic contexts and latency bugs in non-atomic contexts.
Cc: Theodore Ts'o <[email protected]>
Cc: Lukas Czerner <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
fs/ext4/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 88f7b8a88c76..405a66b47311 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2500,7 +2500,7 @@ static void ext4_apply_quota_options(struct fs_context *fc,
qname = rcu_replace_pointer(sbi->s_qf_names[i], qname,
lockdep_is_held(&sb->s_umount));
if (qname)
- kfree_rcu(qname);
+ kfree_rcu_mightsleep(qname);
}
}
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
of the single-argument forms, which can introduce functionality bugs in
atomic contexts and latency bugs in non-atomic contexts.
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/rcu/rcuscale.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 91fb5905a008..98b75995b680 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -716,7 +716,7 @@ kfree_scale_thread(void *arg)
// is tested.
if ((kfree_rcu_test_single && !kfree_rcu_test_double) ||
(kfree_rcu_test_both && torture_random(&tr) & 0x800))
- kfree_rcu(alloc_ptr);
+ kfree_rcu_mightsleep(alloc_ptr);
else
kfree_rcu(alloc_ptr, rh);
}
--
2.40.0.rc1.284.g88254d51c5-goog
From: Zqiang <[email protected]>
Given a non-zero rcutorture.nocbs_nthreads module parameter, the specified
number of nocb kthreads will be created, regardless of whether or not
the RCU implementation under test is capable of offloading callbacks.
Please note that even vanilla RCU is incapable of offloading in kernels
built with CONFIG_RCU_NOCB_CPU=n. And when the RCU implementation is
incapable of offloading callbacks, there is no point in creating those
kthreads.
This commit therefore checks the cur_ops.torture_type module parameter and
CONFIG_RCU_NOCB_CPU Kconfig option in order to avoid creating unnecessary
nocb tasks.
Signed-off-by: Zqiang <[email protected]>
Reviewed-by: Joel Fernandes (Google) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/rcu/rcutorture.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 8e6c023212cb..83870c4ae1b8 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -3501,6 +3501,12 @@ rcu_torture_init(void)
pr_alert("rcu-torture: ->fqs NULL and non-zero fqs_duration, fqs disabled.\n");
fqs_duration = 0;
}
+ if (nocbs_nthreads != 0 && (cur_ops != &rcu_ops ||
+ !IS_ENABLED(CONFIG_RCU_NOCB_CPU))) {
+ pr_alert("rcu-torture types: %s and CONFIG_RCU_NOCB_CPU=%d, nocb toggle disabled.\n",
+ cur_ops->name, IS_ENABLED(CONFIG_RCU_NOCB_CPU));
+ nocbs_nthreads = 0;
+ }
if (cur_ops->init)
cur_ops->init();
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Paul E. McKenney" <[email protected]>
This commit tests the "tsc=watchdog" kernel boot parameter when running
the clocksourcewd torture tests.
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
tools/testing/selftests/rcutorture/bin/torture.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/rcutorture/bin/torture.sh b/tools/testing/selftests/rcutorture/bin/torture.sh
index 130d0de4c3bb..5a2ae2264403 100755
--- a/tools/testing/selftests/rcutorture/bin/torture.sh
+++ b/tools/testing/selftests/rcutorture/bin/torture.sh
@@ -497,16 +497,16 @@ fi
if test "$do_clocksourcewd" = "yes"
then
- torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000"
+ torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000 tsc=watchdog"
torture_set "clocksourcewd-1" tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --kconfig "CONFIG_TEST_CLOCKSOURCE_WATCHDOG=y" --trust-make
- torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000 clocksource.max_cswd_read_retries=1"
+ torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000 clocksource.max_cswd_read_retries=1 tsc=watchdog"
torture_set "clocksourcewd-2" tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --kconfig "CONFIG_TEST_CLOCKSOURCE_WATCHDOG=y" --trust-make
# In case our work is already done...
if test "$do_rcutorture" != "yes"
then
- torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000"
+ torture_bootargs="rcupdate.rcu_cpu_stall_suppress_at_boot=1 torture.disable_onoff_at_boot rcupdate.rcu_task_stall_timeout=30000 tsc=watchdog"
torture_set "clocksourcewd-3" tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --trust-make
fi
fi
--
2.40.0.rc1.284.g88254d51c5-goog
The k[v]free_rcu() macro's single-argument form is deprecated.
Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
is to avoid accidental use of the single-argument forms, which can
introduce functionality bugs in atomic contexts and latency bugs in
non-atomic contexts.
The callers are holding a mutex so the context allows blocking. Hence
using the API with a single argument will be fine, but use its new name.
There is no functionality change with this patch.
Fixes: 57588c71177f ("mac802154: Handle passive scanning")
Reviewed-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
net/mac802154/scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index 9b0933a185eb..5c191bedd72c 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -52,7 +52,7 @@ static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
request = rcu_replace_pointer(local->scan_req, NULL, 1);
if (!request)
return 0;
- kfree_rcu(request);
+ kvfree_rcu_mightsleep(request);
/* Advertize first, while we know the devices cannot be removed */
if (aborted)
@@ -403,7 +403,7 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local,
request = rcu_replace_pointer(local->beacon_req, NULL, 1);
if (!request)
return 0;
- kfree_rcu(request);
+ kvfree_rcu_mightsleep(request);
nl802154_beaconing_done(wpan_dev);
--
2.40.0.rc1.284.g88254d51c5-goog
From: "Uladzislau Rezki (Sony)" <[email protected]>
The kvfree_rcu() and kfree_rcu() APIs are hazardous in that if you forget
the second argument, it works, but might sleep. This sleeping can be a
correctness bug from atomic contexts, and even in non-atomic contexts it
might introduce unacceptable latencies. This commit therefore removes the
single-argument kvfree_rcu() and kfree_rcu() macros. Code that would have
previously used these single-argument kvfree_rcu() and kfree_rcu() macros
should instead use kvfree_rcu_mightsleep() or kfree_rcu_mightsleep().
[ paulmck: Apply Joel Fernandes feedback. ]
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
include/linux/rcupdate.h | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 094321c17e48..7571dbfecb18 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -957,9 +957,8 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
/**
* kfree_rcu() - kfree an object after a grace period.
- * @ptr: pointer to kfree for both single- and double-argument invocations.
- * @rhf: the name of the struct rcu_head within the type of @ptr,
- * but only for double-argument invocations.
+ * @ptr: pointer to kfree for double-argument invocations.
+ * @rhf: the name of the struct rcu_head within the type of @ptr.
*
* Many rcu callbacks functions just call kfree() on the base structure.
* These functions are trivial, but their size adds up, and furthermore
@@ -982,26 +981,18 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
* The BUILD_BUG_ON check must not involve any function calls, hence the
* checks are done in macros here.
*/
-#define kfree_rcu(ptr, rhf...) kvfree_rcu(ptr, ## rhf)
+#define kfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
+#define kvfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
/**
- * kvfree_rcu() - kvfree an object after a grace period.
- *
- * This macro consists of one or two arguments and it is
- * based on whether an object is head-less or not. If it
- * has a head then a semantic stays the same as it used
- * to be before:
- *
- * kvfree_rcu(ptr, rhf);
- *
- * where @ptr is a pointer to kvfree(), @rhf is the name
- * of the rcu_head structure within the type of @ptr.
+ * kfree_rcu_mightsleep() - kfree an object after a grace period.
+ * @ptr: pointer to kfree for single-argument invocations.
*
* When it comes to head-less variant, only one argument
* is passed and that is just a pointer which has to be
* freed after a grace period. Therefore the semantic is
*
- * kvfree_rcu(ptr);
+ * kfree_rcu_mightsleep(ptr);
*
* where @ptr is the pointer to be freed by kvfree().
*
@@ -1010,13 +1001,9 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
* annotation. Otherwise, please switch and embed the
* rcu_head structure within the type of @ptr.
*/
-#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__, \
- kvfree_rcu_arg_2, kvfree_rcu_arg_1)(__VA_ARGS__)
-
+#define kfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
#define kvfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
-#define kfree_rcu_mightsleep(ptr) kvfree_rcu_mightsleep(ptr)
-#define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
#define kvfree_rcu_arg_2(ptr, rhf) \
do { \
typeof (ptr) ___p = (ptr); \
--
2.40.0.rc1.284.g88254d51c5-goog
Single-argument kvfree_rcu() usage is being deprecated [1] [2]. However,
till all users are converted, we would like to introduce checkpatch
errors for new patches submitted.
This patch adds support for the same. Tested with a trial patch.
For now, we are only considering usages that don't have compound
nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
This is sufficient as such usages are unlikely.
Once all users are converted and we remove the old API, we can also revert this
checkpatch patch then.
[1] https://lore.kernel.org/rcu/CAEXW_YRhHaVuq+5f+VgCZM=SF+9xO+QXaxe0yE7oA9iCXK-XPg@mail.gmail.com/
[2] https://lore.kernel.org/rcu/CAEXW_YSY=q2_uaE2qo4XSGjzs4+C102YMVJ7kWwuT5LGmJGGew@mail.gmail.com/
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
scripts/checkpatch.pl | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..9da0a3cb1615 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6388,6 +6388,15 @@ sub process {
}
}
+# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
+ if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
+ if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
+ ERROR("DEPRECATED_API",
+ "Single-argument k[v]free_rcu() API is deprecated, please call the API with an rcu_head object passed, like: k[v]free_rcu(object_ptr, rcu_head_name); " . $herecurr);
+ }
+ }
+
+
# check for unnecessary "Out of Memory" messages
if ($line =~ /^\+.*\b$logFunctions\s*\(/ &&
$prevline =~ /^[ \+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/ &&
--
2.40.0.rc1.284.g88254d51c5-goog
The k[v]free_rcu() macro's single-argument form is deprecated.
Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
is to avoid accidental use of the single-argument forms, which can
introduce functionality bugs in atomic contexts and latency bugs in
non-atomic contexts.
There is no functionality change with this patch.
Link: https://lore.kernel.org/rcu/[email protected]
Acked-by: Zhu Yanjun <[email protected]>
Reviewed-by: Bob Pearson <[email protected]>
Reviewed-by: Paul E. McKenney <[email protected]>
Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()")
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index b10aa1580a64..ae3a100e18fb 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
return -EINVAL;
rxe_cleanup(mr);
- kfree_rcu(mr);
+ kfree_rcu_mightsleep(mr);
return 0;
}
--
2.40.0.rc1.284.g88254d51c5-goog
On 3/15/23 13:19, Joel Fernandes (Google) wrote:
> The k[v]free_rcu() macro's single-argument form is deprecated.
> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> is to avoid accidental use of the single-argument forms, which can
> introduce functionality bugs in atomic contexts and latency bugs in
> non-atomic contexts.
>
> There is no functionality change with this patch.
>
> Link: https://lore.kernel.org/rcu/[email protected]
> Acked-by: Zhu Yanjun <[email protected]>
> Reviewed-by: Bob Pearson <[email protected]>
> Reviewed-by: Paul E. McKenney <[email protected]>
> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()")
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index b10aa1580a64..ae3a100e18fb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> return -EINVAL;
>
> rxe_cleanup(mr);
> - kfree_rcu(mr);
> + kfree_rcu_mightsleep(mr);
> return 0;
> }
>
Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr).
Bob
On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <[email protected]> wrote:
>
> On 3/15/23 13:19, Joel Fernandes (Google) wrote:
> > The k[v]free_rcu() macro's single-argument form is deprecated.
> > Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> > is to avoid accidental use of the single-argument forms, which can
> > introduce functionality bugs in atomic contexts and latency bugs in
> > non-atomic contexts.
> >
> > There is no functionality change with this patch.
> >
> > Link: https://lore.kernel.org/rcu/[email protected]
> > Acked-by: Zhu Yanjun <[email protected]>
> > Reviewed-by: Bob Pearson <[email protected]>
> > Reviewed-by: Paul E. McKenney <[email protected]>
> > Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()")
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
> > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> > index b10aa1580a64..ae3a100e18fb 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> > @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> > return -EINVAL;
> >
> > rxe_cleanup(mr);
> > - kfree_rcu(mr);
> > + kfree_rcu_mightsleep(mr);
> > return 0;
> > }
> >
> Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr).
I thought you said either was Ok, but yeah that's fine with me. I was
trying to play it safe ;-). An extra GP may not hurt, but not having
one when it is needed might. I will update it to just be kfree.
<quote>
> rxe_cleanup(mr);
> - kfree_rcu(mr);
> + kfree_rcu_mightsleep(mr);
> return 0;
> }
>
I would prefer just
- kfree_rcu(mr);
+ kfree(mr);
but either one will work.
Bob
</quote>
- Joel
On 3/15/23 13:41, Joel Fernandes wrote:
> On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <[email protected]> wrote:
>>
>> On 3/15/23 13:19, Joel Fernandes (Google) wrote:
>>> The k[v]free_rcu() macro's single-argument form is deprecated.
>>> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
>>> is to avoid accidental use of the single-argument forms, which can
>>> introduce functionality bugs in atomic contexts and latency bugs in
>>> non-atomic contexts.
>>>
>>> There is no functionality change with this patch.
>>>
>>> Link: https://lore.kernel.org/rcu/[email protected]
>>> Acked-by: Zhu Yanjun <[email protected]>
>>> Reviewed-by: Bob Pearson <[email protected]>
>>> Reviewed-by: Paul E. McKenney <[email protected]>
>>> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()")
>>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>>> ---
>>> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
>>> index b10aa1580a64..ae3a100e18fb 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
>>> @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
>>> return -EINVAL;
>>>
>>> rxe_cleanup(mr);
>>> - kfree_rcu(mr);
>>> + kfree_rcu_mightsleep(mr);
>>> return 0;
>>> }
>>>
>> Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr).
>
> I thought you said either was Ok, but yeah that's fine with me. I was
> trying to play it safe ;-). An extra GP may not hurt, but not having
> one when it is needed might. I will update it to just be kfree.
Thanks. The reason to not add the pause is that it really isn't required and will just make
people think that it is. With the current state of the driver the mr cleanup code will disable
looking up the mr from its rkey or lkey and then wait until all the references to the mr are dropped
before calling kfree. This will always work (unless a new bug is introduced) so no reason to
add the rcu delay.
Bob
>
> <quote>
>> rxe_cleanup(mr);
>> - kfree_rcu(mr);
>> + kfree_rcu_mightsleep(mr);
>> return 0;
>> }
>>
>
> I would prefer just
> - kfree_rcu(mr);
> + kfree(mr);
>
> but either one will work.
> Bob
> </quote>
>
> - Joel
On Wed, Mar 15, 2023 at 06:18:54PM +0000, Joel Fernandes (Google) wrote:
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
> deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
> kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
> of the single-argument forms, which can introduce functionality bugs in
> atomic contexts and latency bugs in non-atomic contexts.
>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Lukas Czerner <[email protected]>
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
Acked-by: Theodore Ts'o <[email protected]>
On Wed, Mar 15, 2023 at 3:05 PM Bob Pearson <[email protected]> wrote:
>
> On 3/15/23 13:41, Joel Fernandes wrote:
> > On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <[email protected]> wrote:
> >>
> >> On 3/15/23 13:19, Joel Fernandes (Google) wrote:
> >>> The k[v]free_rcu() macro's single-argument form is deprecated.
> >>> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> >>> is to avoid accidental use of the single-argument forms, which can
> >>> introduce functionality bugs in atomic contexts and latency bugs in
> >>> non-atomic contexts.
> >>>
> >>> There is no functionality change with this patch.
> >>>
> >>> Link: https://lore.kernel.org/rcu/[email protected]
> >>> Acked-by: Zhu Yanjun <[email protected]>
> >>> Reviewed-by: Bob Pearson <[email protected]>
> >>> Reviewed-by: Paul E. McKenney <[email protected]>
> >>> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()")
> >>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> >>> ---
> >>> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> >>> index b10aa1580a64..ae3a100e18fb 100644
> >>> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> >>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> >>> @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> >>> return -EINVAL;
> >>>
> >>> rxe_cleanup(mr);
> >>> - kfree_rcu(mr);
> >>> + kfree_rcu_mightsleep(mr);
> >>> return 0;
> >>> }
> >>>
> >> Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr).
> >
> > I thought you said either was Ok, but yeah that's fine with me. I was
> > trying to play it safe ;-). An extra GP may not hurt, but not having
> > one when it is needed might. I will update it to just be kfree.
>
> Thanks. The reason to not add the pause is that it really isn't required and will just make
> people think that it is. With the current state of the driver the mr cleanup code will disable
> looking up the mr from its rkey or lkey and then wait until all the references to the mr are dropped
> before calling kfree. This will always work (unless a new bug is introduced) so no reason to
> add the rcu delay.
>
Sounds good! I updated it in my queue, and I will repost it in the
future pending test results and other comments.
thanks,
- Joel
On Wed, Mar 15, 2023 at 06:19:01PM +0000, Joel Fernandes (Google) wrote:
> Single-argument kvfree_rcu() usage is being deprecated [1] [2]. However,
> till all users are converted, we would like to introduce checkpatch
> errors for new patches submitted.
>
> This patch adds support for the same. Tested with a trial patch.
>
> For now, we are only considering usages that don't have compound
> nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> This is sufficient as such usages are unlikely.
>
> Once all users are converted and we remove the old API, we can also revert this
> checkpatch patch then.
>
> [1] https://lore.kernel.org/rcu/CAEXW_YRhHaVuq+5f+VgCZM=SF+9xO+QXaxe0yE7oA9iCXK-XPg@mail.gmail.com/
> [2] https://lore.kernel.org/rcu/CAEXW_YSY=q2_uaE2qo4XSGjzs4+C102YMVJ7kWwuT5LGmJGGew@mail.gmail.com/
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
With or without the change suggested below:
Acked-by: Paul E. McKenney <[email protected]>
> ---
> scripts/checkpatch.pl | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index bd44d12965c9..9da0a3cb1615 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6388,6 +6388,15 @@ sub process {
> }
> }
>
> +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> + if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> + if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> + ERROR("DEPRECATED_API",
> + "Single-argument k[v]free_rcu() API is deprecated, please call the API with an rcu_head object passed, like: k[v]free_rcu(object_ptr, rcu_head_name); " . $herecurr);
Perhaps also point them at kvfree_rcu_mightsleep()?
Thanx, Paul
> + }
> + }
> +
> +
> # check for unnecessary "Out of Memory" messages
> if ($line =~ /^\+.*\b$logFunctions\s*\(/ &&
> $prevline =~ /^[ \+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/ &&
> --
> 2.40.0.rc1.284.g88254d51c5-goog
>
On Wed, Mar 15, 2023 at 06:18:49PM +0000, Joel Fernandes (Google) wrote:
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kvfree_rcu() macro's single-argument form is deprecated. Therefore
> switch to the new kvfree_rcu_mightsleep() variant. The goal is to
> avoid accidental use of the single-argument forms, which can introduce
> functionality bugs in atomic contexts and latency bugs in non-atomic
> contexts.
>
> Cc: Bryan Tan <[email protected]>
> Cc: Vishnu Dasa <[email protected]>
> Reviewed-by: Vishnu Dasa <[email protected]>
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> drivers/misc/vmw_vmci/vmci_context.c | 2 +-
> drivers/misc/vmw_vmci/vmci_event.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Acked-by: Greg Kroah-Hartman <[email protected]>
On 3/15/23 19:18, Joel Fernandes (Google) wrote:
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kvfree_rcu() macro's single-argument form is deprecated. Therefore
> switch to the new kvfree_rcu_mightsleep() variant. The goal is to
> avoid accidental use of the single-argument forms, which can introduce
> functionality bugs in atomic contexts and latency bugs in non-atomic
> contexts.
>
> Cc: Steven Rostedt (VMware) <[email protected]>
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> kernel/trace/trace_osnoise.c | 2 +-
> kernel/trace/trace_probe.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index 04f0fdae19a1..f68ca1e6460f 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -159,7 +159,7 @@ static void osnoise_unregister_instance(struct trace_array *tr)
> if (!found)
> return;
>
> - kvfree_rcu(inst);
> + kvfree_rcu_mightsleep(inst);
> }
>
> /*
Acked-by: Daniel Bristot de Oliveira <[email protected]>
Thanks!
-- Daniel
On Thu, Mar 16, 2023 at 3:38 AM Daniel Bristot de Oliveira
<[email protected]> wrote:
>
> On 3/15/23 19:18, Joel Fernandes (Google) wrote:
> > From: "Uladzislau Rezki (Sony)" <[email protected]>
> >
> > The kvfree_rcu() macro's single-argument form is deprecated. Therefore
> > switch to the new kvfree_rcu_mightsleep() variant. The goal is to
> > avoid accidental use of the single-argument forms, which can introduce
> > functionality bugs in atomic contexts and latency bugs in non-atomic
> > contexts.
> >
> > Cc: Steven Rostedt (VMware) <[email protected]>
> > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
> > kernel/trace/trace_osnoise.c | 2 +-
> > kernel/trace/trace_probe.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> > index 04f0fdae19a1..f68ca1e6460f 100644
> > --- a/kernel/trace/trace_osnoise.c
> > +++ b/kernel/trace/trace_osnoise.c
> > @@ -159,7 +159,7 @@ static void osnoise_unregister_instance(struct trace_array *tr)
> > if (!found)
> > return;
> >
> > - kvfree_rcu(inst);
> > + kvfree_rcu_mightsleep(inst);
> > }
> >
> > /*
>
> Acked-by: Daniel Bristot de Oliveira <[email protected]>
Thanks!
- Joel
On Thu, Mar 16, 2023 at 2:50 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Wed, Mar 15, 2023 at 06:18:49PM +0000, Joel Fernandes (Google) wrote:
> > From: "Uladzislau Rezki (Sony)" <[email protected]>
> >
> > The kvfree_rcu() macro's single-argument form is deprecated. Therefore
> > switch to the new kvfree_rcu_mightsleep() variant. The goal is to
> > avoid accidental use of the single-argument forms, which can introduce
> > functionality bugs in atomic contexts and latency bugs in non-atomic
> > contexts.
> >
> > Cc: Bryan Tan <[email protected]>
> > Cc: Vishnu Dasa <[email protected]>
> > Reviewed-by: Vishnu Dasa <[email protected]>
> > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
> > drivers/misc/vmw_vmci/vmci_context.c | 2 +-
> > drivers/misc/vmw_vmci/vmci_event.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
Thanks!
- Joel
On Wed, Mar 15, 2023 at 3:07 PM Theodore Ts'o <[email protected]> wrote:
>
> On Wed, Mar 15, 2023 at 06:18:54PM +0000, Joel Fernandes (Google) wrote:
> > From: "Uladzislau Rezki (Sony)" <[email protected]>
> >
> > The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
> > deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
> > kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
> > of the single-argument forms, which can introduce functionality bugs in
> > atomic contexts and latency bugs in non-atomic contexts.
> >
> > Cc: Theodore Ts'o <[email protected]>
> > Cc: Lukas Czerner <[email protected]>
> > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
>
> Acked-by: Theodore Ts'o <[email protected]>
>
Thanks!
- Joel
Hello.
On 15.03.23 19:18, Joel Fernandes (Google) wrote:
> The k[v]free_rcu() macro's single-argument form is deprecated.
> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> is to avoid accidental use of the single-argument forms, which can
> introduce functionality bugs in atomic contexts and latency bugs in
> non-atomic contexts.
>
> The callers are holding a mutex so the context allows blocking. Hence
> using the API with a single argument will be fine, but use its new name.
>
> There is no functionality change with this patch.
>
> Fixes: 57588c71177f ("mac802154: Handle passive scanning")
> Reviewed-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> net/mac802154/scan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
> index 9b0933a185eb..5c191bedd72c 100644
> --- a/net/mac802154/scan.c
> +++ b/net/mac802154/scan.c
> @@ -52,7 +52,7 @@ static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
> request = rcu_replace_pointer(local->scan_req, NULL, 1);
> if (!request)
> return 0;
> - kfree_rcu(request);
> + kvfree_rcu_mightsleep(request);
>
> /* Advertize first, while we know the devices cannot be removed */
> if (aborted)
> @@ -403,7 +403,7 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local,
> request = rcu_replace_pointer(local->beacon_req, NULL, 1);
> if (!request)
> return 0;
> - kfree_rcu(request);
> + kvfree_rcu_mightsleep(request);
>
> nl802154_beaconing_done(wpan_dev);
>
I just saw that there is a v2 of this patch. My ACK still stands as for v1.
Acked-by: Stefan Schmidt <[email protected]>
regards
Stefan Schmidt
On Thu, Mar 16, 2023 at 12:41 PM Stefan Schmidt
<[email protected]> wrote:
>
> Hello.
>
> On 15.03.23 19:18, Joel Fernandes (Google) wrote:
> > The k[v]free_rcu() macro's single-argument form is deprecated.
> > Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal
> > is to avoid accidental use of the single-argument forms, which can
> > introduce functionality bugs in atomic contexts and latency bugs in
> > non-atomic contexts.
> >
> > The callers are holding a mutex so the context allows blocking. Hence
> > using the API with a single argument will be fine, but use its new name.
> >
> > There is no functionality change with this patch.
> >
> > Fixes: 57588c71177f ("mac802154: Handle passive scanning")
> > Reviewed-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
> > net/mac802154/scan.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
> > index 9b0933a185eb..5c191bedd72c 100644
> > --- a/net/mac802154/scan.c
> > +++ b/net/mac802154/scan.c
> > @@ -52,7 +52,7 @@ static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
> > request = rcu_replace_pointer(local->scan_req, NULL, 1);
> > if (!request)
> > return 0;
> > - kfree_rcu(request);
> > + kvfree_rcu_mightsleep(request);
> >
> > /* Advertize first, while we know the devices cannot be removed */
> > if (aborted)
> > @@ -403,7 +403,7 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local,
> > request = rcu_replace_pointer(local->beacon_req, NULL, 1);
> > if (!request)
> > return 0;
> > - kfree_rcu(request);
> > + kvfree_rcu_mightsleep(request);
> >
> > nl802154_beaconing_done(wpan_dev);
> >
>
> I just saw that there is a v2 of this patch. My ACK still stands as for v1.
>
>
> Acked-by: Stefan Schmidt <[email protected]>
Thanks! Applied the ack and will be taking it via the RCU tree as we discussed.
- Joel
On Wed, Mar 15, 2023 at 2:19 PM Joel Fernandes (Google)
<[email protected]> wrote:
>
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kvfree_rcu() macro's single-argument form is deprecated. Therefore
> switch to the new kvfree_rcu_mightsleep() variant. The goal is to
> avoid accidental use of the single-argument forms, which can introduce
> functionality bugs in atomic contexts and latency bugs in non-atomic
> contexts.
>
> Cc: Jens Axboe <[email protected]>
> Cc: Philipp Reisner <[email protected]>
> Cc: Lars Ellenberg <[email protected]>
Jens/Others, any chance for an Ack here?
- Joel
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> drivers/block/drbd/drbd_nl.c | 6 +++---
> drivers/block/drbd/drbd_receiver.c | 4 ++--
> drivers/block/drbd/drbd_state.c | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
> index 60757ac31701..f49f2a5282e1 100644
> --- a/drivers/block/drbd/drbd_nl.c
> +++ b/drivers/block/drbd/drbd_nl.c
> @@ -1615,7 +1615,7 @@ int drbd_adm_disk_opts(struct sk_buff *skb, struct genl_info *info)
> drbd_send_sync_param(peer_device);
> }
>
> - kvfree_rcu(old_disk_conf);
> + kvfree_rcu_mightsleep(old_disk_conf);
> kfree(old_plan);
> mod_timer(&device->request_timer, jiffies + HZ);
> goto success;
> @@ -2446,7 +2446,7 @@ int drbd_adm_net_opts(struct sk_buff *skb, struct genl_info *info)
>
> mutex_unlock(&connection->resource->conf_update);
> mutex_unlock(&connection->data.mutex);
> - kvfree_rcu(old_net_conf);
> + kvfree_rcu_mightsleep(old_net_conf);
>
> if (connection->cstate >= C_WF_REPORT_PARAMS) {
> struct drbd_peer_device *peer_device;
> @@ -2860,7 +2860,7 @@ int drbd_adm_resize(struct sk_buff *skb, struct genl_info *info)
> new_disk_conf->disk_size = (sector_t)rs.resize_size;
> rcu_assign_pointer(device->ldev->disk_conf, new_disk_conf);
> mutex_unlock(&device->resource->conf_update);
> - kvfree_rcu(old_disk_conf);
> + kvfree_rcu_mightsleep(old_disk_conf);
> new_disk_conf = NULL;
> }
>
> diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
> index 757f4692b5bd..e197b2a465d2 100644
> --- a/drivers/block/drbd/drbd_receiver.c
> +++ b/drivers/block/drbd/drbd_receiver.c
> @@ -3759,7 +3759,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
> drbd_info(connection, "peer data-integrity-alg: %s\n",
> integrity_alg[0] ? integrity_alg : "(none)");
>
> - kvfree_rcu(old_net_conf);
> + kvfree_rcu_mightsleep(old_net_conf);
> return 0;
>
> disconnect_rcu_unlock:
> @@ -4127,7 +4127,7 @@ static int receive_sizes(struct drbd_connection *connection, struct packet_info
>
> rcu_assign_pointer(device->ldev->disk_conf, new_disk_conf);
> mutex_unlock(&connection->resource->conf_update);
> - kvfree_rcu(old_disk_conf);
> + kvfree_rcu_mightsleep(old_disk_conf);
>
> drbd_info(device, "Peer sets u_size to %lu sectors (old: %lu)\n",
> (unsigned long)p_usize, (unsigned long)my_usize);
> diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
> index 75d13ea0024f..2aeea295fa28 100644
> --- a/drivers/block/drbd/drbd_state.c
> +++ b/drivers/block/drbd/drbd_state.c
> @@ -2071,7 +2071,7 @@ static int w_after_conn_state_ch(struct drbd_work *w, int unused)
> conn_free_crypto(connection);
> mutex_unlock(&connection->resource->conf_update);
>
> - kvfree_rcu(old_conf);
> + kvfree_rcu_mightsleep(old_conf);
> }
>
> if (ns_max.susp_fen) {
> --
> 2.40.0.rc1.284.g88254d51c5-goog
>
On Wed, Mar 15, 2023 at 2:19 PM Joel Fernandes (Google)
<[email protected]> wrote:
>
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
> deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
> kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
> of the single-argument forms, which can introduce functionality bugs in
> atomic contexts and latency bugs in non-atomic contexts.
In a world where patches anxiously await their precious Ack, could
today be our lucky day on this one?
We need Acks to take this in for 6.4. David? Others?
- Joel
>
> Cc: Ariel Levkovich <[email protected]>
> Cc: Saeed Mahameed <[email protected]>
> Cc: Vlad Buslov <[email protected]>
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
> index ca834bbcb44f..8afcec0c5d3c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
> @@ -242,7 +242,7 @@ mlx5e_int_port_remove(struct mlx5e_tc_int_port_priv *priv,
> mlx5_del_flow_rules(int_port->rx_rule);
> mapping_remove(ctx, int_port->mapping);
> mlx5e_int_port_metadata_free(priv, int_port->match_metadata);
> - kfree_rcu(int_port);
> + kfree_rcu_mightsleep(int_port);
> priv->num_ports--;
> }
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> index 08d0929e8260..b811dad7370a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> @@ -670,7 +670,7 @@ static int mlx5e_macsec_del_txsa(struct macsec_context *ctx)
>
> mlx5e_macsec_cleanup_sa(macsec, tx_sa, true);
> mlx5_destroy_encryption_key(macsec->mdev, tx_sa->enc_key_id);
> - kfree_rcu(tx_sa);
> + kfree_rcu_mightsleep(tx_sa);
> macsec_device->tx_sa[assoc_num] = NULL;
>
> out:
> @@ -849,7 +849,7 @@ static void macsec_del_rxsc_ctx(struct mlx5e_macsec *macsec, struct mlx5e_macsec
> xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
> metadata_dst_free(rx_sc->md_dst);
> kfree(rx_sc->sc_xarray_element);
> - kfree_rcu(rx_sc);
> + kfree_rcu_mightsleep(rx_sc);
> }
>
> static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
> --
> 2.40.0.rc1.284.g88254d51c5-goog
>
On Wed, Mar 15, 2023 at 2:19 PM Joel Fernandes (Google)
<[email protected]> wrote:
>
> From: "Uladzislau Rezki (Sony)" <[email protected]>
>
> The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
> deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
> kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
> of the single-argument forms, which can introduce functionality bugs in
> atomic contexts and latency bugs in non-atomic contexts.
>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
Could anyone from the networking side Ack this patch so we can take it for 6.4?
Eric or David?
- Joel
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
> net/core/sysctl_net_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 74842b453407..782273bb93c2 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -177,7 +177,7 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
> if (orig_sock_table) {
> static_branch_dec(&rps_needed);
> static_branch_dec(&rfs_needed);
> - kvfree_rcu(orig_sock_table);
> + kvfree_rcu_mightsleep(orig_sock_table);
> }
> }
> }
> @@ -215,7 +215,7 @@ static int flow_limit_cpu_sysctl(struct ctl_table *table, int write,
> lockdep_is_held(&flow_limit_update_mutex));
> if (cur && !cpumask_test_cpu(i, mask)) {
> RCU_INIT_POINTER(sd->flow_limit, NULL);
> - kfree_rcu(cur);
> + kfree_rcu_mightsleep(cur);
> } else if (!cur && cpumask_test_cpu(i, mask)) {
> cur = kzalloc_node(len, GFP_KERNEL,
> cpu_to_node(i));
> --
> 2.40.0.rc1.284.g88254d51c5-goog
>
On 26 Mar 08:34, Joel Fernandes wrote:
>On Wed, Mar 15, 2023 at 2:19 PM Joel Fernandes (Google)
><[email protected]> wrote:
>>
>> From: "Uladzislau Rezki (Sony)" <[email protected]>
>>
>> The kfree_rcu() and kvfree_rcu() macros' single-argument forms are
>> deprecated. Therefore switch to the new kfree_rcu_mightsleep() and
>> kvfree_rcu_mightsleep() variants. The goal is to avoid accidental use
>> of the single-argument forms, which can introduce functionality bugs in
>> atomic contexts and latency bugs in non-atomic contexts.
>
>In a world where patches anxiously await their precious Ack, could
>today be our lucky day on this one?
>
>We need Acks to take this in for 6.4. David? Others?
>
For mlx5 usually me, but since this is a larger series that is not mlx5
centric and targeting multiple tree, I really don't know which subsystem
you should be targeting.. for netdev submissions you need to specify the
targeted branch e.g. [PATCH v2 net-next 06/14] ...
FWIW:
Reviewed-by: Saeed Mahameed <[email protected]>
> - Joel
>
>
>>
>> Cc: Ariel Levkovich <[email protected]>
>> Cc: Saeed Mahameed <[email protected]>
>> Cc: Vlad Buslov <[email protected]>
>> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
>> Signed-off-by: Paul E. McKenney <[email protected]>
>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c | 2 +-
>> drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 4 ++--
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
>> index ca834bbcb44f..8afcec0c5d3c 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c
>> @@ -242,7 +242,7 @@ mlx5e_int_port_remove(struct mlx5e_tc_int_port_priv *priv,
>> mlx5_del_flow_rules(int_port->rx_rule);
>> mapping_remove(ctx, int_port->mapping);
>> mlx5e_int_port_metadata_free(priv, int_port->match_metadata);
>> - kfree_rcu(int_port);
>> + kfree_rcu_mightsleep(int_port);
>> priv->num_ports--;
>> }
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
>> index 08d0929e8260..b811dad7370a 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
>> @@ -670,7 +670,7 @@ static int mlx5e_macsec_del_txsa(struct macsec_context *ctx)
>>
>> mlx5e_macsec_cleanup_sa(macsec, tx_sa, true);
>> mlx5_destroy_encryption_key(macsec->mdev, tx_sa->enc_key_id);
>> - kfree_rcu(tx_sa);
>> + kfree_rcu_mightsleep(tx_sa);
>> macsec_device->tx_sa[assoc_num] = NULL;
>>
>> out:
>> @@ -849,7 +849,7 @@ static void macsec_del_rxsc_ctx(struct mlx5e_macsec *macsec, struct mlx5e_macsec
>> xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
>> metadata_dst_free(rx_sc->md_dst);
>> kfree(rx_sc->sc_xarray_element);
>> - kfree_rcu(rx_sc);
>> + kfree_rcu_mightsleep(rx_sc);
>> }
>>
>> static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
>> --
>> 2.40.0.rc1.284.g88254d51c5-goog
>>
On Sun, 26 Mar 2023 08:28:45 -0400 Joel Fernandes wrote:
> > Cc: Eric Dumazet <[email protected]>
> > Cc: David S. Miller <[email protected]>
>
> Could anyone from the networking side Ack this patch so we can take it for 6.4?
>
> Eric or David?
Let me help you. Perhaps it's a data point against keeping maintainers
in an alphabetical order :-)
Acked-by: Jakub Kicinski <[email protected]>
On 3/26/23 6:27 AM, Joel Fernandes wrote:
> On Wed, Mar 15, 2023 at 2:19 PM Joel Fernandes (Google)
> <[email protected]> wrote:
>>
>> From: "Uladzislau Rezki (Sony)" <[email protected]>
>>
>> The kvfree_rcu() macro's single-argument form is deprecated. Therefore
>> switch to the new kvfree_rcu_mightsleep() variant. The goal is to
>> avoid accidental use of the single-argument forms, which can introduce
>> functionality bugs in atomic contexts and latency bugs in non-atomic
>> contexts.
>>
>> Cc: Jens Axboe <[email protected]>
>> Cc: Philipp Reisner <[email protected]>
>> Cc: Lars Ellenberg <[email protected]>
>
> Jens/Others, any chance for an Ack here?
Begrudgingly-acked-by: Jens Axboe <[email protected]>
--
Jens Axboe