* [PATCH RESEND net-next 0/5] WWAN debugfs tweaks
@ 2021-11-28 12:55 Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code Sergey Ryazanov
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
Resend with proper target tree. Also I should mention that the series is
mostly compile-tested since I do not have IOSM supported device, so it
needs Ack from the IOSM developers.
This is a follow-up series to just applied IOSM (and WWAN) debugfs
interface support [1]. The series has two main goals:
1. move the driver-specific debugfs knobs to a subdirectory;
2. make the debugfs interface optional for both IOSM and for the WWAN
core.
As for the first part, I must say that it was my mistake. I suggested to
place debugfs entries under a common per WWAN device directory. But I
missed the driver subdirectory in the example, so it become:
/sys/kernel/debugfs/wwan/wwan0/trace
Since the traces collection is a driver-specific feature, it is better
to keep it under the driver-specific subdirectory:
/sys/kernel/debugfs/wwan/wwan0/iosm/trace
It is desirable to be able to entirely disable the debugfs interface. It
can be disabled for several reasons, including security and consumed
storage space.
The changes themselves are relatively simple, but require a code
rearrangement. So to make changes clear, I chose to split them into
preparatory and main changes and properly describe each of them.
1. https://lore.kernel.org/netdev/20211120162155.1216081-1-m.chetan.kumar@linux.intel.com
Cc: M Chetan Kumar <m.chetan.kumar@intel.com>
Cc: Intel Corporation <linuxwwan@intel.com>
Cc: Loic Poulain <loic.poulain@linaro.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Sergey Ryazanov (5):
net: wwan: iosm: consolidate trace port init code
net: wwan: iosm: allow trace port be uninitialized
net: wwan: iosm: move debugfs knobs into a subdir
net: wwan: iosm: make debugfs optional
net: wwan: core: make debugfs optional
drivers/net/wwan/Kconfig | 17 +++++++++++++
drivers/net/wwan/iosm/Makefile | 5 +++-
drivers/net/wwan/iosm/iosm_ipc_debugfs.c | 29 +++++++++++++++++++++++
drivers/net/wwan/iosm/iosm_ipc_debugfs.h | 17 +++++++++++++
drivers/net/wwan/iosm/iosm_ipc_imem.c | 13 ++++------
drivers/net/wwan/iosm/iosm_ipc_imem.h | 5 ++++
drivers/net/wwan/iosm/iosm_ipc_imem_ops.c | 18 --------------
drivers/net/wwan/iosm/iosm_ipc_imem_ops.h | 2 +-
drivers/net/wwan/iosm/iosm_ipc_trace.c | 23 ++++++++++++------
drivers/net/wwan/iosm/iosm_ipc_trace.h | 25 ++++++++++++++++++-
drivers/net/wwan/wwan_core.c | 8 +++++++
include/linux/wwan.h | 7 ++++++
12 files changed, 133 insertions(+), 36 deletions(-)
create mode 100644 drivers/net/wwan/iosm/iosm_ipc_debugfs.c
create mode 100644 drivers/net/wwan/iosm/iosm_ipc_debugfs.h
--
2.32.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
@ 2021-11-28 12:55 ` Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 2/5] net: wwan: iosm: allow trace port be uninitialized Sergey Ryazanov
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
Move the channel related structures initialization from
ipc_imem_channel_init() to ipc_trace_init() and call it directly. On the
one hand, this makes the trace port initialization symmetric to the
deitialization, that is, it removes the additional wrapper.
On the other hand, this change consolidates the trace port related code
into a single source file, what facilitates an upcoming disabling of
this functionality by a user choise.
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_imem.c | 2 +-
drivers/net/wwan/iosm/iosm_ipc_imem_ops.c | 18 ------------------
drivers/net/wwan/iosm/iosm_ipc_imem_ops.h | 2 +-
drivers/net/wwan/iosm/iosm_ipc_trace.c | 8 +++++++-
4 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 1be07114c85d..49bdadb855e5 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -554,7 +554,7 @@ static void ipc_imem_run_state_worker(struct work_struct *instance)
ctrl_chl_idx++;
}
- ipc_imem->trace = ipc_imem_trace_channel_init(ipc_imem);
+ ipc_imem->trace = ipc_trace_init(ipc_imem);
if (!ipc_imem->trace) {
dev_err(ipc_imem->dev, "trace channel init failed");
return;
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
index 43f1796a8984..d2072a84ab08 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
@@ -11,7 +11,6 @@
#include "iosm_ipc_imem_ops.h"
#include "iosm_ipc_port.h"
#include "iosm_ipc_task_queue.h"
-#include "iosm_ipc_trace.h"
/* Open a packet data online channel between the network layer and CP. */
int ipc_imem_sys_wwan_open(struct iosm_imem *ipc_imem, int if_id)
@@ -108,23 +107,6 @@ void ipc_imem_wwan_channel_init(struct iosm_imem *ipc_imem,
"failed to register the ipc_wwan interfaces");
}
-/**
- * ipc_imem_trace_channel_init - Initializes trace channel.
- * @ipc_imem: Pointer to iosm_imem struct.
- *
- * Returns: Pointer to trace instance on success else NULL
- */
-struct iosm_trace *ipc_imem_trace_channel_init(struct iosm_imem *ipc_imem)
-{
- struct ipc_chnl_cfg chnl_cfg = { 0 };
-
- ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
- ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
- IRQ_MOD_OFF);
-
- return ipc_trace_init(ipc_imem);
-}
-
/* Map SKB to DMA for transfer */
static int ipc_imem_map_skb_to_dma(struct iosm_imem *ipc_imem,
struct sk_buff *skb)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
index e36ee2782629..f8afb217d9e2 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
@@ -141,5 +141,5 @@ int ipc_imem_sys_devlink_read(struct iosm_devlink *ipc_devlink, u8 *data,
*/
int ipc_imem_sys_devlink_write(struct iosm_devlink *ipc_devlink,
unsigned char *buf, int count);
-struct iosm_trace *ipc_imem_trace_channel_init(struct iosm_imem *ipc_imem);
+
#endif
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.c b/drivers/net/wwan/iosm/iosm_ipc_trace.c
index c5fa12599c2b..5f5cfd39bede 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.c
@@ -132,9 +132,15 @@ static const struct file_operations ipc_trace_fops = {
*/
struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
{
- struct iosm_trace *ipc_trace = kzalloc(sizeof(*ipc_trace), GFP_KERNEL);
+ struct ipc_chnl_cfg chnl_cfg = { 0 };
+ struct iosm_trace *ipc_trace;
struct dentry *debugfs_pdev;
+ ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
+ ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
+ IRQ_MOD_OFF);
+
+ ipc_trace = kzalloc(sizeof(*ipc_trace), GFP_KERNEL);
if (!ipc_trace)
return NULL;
--
2.32.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND net-next 2/5] net: wwan: iosm: allow trace port be uninitialized
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code Sergey Ryazanov
@ 2021-11-28 12:55 ` Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 3/5] net: wwan: iosm: move debugfs knobs into a subdir Sergey Ryazanov
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
Collecting modem firmware traces is optional for the regular modem use.
There are not many reasons for aborting device initialization due to an
inability to initialize the trace port and (or) its debugfs interface.
So, demote the initialization failure erro message into a warning and do
not break the initialization sequence in this case. Rework packet
processing and deinitialization so that they do not crash in case of
uninitialized trace port.
This change is mainly a preparation for an upcoming configuration option
introduction that will help disable driver debugfs functionality.
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_imem.c | 8 +++-----
drivers/net/wwan/iosm/iosm_ipc_trace.c | 3 +++
drivers/net/wwan/iosm/iosm_ipc_trace.h | 5 +++++
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 49bdadb855e5..a60b93cefd2e 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -272,7 +272,7 @@ static void ipc_imem_dl_skb_process(struct iosm_imem *ipc_imem,
if (port_id == IPC_MEM_CTRL_CHL_ID_7)
ipc_imem_sys_devlink_notify_rx(ipc_imem->ipc_devlink,
skb);
- else if (port_id == ipc_imem->trace->chl_id)
+ else if (ipc_is_trace_channel(ipc_imem, port_id))
ipc_trace_port_rx(ipc_imem->trace, skb);
else
wwan_port_rx(ipc_imem->ipc_port[port_id]->iosm_port,
@@ -555,10 +555,8 @@ static void ipc_imem_run_state_worker(struct work_struct *instance)
}
ipc_imem->trace = ipc_trace_init(ipc_imem);
- if (!ipc_imem->trace) {
- dev_err(ipc_imem->dev, "trace channel init failed");
- return;
- }
+ if (!ipc_imem->trace)
+ dev_warn(ipc_imem->dev, "trace channel init failed");
ipc_task_queue_send_task(ipc_imem, ipc_imem_send_mdm_rdy_cb, 0, NULL, 0,
false);
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.c b/drivers/net/wwan/iosm/iosm_ipc_trace.c
index 5f5cfd39bede..c588a394cd94 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.c
@@ -172,6 +172,9 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
*/
void ipc_trace_deinit(struct iosm_trace *ipc_trace)
{
+ if (!ipc_trace)
+ return;
+
debugfs_remove(ipc_trace->ctrl_file);
relay_close(ipc_trace->ipc_rchan);
mutex_destroy(&ipc_trace->trc_mutex);
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.h b/drivers/net/wwan/iosm/iosm_ipc_trace.h
index 53346183af9c..419540c91219 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.h
@@ -45,6 +45,11 @@ struct iosm_trace {
enum trace_ctrl_mode mode;
};
+static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
+{
+ return ipc_mem->trace && ipc_mem->trace->chl_id == chl_id;
+}
+
struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem);
void ipc_trace_deinit(struct iosm_trace *ipc_trace);
void ipc_trace_port_rx(struct iosm_trace *ipc_trace, struct sk_buff *skb);
--
2.32.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND net-next 3/5] net: wwan: iosm: move debugfs knobs into a subdir
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 2/5] net: wwan: iosm: allow trace port be uninitialized Sergey Ryazanov
@ 2021-11-28 12:55 ` Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 4/5] net: wwan: iosm: make debugfs optional Sergey Ryazanov
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
The modem traces collection is a device (and so driver) specific option.
Therefore, move the related debugfs files into a driver-specific
subdirectory under the common per WWAN device directory.
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/Makefile | 1 +
drivers/net/wwan/iosm/iosm_ipc_debugfs.c | 29 ++++++++++++++++++++++++
drivers/net/wwan/iosm/iosm_ipc_debugfs.h | 12 ++++++++++
drivers/net/wwan/iosm/iosm_ipc_imem.c | 7 +++---
drivers/net/wwan/iosm/iosm_ipc_imem.h | 1 +
drivers/net/wwan/iosm/iosm_ipc_trace.c | 6 ++---
6 files changed, 48 insertions(+), 8 deletions(-)
create mode 100644 drivers/net/wwan/iosm/iosm_ipc_debugfs.c
create mode 100644 drivers/net/wwan/iosm/iosm_ipc_debugfs.h
diff --git a/drivers/net/wwan/iosm/Makefile b/drivers/net/wwan/iosm/Makefile
index 5c2528beca2a..5091f664af0d 100644
--- a/drivers/net/wwan/iosm/Makefile
+++ b/drivers/net/wwan/iosm/Makefile
@@ -22,6 +22,7 @@ iosm-y = \
iosm_ipc_devlink.o \
iosm_ipc_flash.o \
iosm_ipc_coredump.o \
+ iosm_ipc_debugfs.o \
iosm_ipc_trace.o
obj-$(CONFIG_IOSM) := iosm.o
diff --git a/drivers/net/wwan/iosm/iosm_ipc_debugfs.c b/drivers/net/wwan/iosm/iosm_ipc_debugfs.c
new file mode 100644
index 000000000000..f2f57751a7d2
--- /dev/null
+++ b/drivers/net/wwan/iosm/iosm_ipc_debugfs.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020-2021 Intel Corporation.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/wwan.h>
+
+#include "iosm_ipc_imem.h"
+#include "iosm_ipc_trace.h"
+#include "iosm_ipc_debugfs.h"
+
+void ipc_debugfs_init(struct iosm_imem *ipc_imem)
+{
+ struct dentry *debugfs_pdev = wwan_get_debugfs_dir(ipc_imem->dev);
+
+ ipc_imem->debugfs_dir = debugfs_create_dir(KBUILD_MODNAME,
+ debugfs_pdev);
+
+ ipc_imem->trace = ipc_trace_init(ipc_imem);
+ if (!ipc_imem->trace)
+ dev_warn(ipc_imem->dev, "trace channel init failed");
+}
+
+void ipc_debugfs_deinit(struct iosm_imem *ipc_imem)
+{
+ ipc_trace_deinit(ipc_imem->trace);
+ debugfs_remove_recursive(ipc_imem->debugfs_dir);
+}
diff --git a/drivers/net/wwan/iosm/iosm_ipc_debugfs.h b/drivers/net/wwan/iosm/iosm_ipc_debugfs.h
new file mode 100644
index 000000000000..35788039f13f
--- /dev/null
+++ b/drivers/net/wwan/iosm/iosm_ipc_debugfs.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (C) 2020-2021 Intel Corporation.
+ */
+
+#ifndef IOSM_IPC_DEBUGFS_H
+#define IOSM_IPC_DEBUGFS_H
+
+void ipc_debugfs_init(struct iosm_imem *ipc_imem);
+void ipc_debugfs_deinit(struct iosm_imem *ipc_imem);
+
+#endif
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index a60b93cefd2e..25b889922912 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -11,6 +11,7 @@
#include "iosm_ipc_imem.h"
#include "iosm_ipc_port.h"
#include "iosm_ipc_trace.h"
+#include "iosm_ipc_debugfs.h"
/* Check the wwan ips if it is valid with Channel as input. */
static int ipc_imem_check_wwan_ips(struct ipc_mem_channel *chnl)
@@ -554,9 +555,7 @@ static void ipc_imem_run_state_worker(struct work_struct *instance)
ctrl_chl_idx++;
}
- ipc_imem->trace = ipc_trace_init(ipc_imem);
- if (!ipc_imem->trace)
- dev_warn(ipc_imem->dev, "trace channel init failed");
+ ipc_debugfs_init(ipc_imem);
ipc_task_queue_send_task(ipc_imem, ipc_imem_send_mdm_rdy_cb, 0, NULL, 0,
false);
@@ -1173,7 +1172,7 @@ void ipc_imem_cleanup(struct iosm_imem *ipc_imem)
if (test_and_clear_bit(FULLY_FUNCTIONAL, &ipc_imem->flag)) {
ipc_mux_deinit(ipc_imem->mux);
- ipc_trace_deinit(ipc_imem->trace);
+ ipc_debugfs_deinit(ipc_imem);
ipc_wwan_deinit(ipc_imem->wwan);
ipc_port_deinit(ipc_imem->ipc_port);
}
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.h b/drivers/net/wwan/iosm/iosm_ipc_imem.h
index cec38009c44a..df3b471f6fa9 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.h
@@ -380,6 +380,7 @@ struct iosm_imem {
ev_mux_net_transmit_pending:1,
reset_det_n:1,
pcie_wake_n:1;
+ struct dentry *debugfs_dir;
};
/**
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.c b/drivers/net/wwan/iosm/iosm_ipc_trace.c
index c588a394cd94..5243ead90b5f 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.c
@@ -134,7 +134,6 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
{
struct ipc_chnl_cfg chnl_cfg = { 0 };
struct iosm_trace *ipc_trace;
- struct dentry *debugfs_pdev;
ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
@@ -150,15 +149,14 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
ipc_trace->chl_id = IPC_MEM_CTRL_CHL_ID_3;
mutex_init(&ipc_trace->trc_mutex);
- debugfs_pdev = wwan_get_debugfs_dir(ipc_imem->dev);
ipc_trace->ctrl_file = debugfs_create_file(IOSM_TRC_DEBUGFS_TRACE_CTRL,
IOSM_TRC_FILE_PERM,
- debugfs_pdev,
+ ipc_imem->debugfs_dir,
ipc_trace, &ipc_trace_fops);
ipc_trace->ipc_rchan = relay_open(IOSM_TRC_DEBUGFS_TRACE,
- debugfs_pdev,
+ ipc_imem->debugfs_dir,
IOSM_TRC_SUB_BUFF_SIZE,
IOSM_TRC_N_SUB_BUFF,
&relay_callbacks, NULL);
--
2.32.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND net-next 4/5] net: wwan: iosm: make debugfs optional
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
` (2 preceding siblings ...)
2021-11-28 12:55 ` [PATCH RESEND net-next 3/5] net: wwan: iosm: move debugfs knobs into a subdir Sergey Ryazanov
@ 2021-11-28 12:55 ` Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
2021-11-28 18:27 ` [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Leon Romanovsky
5 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
Collecting modem firmware traces is optional for the regular modem use.
Some distros and users will want to disable this feature for security or
kernel size reasons. So add a configuration option that allows to
completely disable the driver debugfs interface.
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/Kconfig | 8 ++++++++
drivers/net/wwan/iosm/Makefile | 4 +++-
drivers/net/wwan/iosm/iosm_ipc_debugfs.h | 5 +++++
drivers/net/wwan/iosm/iosm_ipc_imem.c | 2 +-
drivers/net/wwan/iosm/iosm_ipc_imem.h | 4 ++++
drivers/net/wwan/iosm/iosm_ipc_trace.c | 6 ++++--
drivers/net/wwan/iosm/iosm_ipc_trace.h | 20 +++++++++++++++++++-
7 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
index 17543be14665..e204e74edcec 100644
--- a/drivers/net/wwan/Kconfig
+++ b/drivers/net/wwan/Kconfig
@@ -80,6 +80,14 @@ config IOSM
If unsure, say N.
+config IOSM_DEBUGFS
+ bool "IOSM Debugfs support"
+ depends on IOSM && DEBUG_FS
+ help
+ Enables debugfs driver interface for traces collection.
+
+ If unsure, say N.
+
endif # WWAN
endmenu
diff --git a/drivers/net/wwan/iosm/Makefile b/drivers/net/wwan/iosm/Makefile
index 5091f664af0d..bf28b29f6151 100644
--- a/drivers/net/wwan/iosm/Makefile
+++ b/drivers/net/wwan/iosm/Makefile
@@ -21,7 +21,9 @@ iosm-y = \
iosm_ipc_mux_codec.o \
iosm_ipc_devlink.o \
iosm_ipc_flash.o \
- iosm_ipc_coredump.o \
+ iosm_ipc_coredump.o
+
+iosm-$(CONFIG_IOSM_DEBUGFS) += \
iosm_ipc_debugfs.o \
iosm_ipc_trace.o
diff --git a/drivers/net/wwan/iosm/iosm_ipc_debugfs.h b/drivers/net/wwan/iosm/iosm_ipc_debugfs.h
index 35788039f13f..3e3bb968fa03 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_debugfs.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_debugfs.h
@@ -6,7 +6,12 @@
#ifndef IOSM_IPC_DEBUGFS_H
#define IOSM_IPC_DEBUGFS_H
+#ifdef CONFIG_IOSM_DEBUGFS
void ipc_debugfs_init(struct iosm_imem *ipc_imem);
void ipc_debugfs_deinit(struct iosm_imem *ipc_imem);
+#else
+static inline void ipc_debugfs_init(struct iosm_imem *ipc_imem) {}
+static inline void ipc_debugfs_deinit(struct iosm_imem *ipc_imem) {}
+#endif
#endif
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 25b889922912..2a6ddd7c6c88 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -274,7 +274,7 @@ static void ipc_imem_dl_skb_process(struct iosm_imem *ipc_imem,
ipc_imem_sys_devlink_notify_rx(ipc_imem->ipc_devlink,
skb);
else if (ipc_is_trace_channel(ipc_imem, port_id))
- ipc_trace_port_rx(ipc_imem->trace, skb);
+ ipc_trace_port_rx(ipc_imem, skb);
else
wwan_port_rx(ipc_imem->ipc_port[port_id]->iosm_port,
skb);
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.h b/drivers/net/wwan/iosm/iosm_ipc_imem.h
index df3b471f6fa9..cca4b32c63cd 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.h
@@ -350,7 +350,9 @@ struct iosm_imem {
struct iosm_mux *mux;
struct iosm_cdev *ipc_port[IPC_MEM_MAX_CHANNELS];
struct iosm_pcie *pcie;
+#ifdef CONFIG_IOSM_DEBUGFS
struct iosm_trace *trace;
+#endif
struct device *dev;
enum ipc_mem_device_ipc_state ipc_requested_state;
struct ipc_mem_channel channels[IPC_MEM_MAX_CHANNELS];
@@ -380,7 +382,9 @@ struct iosm_imem {
ev_mux_net_transmit_pending:1,
reset_det_n:1,
pcie_wake_n:1;
+#ifdef CONFIG_IOSM_DEBUGFS
struct dentry *debugfs_dir;
+#endif
};
/**
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.c b/drivers/net/wwan/iosm/iosm_ipc_trace.c
index 5243ead90b5f..eeecfa3d10c5 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.c
@@ -17,11 +17,13 @@
/**
* ipc_trace_port_rx - Receive trace packet from cp and write to relay buffer
- * @ipc_trace: Pointer to the ipc trace data-struct
+ * @ipc_imem: Pointer to iosm_imem structure
* @skb: Pointer to struct sk_buff
*/
-void ipc_trace_port_rx(struct iosm_trace *ipc_trace, struct sk_buff *skb)
+void ipc_trace_port_rx(struct iosm_imem *ipc_imem, struct sk_buff *skb)
{
+ struct iosm_trace *ipc_trace = ipc_imem->trace;
+
if (ipc_trace->ipc_rchan)
relay_write(ipc_trace->ipc_rchan, skb->data, skb->len);
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.h b/drivers/net/wwan/iosm/iosm_ipc_trace.h
index 419540c91219..0d74836df90c 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.h
@@ -45,6 +45,8 @@ struct iosm_trace {
enum trace_ctrl_mode mode;
};
+#ifdef CONFIG_IOSM_DEBUGFS
+
static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
{
return ipc_mem->trace && ipc_mem->trace->chl_id == chl_id;
@@ -52,5 +54,21 @@ static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem);
void ipc_trace_deinit(struct iosm_trace *ipc_trace);
-void ipc_trace_port_rx(struct iosm_trace *ipc_trace, struct sk_buff *skb);
+void ipc_trace_port_rx(struct iosm_imem *ipc_imem, struct sk_buff *skb);
+
+#else
+
+static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
+{
+ return false;
+}
+
+static inline void ipc_trace_port_rx(struct iosm_imem *ipc_imem,
+ struct sk_buff *skb)
+{
+ dev_kfree_skb(skb);
+}
+
+#endif
+
#endif
--
2.32.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
` (3 preceding siblings ...)
2021-11-28 12:55 ` [PATCH RESEND net-next 4/5] net: wwan: iosm: make debugfs optional Sergey Ryazanov
@ 2021-11-28 12:55 ` Sergey Ryazanov
2021-11-28 17:01 ` Johannes Berg
` (2 more replies)
2021-11-28 18:27 ` [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Leon Romanovsky
5 siblings, 3 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 12:55 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain, Johannes Berg
Current WWAN debugfs interface does not take too much space, but it is
useless without driver-specific debugfs interfaces. To avoid overloading
debugfs with empty directories, make the common WWAN debugfs interface
optional. And force its selection if any driver-specific interface (only
IOSM at the moment) is enabled by user.
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/Kconfig | 9 +++++++++
drivers/net/wwan/wwan_core.c | 8 ++++++++
include/linux/wwan.h | 7 +++++++
3 files changed, 24 insertions(+)
diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
index e204e74edcec..6e1ef08650c9 100644
--- a/drivers/net/wwan/Kconfig
+++ b/drivers/net/wwan/Kconfig
@@ -16,6 +16,14 @@ config WWAN
if WWAN
+config WWAN_DEBUGFS
+ bool "WWAN subsystem common debugfs interface"
+ depends on DEBUG_FS
+ help
+ Enables common debugfs infrastructure for WWAN devices.
+
+ If unsure, say N.
+
config WWAN_HWSIM
tristate "Simulated WWAN device"
help
@@ -83,6 +91,7 @@ config IOSM
config IOSM_DEBUGFS
bool "IOSM Debugfs support"
depends on IOSM && DEBUG_FS
+ select WWAN_DEBUGFS
help
Enables debugfs driver interface for traces collection.
diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 5bf62dc35ac7..b41104129d1a 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -146,6 +146,7 @@ static struct wwan_device *wwan_dev_get_by_name(const char *name)
return to_wwan_dev(dev);
}
+#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *wwan_get_debugfs_dir(struct device *parent)
{
struct wwan_device *wwandev;
@@ -157,6 +158,7 @@ struct dentry *wwan_get_debugfs_dir(struct device *parent)
return wwandev->debugfs_dir;
}
EXPORT_SYMBOL_GPL(wwan_get_debugfs_dir);
+#endif
/* This function allocates and registers a new WWAN device OR if a WWAN device
* already exist for the given parent, it gets a reference and return it.
@@ -207,8 +209,10 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
}
wwandev_name = kobject_name(&wwandev->dev.kobj);
+#ifdef CONFIG_WWAN_DEBUGFS
wwandev->debugfs_dir = debugfs_create_dir(wwandev_name,
wwan_debugfs_dir);
+#endif
done_unlock:
mutex_unlock(&wwan_register_lock);
@@ -240,7 +244,9 @@ static void wwan_remove_dev(struct wwan_device *wwandev)
ret = device_for_each_child(&wwandev->dev, NULL, is_wwan_child);
if (!ret) {
+#ifdef CONFIG_WWAN_DEBUGFS
debugfs_remove_recursive(wwandev->debugfs_dir);
+#endif
device_unregister(&wwandev->dev);
} else {
put_device(&wwandev->dev);
@@ -1140,7 +1146,9 @@ static int __init wwan_init(void)
goto destroy;
}
+#ifdef CONFIG_WWAN_DEBUGFS
wwan_debugfs_dir = debugfs_create_dir("wwan", NULL);
+#endif
return 0;
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index 1646aa3e6779..b84ccf7d34da 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h
@@ -171,6 +171,13 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
void wwan_unregister_ops(struct device *parent);
+#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *wwan_get_debugfs_dir(struct device *parent);
+#else
+static inline struct dentry *wwan_get_debugfs_dir(struct device *parent)
+{
+ return NULL;
+}
+#endif
#endif /* __WWAN_H */
--
2.32.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
@ 2021-11-28 17:01 ` Johannes Berg
2021-11-28 23:45 ` Sergey Ryazanov
2021-11-28 17:04 ` kernel test robot
2021-11-28 17:05 ` Johannes Berg
2 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2021-11-28 17:01 UTC (permalink / raw)
To: Sergey Ryazanov, David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain
On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>
> +config WWAN_DEBUGFS
> + bool "WWAN subsystem common debugfs interface"
> + depends on DEBUG_FS
> + help
> + Enables common debugfs infrastructure for WWAN devices.
> +
> + If unsure, say N.
>
I wonder if that really should even say "If unsure, say N." because
really, once you have DEBUG_FS enabled, you can expect things to show up
there?
And I'd probably even argue that it should be
bool "..." if EXPERT
default y
depends on DEBUG_FS
so most people aren't even bothered by the question?
> config WWAN_HWSIM
> tristate "Simulated WWAN device"
> help
> @@ -83,6 +91,7 @@ config IOSM
> config IOSM_DEBUGFS
> bool "IOSM Debugfs support"
> depends on IOSM && DEBUG_FS
> + select WWAN_DEBUGFS
>
I guess it's kind of a philosophical question, but perhaps it would make
more sense for that to be "depends on" (and then you can remove &&
DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
debugfs and not have to worry about individual driver settings?
And after that change, I'd probably just make this one "def_bool y"
instead of asking the user.
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
@ 2021-11-28 17:04 ` kernel test robot
2021-11-28 17:04 ` kernel test robot
2021-11-28 17:05 ` Johannes Berg
2 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-11-28 17:04 UTC (permalink / raw)
To: Sergey Ryazanov, David S . Miller, Jakub Kicinski
Cc: llvm, kbuild-all, netdev, M Chetan Kumar, Intel Corporation,
Loic Poulain, Johannes Berg
Hi Sergey,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Sergey-Ryazanov/WWAN-debugfs-tweaks/20211128-210031
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d40ce48cb3a68b54be123a1f99157c5ac613e260
config: i386-randconfig-a001-20211128 (https://download.01.org/0day-ci/archive/20211129/202111290026.HEuigppG-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5c64d8ef8cc0c0ed3e0f2ae693d99e7f70f20a84)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/89654e5e973a53b8375f37395c03359c59b63a99
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sergey-Ryazanov/WWAN-debugfs-tweaks/20211128-210031
git checkout 89654e5e973a53b8375f37395c03359c59b63a99
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/wwan/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/wwan/wwan_core.c:171:14: warning: variable 'wwandev_name' set but not used [-Wunused-but-set-variable]
const char *wwandev_name;
^
1 warning generated.
vim +/wwandev_name +171 drivers/net/wwan/wwan_core.c
c4804670026b93f M Chetan Kumar 2021-11-20 162
9a44c1cc6388762 Loic Poulain 2021-04-16 163 /* This function allocates and registers a new WWAN device OR if a WWAN device
9a44c1cc6388762 Loic Poulain 2021-04-16 164 * already exist for the given parent, it gets a reference and return it.
9a44c1cc6388762 Loic Poulain 2021-04-16 165 * This function is not exported (for now), it is called indirectly via
9a44c1cc6388762 Loic Poulain 2021-04-16 166 * wwan_create_port().
9a44c1cc6388762 Loic Poulain 2021-04-16 167 */
9a44c1cc6388762 Loic Poulain 2021-04-16 168 static struct wwan_device *wwan_create_dev(struct device *parent)
9a44c1cc6388762 Loic Poulain 2021-04-16 169 {
9a44c1cc6388762 Loic Poulain 2021-04-16 170 struct wwan_device *wwandev;
c4804670026b93f M Chetan Kumar 2021-11-20 @171 const char *wwandev_name;
9a44c1cc6388762 Loic Poulain 2021-04-16 172 int err, id;
9a44c1cc6388762 Loic Poulain 2021-04-16 173
9a44c1cc6388762 Loic Poulain 2021-04-16 174 /* The 'find-alloc-register' operation must be protected against
9a44c1cc6388762 Loic Poulain 2021-04-16 175 * concurrent execution, a WWAN device is possibly shared between
9a44c1cc6388762 Loic Poulain 2021-04-16 176 * multiple callers or concurrently unregistered from wwan_remove_dev().
9a44c1cc6388762 Loic Poulain 2021-04-16 177 */
9a44c1cc6388762 Loic Poulain 2021-04-16 178 mutex_lock(&wwan_register_lock);
9a44c1cc6388762 Loic Poulain 2021-04-16 179
9a44c1cc6388762 Loic Poulain 2021-04-16 180 /* If wwandev already exists, return it */
9a44c1cc6388762 Loic Poulain 2021-04-16 181 wwandev = wwan_dev_get_by_parent(parent);
9a44c1cc6388762 Loic Poulain 2021-04-16 182 if (!IS_ERR(wwandev))
9a44c1cc6388762 Loic Poulain 2021-04-16 183 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 184
9a44c1cc6388762 Loic Poulain 2021-04-16 185 id = ida_alloc(&wwan_dev_ids, GFP_KERNEL);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 186 if (id < 0) {
d9d5b8961284b00 Andy Shevchenko 2021-08-11 187 wwandev = ERR_PTR(id);
9a44c1cc6388762 Loic Poulain 2021-04-16 188 goto done_unlock;
d9d5b8961284b00 Andy Shevchenko 2021-08-11 189 }
9a44c1cc6388762 Loic Poulain 2021-04-16 190
9a44c1cc6388762 Loic Poulain 2021-04-16 191 wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
9a44c1cc6388762 Loic Poulain 2021-04-16 192 if (!wwandev) {
d9d5b8961284b00 Andy Shevchenko 2021-08-11 193 wwandev = ERR_PTR(-ENOMEM);
9a44c1cc6388762 Loic Poulain 2021-04-16 194 ida_free(&wwan_dev_ids, id);
9a44c1cc6388762 Loic Poulain 2021-04-16 195 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 196 }
9a44c1cc6388762 Loic Poulain 2021-04-16 197
9a44c1cc6388762 Loic Poulain 2021-04-16 198 wwandev->dev.parent = parent;
9a44c1cc6388762 Loic Poulain 2021-04-16 199 wwandev->dev.class = wwan_class;
9a44c1cc6388762 Loic Poulain 2021-04-16 200 wwandev->dev.type = &wwan_dev_type;
9a44c1cc6388762 Loic Poulain 2021-04-16 201 wwandev->id = id;
9a44c1cc6388762 Loic Poulain 2021-04-16 202 dev_set_name(&wwandev->dev, "wwan%d", wwandev->id);
9a44c1cc6388762 Loic Poulain 2021-04-16 203
9a44c1cc6388762 Loic Poulain 2021-04-16 204 err = device_register(&wwandev->dev);
9a44c1cc6388762 Loic Poulain 2021-04-16 205 if (err) {
9a44c1cc6388762 Loic Poulain 2021-04-16 206 put_device(&wwandev->dev);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 207 wwandev = ERR_PTR(err);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 208 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 209 }
9a44c1cc6388762 Loic Poulain 2021-04-16 210
c4804670026b93f M Chetan Kumar 2021-11-20 211 wwandev_name = kobject_name(&wwandev->dev.kobj);
89654e5e973a53b Sergey Ryazanov 2021-11-28 212 #ifdef CONFIG_WWAN_DEBUGFS
c4804670026b93f M Chetan Kumar 2021-11-20 213 wwandev->debugfs_dir = debugfs_create_dir(wwandev_name,
c4804670026b93f M Chetan Kumar 2021-11-20 214 wwan_debugfs_dir);
89654e5e973a53b Sergey Ryazanov 2021-11-28 215 #endif
c4804670026b93f M Chetan Kumar 2021-11-20 216
9a44c1cc6388762 Loic Poulain 2021-04-16 217 done_unlock:
9a44c1cc6388762 Loic Poulain 2021-04-16 218 mutex_unlock(&wwan_register_lock);
9a44c1cc6388762 Loic Poulain 2021-04-16 219
9a44c1cc6388762 Loic Poulain 2021-04-16 220 return wwandev;
9a44c1cc6388762 Loic Poulain 2021-04-16 221 }
9a44c1cc6388762 Loic Poulain 2021-04-16 222
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
@ 2021-11-28 17:04 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-11-28 17:04 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6449 bytes --]
Hi Sergey,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Sergey-Ryazanov/WWAN-debugfs-tweaks/20211128-210031
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d40ce48cb3a68b54be123a1f99157c5ac613e260
config: i386-randconfig-a001-20211128 (https://download.01.org/0day-ci/archive/20211129/202111290026.HEuigppG-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5c64d8ef8cc0c0ed3e0f2ae693d99e7f70f20a84)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/89654e5e973a53b8375f37395c03359c59b63a99
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sergey-Ryazanov/WWAN-debugfs-tweaks/20211128-210031
git checkout 89654e5e973a53b8375f37395c03359c59b63a99
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/wwan/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/wwan/wwan_core.c:171:14: warning: variable 'wwandev_name' set but not used [-Wunused-but-set-variable]
const char *wwandev_name;
^
1 warning generated.
vim +/wwandev_name +171 drivers/net/wwan/wwan_core.c
c4804670026b93f M Chetan Kumar 2021-11-20 162
9a44c1cc6388762 Loic Poulain 2021-04-16 163 /* This function allocates and registers a new WWAN device OR if a WWAN device
9a44c1cc6388762 Loic Poulain 2021-04-16 164 * already exist for the given parent, it gets a reference and return it.
9a44c1cc6388762 Loic Poulain 2021-04-16 165 * This function is not exported (for now), it is called indirectly via
9a44c1cc6388762 Loic Poulain 2021-04-16 166 * wwan_create_port().
9a44c1cc6388762 Loic Poulain 2021-04-16 167 */
9a44c1cc6388762 Loic Poulain 2021-04-16 168 static struct wwan_device *wwan_create_dev(struct device *parent)
9a44c1cc6388762 Loic Poulain 2021-04-16 169 {
9a44c1cc6388762 Loic Poulain 2021-04-16 170 struct wwan_device *wwandev;
c4804670026b93f M Chetan Kumar 2021-11-20 @171 const char *wwandev_name;
9a44c1cc6388762 Loic Poulain 2021-04-16 172 int err, id;
9a44c1cc6388762 Loic Poulain 2021-04-16 173
9a44c1cc6388762 Loic Poulain 2021-04-16 174 /* The 'find-alloc-register' operation must be protected against
9a44c1cc6388762 Loic Poulain 2021-04-16 175 * concurrent execution, a WWAN device is possibly shared between
9a44c1cc6388762 Loic Poulain 2021-04-16 176 * multiple callers or concurrently unregistered from wwan_remove_dev().
9a44c1cc6388762 Loic Poulain 2021-04-16 177 */
9a44c1cc6388762 Loic Poulain 2021-04-16 178 mutex_lock(&wwan_register_lock);
9a44c1cc6388762 Loic Poulain 2021-04-16 179
9a44c1cc6388762 Loic Poulain 2021-04-16 180 /* If wwandev already exists, return it */
9a44c1cc6388762 Loic Poulain 2021-04-16 181 wwandev = wwan_dev_get_by_parent(parent);
9a44c1cc6388762 Loic Poulain 2021-04-16 182 if (!IS_ERR(wwandev))
9a44c1cc6388762 Loic Poulain 2021-04-16 183 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 184
9a44c1cc6388762 Loic Poulain 2021-04-16 185 id = ida_alloc(&wwan_dev_ids, GFP_KERNEL);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 186 if (id < 0) {
d9d5b8961284b00 Andy Shevchenko 2021-08-11 187 wwandev = ERR_PTR(id);
9a44c1cc6388762 Loic Poulain 2021-04-16 188 goto done_unlock;
d9d5b8961284b00 Andy Shevchenko 2021-08-11 189 }
9a44c1cc6388762 Loic Poulain 2021-04-16 190
9a44c1cc6388762 Loic Poulain 2021-04-16 191 wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
9a44c1cc6388762 Loic Poulain 2021-04-16 192 if (!wwandev) {
d9d5b8961284b00 Andy Shevchenko 2021-08-11 193 wwandev = ERR_PTR(-ENOMEM);
9a44c1cc6388762 Loic Poulain 2021-04-16 194 ida_free(&wwan_dev_ids, id);
9a44c1cc6388762 Loic Poulain 2021-04-16 195 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 196 }
9a44c1cc6388762 Loic Poulain 2021-04-16 197
9a44c1cc6388762 Loic Poulain 2021-04-16 198 wwandev->dev.parent = parent;
9a44c1cc6388762 Loic Poulain 2021-04-16 199 wwandev->dev.class = wwan_class;
9a44c1cc6388762 Loic Poulain 2021-04-16 200 wwandev->dev.type = &wwan_dev_type;
9a44c1cc6388762 Loic Poulain 2021-04-16 201 wwandev->id = id;
9a44c1cc6388762 Loic Poulain 2021-04-16 202 dev_set_name(&wwandev->dev, "wwan%d", wwandev->id);
9a44c1cc6388762 Loic Poulain 2021-04-16 203
9a44c1cc6388762 Loic Poulain 2021-04-16 204 err = device_register(&wwandev->dev);
9a44c1cc6388762 Loic Poulain 2021-04-16 205 if (err) {
9a44c1cc6388762 Loic Poulain 2021-04-16 206 put_device(&wwandev->dev);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 207 wwandev = ERR_PTR(err);
d9d5b8961284b00 Andy Shevchenko 2021-08-11 208 goto done_unlock;
9a44c1cc6388762 Loic Poulain 2021-04-16 209 }
9a44c1cc6388762 Loic Poulain 2021-04-16 210
c4804670026b93f M Chetan Kumar 2021-11-20 211 wwandev_name = kobject_name(&wwandev->dev.kobj);
89654e5e973a53b Sergey Ryazanov 2021-11-28 212 #ifdef CONFIG_WWAN_DEBUGFS
c4804670026b93f M Chetan Kumar 2021-11-20 213 wwandev->debugfs_dir = debugfs_create_dir(wwandev_name,
c4804670026b93f M Chetan Kumar 2021-11-20 214 wwan_debugfs_dir);
89654e5e973a53b Sergey Ryazanov 2021-11-28 215 #endif
c4804670026b93f M Chetan Kumar 2021-11-20 216
9a44c1cc6388762 Loic Poulain 2021-04-16 217 done_unlock:
9a44c1cc6388762 Loic Poulain 2021-04-16 218 mutex_unlock(&wwan_register_lock);
9a44c1cc6388762 Loic Poulain 2021-04-16 219
9a44c1cc6388762 Loic Poulain 2021-04-16 220 return wwandev;
9a44c1cc6388762 Loic Poulain 2021-04-16 221 }
9a44c1cc6388762 Loic Poulain 2021-04-16 222
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
2021-11-28 17:01 ` Johannes Berg
2021-11-28 17:04 ` kernel test robot
@ 2021-11-28 17:05 ` Johannes Berg
2021-11-28 22:57 ` Sergey Ryazanov
2 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2021-11-28 17:05 UTC (permalink / raw)
To: Sergey Ryazanov, David S . Miller, Jakub Kicinski
Cc: netdev, M Chetan Kumar, Intel Corporation, Loic Poulain
On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>
> +#ifdef CONFIG_WWAN_DEBUGFS
> struct dentry *wwan_get_debugfs_dir(struct device *parent);
> +#else
> +static inline struct dentry *wwan_get_debugfs_dir(struct device *parent)
> +{
> + return NULL;
> +}
> +#endif
Now I have to send another email anyway ... but this one probably should
be ERR_PTR(-ENODEV) or something, a la debugfs_create_dir() if debugfs
is disabled, because then a trivial user of wwan's debugfs doesn't even
have to care about whether it's enabled or not, it can just
debugfs_create_dir() for its own and the debugfs core code will check
and return immediately. Yes that's a bit more code space, but if you
just have a debugfs file or two, having an extra Kconfig option is
possibly overkill too. Especially if we get into this path because
DEBUG_FS is disabled *entirely*, and thus all the functions will be
empty inlines (but it might not be, so it should be consistent with
debugfs always returning non-NULL).
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 0/5] WWAN debugfs tweaks
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
` (4 preceding siblings ...)
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
@ 2021-11-28 18:27 ` Leon Romanovsky
2021-11-30 8:03 ` Johannes Berg
5 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2021-11-28 18:27 UTC (permalink / raw)
To: Sergey Ryazanov
Cc: David S . Miller, Jakub Kicinski, netdev, M Chetan Kumar,
Intel Corporation, Loic Poulain, Johannes Berg
On Sun, Nov 28, 2021 at 03:55:17PM +0300, Sergey Ryazanov wrote:
> Resend with proper target tree. Also I should mention that the series is
> mostly compile-tested since I do not have IOSM supported device, so it
> needs Ack from the IOSM developers.
>
> This is a follow-up series to just applied IOSM (and WWAN) debugfs
> interface support [1]. The series has two main goals:
> 1. move the driver-specific debugfs knobs to a subdirectory;
> 2. make the debugfs interface optional for both IOSM and for the WWAN
> core.
>
> As for the first part, I must say that it was my mistake. I suggested to
> place debugfs entries under a common per WWAN device directory. But I
> missed the driver subdirectory in the example, so it become:
>
> /sys/kernel/debugfs/wwan/wwan0/trace
>
> Since the traces collection is a driver-specific feature, it is better
> to keep it under the driver-specific subdirectory:
>
> /sys/kernel/debugfs/wwan/wwan0/iosm/trace
>
> It is desirable to be able to entirely disable the debugfs interface. It
> can be disabled for several reasons, including security and consumed
> storage space.
When such needs arise, the disable is done with CONFIG_DEBUGFS knob and
not with per-subsystem configs.
I personally see your CONFIG_*_DEBUGFS patches as a mistake, which
complicates code without any gain at all. Even an opposite is true,
by adding more knobs, you can find yourself with the system which
has CONFIG_DEBUGFS enabled but with your CONFIG_*_DEBUGFS disabled.
Thanks
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 17:05 ` Johannes Berg
@ 2021-11-28 22:57 ` Sergey Ryazanov
0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 22:57 UTC (permalink / raw)
To: Johannes Berg
Cc: David S . Miller, Jakub Kicinski, netdev, M Chetan Kumar,
Intel Corporation, Loic Poulain
On Sun, Nov 28, 2021 at 8:05 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>> +#ifdef CONFIG_WWAN_DEBUGFS
>> struct dentry *wwan_get_debugfs_dir(struct device *parent);
>> +#else
>> +static inline struct dentry *wwan_get_debugfs_dir(struct device *parent)
>> +{
>> + return NULL;
>> +}
>> +#endif
>
> Now I have to send another email anyway ... but this one probably should
> be ERR_PTR(-ENODEV) or something, a la debugfs_create_dir() if debugfs
> is disabled, because then a trivial user of wwan's debugfs doesn't even
> have to care about whether it's enabled or not, it can just
> debugfs_create_dir() for its own and the debugfs core code will check
> and return immediately. Yes that's a bit more code space, but if you
> just have a debugfs file or two, having an extra Kconfig option is
> possibly overkill too. Especially if we get into this path because
> DEBUG_FS is disabled *entirely*, and thus all the functions will be
> empty inlines (but it might not be, so it should be consistent with
> debugfs always returning non-NULL).
Nice catch, thank you! Will rework in V2 to return ERR_PTR(-ENODEV).
--
Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 17:01 ` Johannes Berg
@ 2021-11-28 23:45 ` Sergey Ryazanov
2021-11-29 6:40 ` Leon Romanovsky
0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-28 23:45 UTC (permalink / raw)
To: Johannes Berg
Cc: David S . Miller, Jakub Kicinski, netdev, M Chetan Kumar,
Intel Corporation, Loic Poulain, Leon Romanovsky
Add Leon to CC to merge both conversations.
On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>>
>> +config WWAN_DEBUGFS
>> + bool "WWAN subsystem common debugfs interface"
>> + depends on DEBUG_FS
>> + help
>> + Enables common debugfs infrastructure for WWAN devices.
>> +
>> + If unsure, say N.
>>
>
> I wonder if that really should even say "If unsure, say N." because
> really, once you have DEBUG_FS enabled, you can expect things to show up
> there?
>
> And I'd probably even argue that it should be
>
> bool "..." if EXPERT
> default y
> depends on DEBUG_FS
>
> so most people aren't even bothered by the question?
>
>
>> config WWAN_HWSIM
>> tristate "Simulated WWAN device"
>> help
>> @@ -83,6 +91,7 @@ config IOSM
>> config IOSM_DEBUGFS
>> bool "IOSM Debugfs support"
>> depends on IOSM && DEBUG_FS
>> + select WWAN_DEBUGFS
>>
> I guess it's kind of a philosophical question, but perhaps it would make
> more sense for that to be "depends on" (and then you can remove &&
> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
> debugfs and not have to worry about individual driver settings?
>
>
> And after that change, I'd probably just make this one "def_bool y"
> instead of asking the user.
When I was preparing this series, my primary considered use case was
embedded firmwares. For example, in OpenWrt, you can not completely
disable debugfs, as a lot of wireless stuff can only be configured and
monitored with the debugfs knobs. At the same time, reducing the size
of a kernel and modules is an essential task in the world of embedded
software. Disabling the WWAN and IOSM debugfs interfaces allows us to
save 50K (x86-64 build) of space for module storage. Not much, but
already considerable when you only have 16MB of storage.
I personally like Johannes' suggestion to enable these symbols by
default to avoid bothering PC users with such negligible things for
them. One thing that makes me doubtful is whether we should hide the
debugfs disabling option under the EXPERT. Or it would be an EXPERT
option misuse, since the debugfs knobs existence themself does not
affect regular WWAN device use.
Leon, would it be Ok with you to add these options to the kernel
configuration and enable them by default?
--
Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-28 23:45 ` Sergey Ryazanov
@ 2021-11-29 6:40 ` Leon Romanovsky
2021-11-29 23:44 ` Sergey Ryazanov
0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2021-11-29 6:40 UTC (permalink / raw)
To: Sergey Ryazanov
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Mon, Nov 29, 2021 at 02:45:16AM +0300, Sergey Ryazanov wrote:
> Add Leon to CC to merge both conversations.
>
> On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
> >>
> >> +config WWAN_DEBUGFS
> >> + bool "WWAN subsystem common debugfs interface"
> >> + depends on DEBUG_FS
> >> + help
> >> + Enables common debugfs infrastructure for WWAN devices.
> >> +
> >> + If unsure, say N.
> >>
> >
> > I wonder if that really should even say "If unsure, say N." because
> > really, once you have DEBUG_FS enabled, you can expect things to show up
> > there?
> >
> > And I'd probably even argue that it should be
> >
> > bool "..." if EXPERT
> > default y
> > depends on DEBUG_FS
> >
> > so most people aren't even bothered by the question?
> >
> >
> >> config WWAN_HWSIM
> >> tristate "Simulated WWAN device"
> >> help
> >> @@ -83,6 +91,7 @@ config IOSM
> >> config IOSM_DEBUGFS
> >> bool "IOSM Debugfs support"
> >> depends on IOSM && DEBUG_FS
> >> + select WWAN_DEBUGFS
> >>
> > I guess it's kind of a philosophical question, but perhaps it would make
> > more sense for that to be "depends on" (and then you can remove &&
> > DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
> > debugfs and not have to worry about individual driver settings?
> >
> >
> > And after that change, I'd probably just make this one "def_bool y"
> > instead of asking the user.
>
> When I was preparing this series, my primary considered use case was
> embedded firmwares. For example, in OpenWrt, you can not completely
> disable debugfs, as a lot of wireless stuff can only be configured and
> monitored with the debugfs knobs. At the same time, reducing the size
> of a kernel and modules is an essential task in the world of embedded
> software. Disabling the WWAN and IOSM debugfs interfaces allows us to
> save 50K (x86-64 build) of space for module storage. Not much, but
> already considerable when you only have 16MB of storage.
>
> I personally like Johannes' suggestion to enable these symbols by
> default to avoid bothering PC users with such negligible things for
> them. One thing that makes me doubtful is whether we should hide the
> debugfs disabling option under the EXPERT. Or it would be an EXPERT
> option misuse, since the debugfs knobs existence themself does not
> affect regular WWAN device use.
>
> Leon, would it be Ok with you to add these options to the kernel
> configuration and enable them by default?
I didn't block your previous proposal either. Just pointed that your
description doesn't correlate with the actual rationale for the patches.
Instead of security claims, just use your OpenWrt case as a base for
the commit message, which is very reasonable and valuable case.
However you should ask yourself if both IOSM_DEBUGFS and WWAN_DEBUGFS
are needed. You wrote that wwan debugfs is empty without ioasm. Isn't
better to allow user to select WWAN_DEBUGFS and change iosm code to
rely on it instead of IOSM_DEBUGFS?
Thanks
>
> --
> Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-29 6:40 ` Leon Romanovsky
@ 2021-11-29 23:44 ` Sergey Ryazanov
2021-11-30 10:05 ` Leon Romanovsky
0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ryazanov @ 2021-11-29 23:44 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Mon, Nov 29, 2021 at 9:40 AM Leon Romanovsky <leon@kernel.org> wrote:
> On Mon, Nov 29, 2021 at 02:45:16AM +0300, Sergey Ryazanov wrote:
>> Add Leon to CC to merge both conversations.
>>
>> On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>>> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>>>>
>>>> +config WWAN_DEBUGFS
>>>> + bool "WWAN subsystem common debugfs interface"
>>>> + depends on DEBUG_FS
>>>> + help
>>>> + Enables common debugfs infrastructure for WWAN devices.
>>>> +
>>>> + If unsure, say N.
>>>>
>>>
>>> I wonder if that really should even say "If unsure, say N." because
>>> really, once you have DEBUG_FS enabled, you can expect things to show up
>>> there?
>>>
>>> And I'd probably even argue that it should be
>>>
>>> bool "..." if EXPERT
>>> default y
>>> depends on DEBUG_FS
>>>
>>> so most people aren't even bothered by the question?
>>>
>>>
>>>> config WWAN_HWSIM
>>>> tristate "Simulated WWAN device"
>>>> help
>>>> @@ -83,6 +91,7 @@ config IOSM
>>>> config IOSM_DEBUGFS
>>>> bool "IOSM Debugfs support"
>>>> depends on IOSM && DEBUG_FS
>>>> + select WWAN_DEBUGFS
>>>>
>>> I guess it's kind of a philosophical question, but perhaps it would make
>>> more sense for that to be "depends on" (and then you can remove &&
>>> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
>>> debugfs and not have to worry about individual driver settings?
>>>
>>>
>>> And after that change, I'd probably just make this one "def_bool y"
>>> instead of asking the user.
>>
>> When I was preparing this series, my primary considered use case was
>> embedded firmwares. For example, in OpenWrt, you can not completely
>> disable debugfs, as a lot of wireless stuff can only be configured and
>> monitored with the debugfs knobs. At the same time, reducing the size
>> of a kernel and modules is an essential task in the world of embedded
>> software. Disabling the WWAN and IOSM debugfs interfaces allows us to
>> save 50K (x86-64 build) of space for module storage. Not much, but
>> already considerable when you only have 16MB of storage.
>>
>> I personally like Johannes' suggestion to enable these symbols by
>> default to avoid bothering PC users with such negligible things for
>> them. One thing that makes me doubtful is whether we should hide the
>> debugfs disabling option under the EXPERT. Or it would be an EXPERT
>> option misuse, since the debugfs knobs existence themself does not
>> affect regular WWAN device use.
>>
>> Leon, would it be Ok with you to add these options to the kernel
>> configuration and enable them by default?
>
> I didn't block your previous proposal either. Just pointed that your
> description doesn't correlate with the actual rationale for the patches.
>
> Instead of security claims, just use your OpenWrt case as a base for
> the commit message, which is very reasonable and valuable case.
Sure. Previous messages were too shallow and unclear. Thanks for
pointing me to this issue. I will improve them based on the feedback
received.
I still think we need separate options for the subsystem and for the
driver (see the rationale below). And I doubt, should I place the
detailed description of the OpenWrt case in each commit message, or it
would be sufficient to place it in a cover letter and add a shorter
version to each commit message. On the one hand, the cover letter
would not show up in the git log. On the other hand, it is not
genteelly to blow up each commit message with the duplicated story.
> However you should ask yourself if both IOSM_DEBUGFS and WWAN_DEBUGFS
> are needed. You wrote that wwan debugfs is empty without ioasm. Isn't
> better to allow user to select WWAN_DEBUGFS and change iosm code to
> rely on it instead of IOSM_DEBUGFS?
Yep, WWAN debugfs interface is useless without driver-specific knobs.
At the moment, only the IOSM driver implements the specific debugfs
interface. But a WWAN modem is a complex device with a lot of
features. For example, see a set of debug and test interfaces
implemented in the proposed driver for the Mediatek T7xx chipset [1].
Without general support from the kernel, all of these debug and test
features will most probably be implemented using the debugfs
interface.
Initially, I also had a plan to implement a single subsystem-wide
option to disable debugfs entirely. But then I considered how many
driver developers would want to create a driver-specific debugfs
interface, and changed my mind in favor of individual options. Just to
avoid an all-or-nothing case.
1. https://lore.kernel.org/all/20211101035635.26999-14-ricardo.martinez@linux.intel.com
--
Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 0/5] WWAN debugfs tweaks
2021-11-28 18:27 ` [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Leon Romanovsky
@ 2021-11-30 8:03 ` Johannes Berg
0 siblings, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2021-11-30 8:03 UTC (permalink / raw)
To: Leon Romanovsky, Sergey Ryazanov
Cc: David S . Miller, Jakub Kicinski, netdev, M Chetan Kumar,
Intel Corporation, Loic Poulain
On Sun, 2021-11-28 at 20:27 +0200, Leon Romanovsky wrote:
>
> I personally see your CONFIG_*_DEBUGFS patches as a mistake, which
> complicates code without any gain at all. Even an opposite is true,
> by adding more knobs, you can find yourself with the system which
> has CONFIG_DEBUGFS enabled but with your CONFIG_*_DEBUGFS disabled.
>
I tend to agree with this - it has already happened to me "in the wild"
that I've had to walk people through a handful of DEBUGFS options to
finally get all the right data ...
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-29 23:44 ` Sergey Ryazanov
@ 2021-11-30 10:05 ` Leon Romanovsky
2021-12-01 22:03 ` Sergey Ryazanov
0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2021-11-30 10:05 UTC (permalink / raw)
To: Sergey Ryazanov
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Tue, Nov 30, 2021 at 02:44:35AM +0300, Sergey Ryazanov wrote:
> On Mon, Nov 29, 2021 at 9:40 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Mon, Nov 29, 2021 at 02:45:16AM +0300, Sergey Ryazanov wrote:
> >> Add Leon to CC to merge both conversations.
> >>
> >> On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> >>> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
> >>>>
> >>>> +config WWAN_DEBUGFS
> >>>> + bool "WWAN subsystem common debugfs interface"
> >>>> + depends on DEBUG_FS
> >>>> + help
> >>>> + Enables common debugfs infrastructure for WWAN devices.
> >>>> +
> >>>> + If unsure, say N.
> >>>>
> >>>
> >>> I wonder if that really should even say "If unsure, say N." because
> >>> really, once you have DEBUG_FS enabled, you can expect things to show up
> >>> there?
> >>>
> >>> And I'd probably even argue that it should be
> >>>
> >>> bool "..." if EXPERT
> >>> default y
> >>> depends on DEBUG_FS
> >>>
> >>> so most people aren't even bothered by the question?
> >>>
> >>>
> >>>> config WWAN_HWSIM
> >>>> tristate "Simulated WWAN device"
> >>>> help
> >>>> @@ -83,6 +91,7 @@ config IOSM
> >>>> config IOSM_DEBUGFS
> >>>> bool "IOSM Debugfs support"
> >>>> depends on IOSM && DEBUG_FS
> >>>> + select WWAN_DEBUGFS
> >>>>
> >>> I guess it's kind of a philosophical question, but perhaps it would make
> >>> more sense for that to be "depends on" (and then you can remove &&
> >>> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
> >>> debugfs and not have to worry about individual driver settings?
> >>>
> >>>
> >>> And after that change, I'd probably just make this one "def_bool y"
> >>> instead of asking the user.
> >>
> >> When I was preparing this series, my primary considered use case was
> >> embedded firmwares. For example, in OpenWrt, you can not completely
> >> disable debugfs, as a lot of wireless stuff can only be configured and
> >> monitored with the debugfs knobs. At the same time, reducing the size
> >> of a kernel and modules is an essential task in the world of embedded
> >> software. Disabling the WWAN and IOSM debugfs interfaces allows us to
> >> save 50K (x86-64 build) of space for module storage. Not much, but
> >> already considerable when you only have 16MB of storage.
> >>
> >> I personally like Johannes' suggestion to enable these symbols by
> >> default to avoid bothering PC users with such negligible things for
> >> them. One thing that makes me doubtful is whether we should hide the
> >> debugfs disabling option under the EXPERT. Or it would be an EXPERT
> >> option misuse, since the debugfs knobs existence themself does not
> >> affect regular WWAN device use.
> >>
> >> Leon, would it be Ok with you to add these options to the kernel
> >> configuration and enable them by default?
> >
> > I didn't block your previous proposal either. Just pointed that your
> > description doesn't correlate with the actual rationale for the patches.
> >
> > Instead of security claims, just use your OpenWrt case as a base for
> > the commit message, which is very reasonable and valuable case.
>
> Sure. Previous messages were too shallow and unclear. Thanks for
> pointing me to this issue. I will improve them based on the feedback
> received.
>
> I still think we need separate options for the subsystem and for the
> driver (see the rationale below). And I doubt, should I place the
> detailed description of the OpenWrt case in each commit message, or it
> would be sufficient to place it in a cover letter and add a shorter
> version to each commit message. On the one hand, the cover letter
> would not show up in the git log. On the other hand, it is not
> genteelly to blow up each commit message with the duplicated story.
I didn't check who is going to apply your patches, but many maintainers
use cover letter as a description for merge commit. I would write about
OpenWrt in the cover letter only.
>
> > However you should ask yourself if both IOSM_DEBUGFS and WWAN_DEBUGFS
> > are needed. You wrote that wwan debugfs is empty without ioasm. Isn't
> > better to allow user to select WWAN_DEBUGFS and change iosm code to
> > rely on it instead of IOSM_DEBUGFS?
>
> Yep, WWAN debugfs interface is useless without driver-specific knobs.
> At the moment, only the IOSM driver implements the specific debugfs
> interface. But a WWAN modem is a complex device with a lot of
> features. For example, see a set of debug and test interfaces
> implemented in the proposed driver for the Mediatek T7xx chipset [1].
> Without general support from the kernel, all of these debug and test
> features will most probably be implemented using the debugfs
> interface.
>
> Initially, I also had a plan to implement a single subsystem-wide
> option to disable debugfs entirely. But then I considered how many
> driver developers would want to create a driver-specific debugfs
> interface, and changed my mind in favor of individual options. Just to
> avoid an all-or-nothing case.
Usually, the answer here is "don't over-engineer". Once such
functionality will be needed, it will be implemented pretty easily.
>
> 1. https://lore.kernel.org/all/20211101035635.26999-14-ricardo.martinez@linux.intel.com
>
> --
> Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-11-30 10:05 ` Leon Romanovsky
@ 2021-12-01 22:03 ` Sergey Ryazanov
2021-12-02 12:03 ` Leon Romanovsky
0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ryazanov @ 2021-12-01 22:03 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Tue, Nov 30, 2021 at 1:05 PM Leon Romanovsky <leon@kernel.org> wrote:
> On Tue, Nov 30, 2021 at 02:44:35AM +0300, Sergey Ryazanov wrote:
>> On Mon, Nov 29, 2021 at 9:40 AM Leon Romanovsky <leon@kernel.org> wrote:
>>> On Mon, Nov 29, 2021 at 02:45:16AM +0300, Sergey Ryazanov wrote:
>>>> Add Leon to CC to merge both conversations.
>>>>
>>>> On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>>>>> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>>>>>> +config WWAN_DEBUGFS
>>>>>> + bool "WWAN subsystem common debugfs interface"
>>>>>> + depends on DEBUG_FS
>>>>>> + help
>>>>>> + Enables common debugfs infrastructure for WWAN devices.
>>>>>> +
>>>>>> + If unsure, say N.
>>>>>
>>>>> I wonder if that really should even say "If unsure, say N." because
>>>>> really, once you have DEBUG_FS enabled, you can expect things to show up
>>>>> there?
>>>>>
>>>>> And I'd probably even argue that it should be
>>>>>
>>>>> bool "..." if EXPERT
>>>>> default y
>>>>> depends on DEBUG_FS
>>>>>
>>>>> so most people aren't even bothered by the question?
>>>>>
>>>>>> config WWAN_HWSIM
>>>>>> tristate "Simulated WWAN device"
>>>>>> help
>>>>>> @@ -83,6 +91,7 @@ config IOSM
>>>>>> config IOSM_DEBUGFS
>>>>>> bool "IOSM Debugfs support"
>>>>>> depends on IOSM && DEBUG_FS
>>>>>> + select WWAN_DEBUGFS
>>>>>>
>>>>> I guess it's kind of a philosophical question, but perhaps it would make
>>>>> more sense for that to be "depends on" (and then you can remove &&
>>>>> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
>>>>> debugfs and not have to worry about individual driver settings?
>>>>>
>>>>>
>>>>> And after that change, I'd probably just make this one "def_bool y"
>>>>> instead of asking the user.
>>>>
>>>> When I was preparing this series, my primary considered use case was
>>>> embedded firmwares. For example, in OpenWrt, you can not completely
>>>> disable debugfs, as a lot of wireless stuff can only be configured and
>>>> monitored with the debugfs knobs. At the same time, reducing the size
>>>> of a kernel and modules is an essential task in the world of embedded
>>>> software. Disabling the WWAN and IOSM debugfs interfaces allows us to
>>>> save 50K (x86-64 build) of space for module storage. Not much, but
>>>> already considerable when you only have 16MB of storage.
>>>>
>>>> I personally like Johannes' suggestion to enable these symbols by
>>>> default to avoid bothering PC users with such negligible things for
>>>> them. One thing that makes me doubtful is whether we should hide the
>>>> debugfs disabling option under the EXPERT. Or it would be an EXPERT
>>>> option misuse, since the debugfs knobs existence themself does not
>>>> affect regular WWAN device use.
>>>>
>>>> Leon, would it be Ok with you to add these options to the kernel
>>>> configuration and enable them by default?
>>>
>>> I didn't block your previous proposal either. Just pointed that your
>>> description doesn't correlate with the actual rationale for the patches.
>>>
>>> Instead of security claims, just use your OpenWrt case as a base for
>>> the commit message, which is very reasonable and valuable case.
>>
>> Sure. Previous messages were too shallow and unclear. Thanks for
>> pointing me to this issue. I will improve them based on the feedback
>> received.
>>
>> I still think we need separate options for the subsystem and for the
>> driver (see the rationale below). And I doubt, should I place the
>> detailed description of the OpenWrt case in each commit message, or it
>> would be sufficient to place it in a cover letter and add a shorter
>> version to each commit message. On the one hand, the cover letter
>> would not show up in the git log. On the other hand, it is not
>> genteelly to blow up each commit message with the duplicated story.
>
> I didn't check who is going to apply your patches, but many maintainers
> use cover letter as a description for merge commit. I would write about
> OpenWrt in the cover letter only.
>
>>> However you should ask yourself if both IOSM_DEBUGFS and WWAN_DEBUGFS
>>> are needed. You wrote that wwan debugfs is empty without ioasm. Isn't
>>> better to allow user to select WWAN_DEBUGFS and change iosm code to
>>> rely on it instead of IOSM_DEBUGFS?
>>
>> Yep, WWAN debugfs interface is useless without driver-specific knobs.
>> At the moment, only the IOSM driver implements the specific debugfs
>> interface. But a WWAN modem is a complex device with a lot of
>> features. For example, see a set of debug and test interfaces
>> implemented in the proposed driver for the Mediatek T7xx chipset [1].
>> Without general support from the kernel, all of these debug and test
>> features will most probably be implemented using the debugfs
>> interface.
>>
>> Initially, I also had a plan to implement a single subsystem-wide
>> option to disable debugfs entirely. But then I considered how many
>> driver developers would want to create a driver-specific debugfs
>> interface, and changed my mind in favor of individual options. Just to
>> avoid an all-or-nothing case.
>
> Usually, the answer here is "don't over-engineer". Once such
> functionality will be needed, it will be implemented pretty easily.
Ironically, I took your "don't over-engineer" argument and started
removing the IOSM specific configuration option when I realized that
the IOSM debugfs interface depends on relayfs and so it should select
the RELAY option. Without the IOSM debugfs option, we should either
place RELAY selection to an option that enables the driver itself, or
to the WWAN subsystem debugfs enabling option. The former will cause
the kernel inflation even with the WWAN debugfs interface disabled.
The latter will simply be misleading. In the end, I decided to keep
both config options in the V2.
--
Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-12-01 22:03 ` Sergey Ryazanov
@ 2021-12-02 12:03 ` Leon Romanovsky
2021-12-02 22:42 ` Sergey Ryazanov
0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2021-12-02 12:03 UTC (permalink / raw)
To: Sergey Ryazanov
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Thu, Dec 02, 2021 at 01:03:33AM +0300, Sergey Ryazanov wrote:
> On Tue, Nov 30, 2021 at 1:05 PM Leon Romanovsky <leon@kernel.org> wrote:
> > On Tue, Nov 30, 2021 at 02:44:35AM +0300, Sergey Ryazanov wrote:
> >> On Mon, Nov 29, 2021 at 9:40 AM Leon Romanovsky <leon@kernel.org> wrote:
> >>> On Mon, Nov 29, 2021 at 02:45:16AM +0300, Sergey Ryazanov wrote:
> >>>> Add Leon to CC to merge both conversations.
> >>>>
> >>>> On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> >>>>> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
> >>>>>> +config WWAN_DEBUGFS
> >>>>>> + bool "WWAN subsystem common debugfs interface"
> >>>>>> + depends on DEBUG_FS
> >>>>>> + help
> >>>>>> + Enables common debugfs infrastructure for WWAN devices.
> >>>>>> +
> >>>>>> + If unsure, say N.
> >>>>>
> >>>>> I wonder if that really should even say "If unsure, say N." because
> >>>>> really, once you have DEBUG_FS enabled, you can expect things to show up
> >>>>> there?
> >>>>>
> >>>>> And I'd probably even argue that it should be
> >>>>>
> >>>>> bool "..." if EXPERT
> >>>>> default y
> >>>>> depends on DEBUG_FS
> >>>>>
> >>>>> so most people aren't even bothered by the question?
> >>>>>
> >>>>>> config WWAN_HWSIM
> >>>>>> tristate "Simulated WWAN device"
> >>>>>> help
> >>>>>> @@ -83,6 +91,7 @@ config IOSM
> >>>>>> config IOSM_DEBUGFS
> >>>>>> bool "IOSM Debugfs support"
> >>>>>> depends on IOSM && DEBUG_FS
> >>>>>> + select WWAN_DEBUGFS
> >>>>>>
> >>>>> I guess it's kind of a philosophical question, but perhaps it would make
> >>>>> more sense for that to be "depends on" (and then you can remove &&
> >>>>> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
> >>>>> debugfs and not have to worry about individual driver settings?
> >>>>>
> >>>>>
> >>>>> And after that change, I'd probably just make this one "def_bool y"
> >>>>> instead of asking the user.
> >>>>
> >>>> When I was preparing this series, my primary considered use case was
> >>>> embedded firmwares. For example, in OpenWrt, you can not completely
> >>>> disable debugfs, as a lot of wireless stuff can only be configured and
> >>>> monitored with the debugfs knobs. At the same time, reducing the size
> >>>> of a kernel and modules is an essential task in the world of embedded
> >>>> software. Disabling the WWAN and IOSM debugfs interfaces allows us to
> >>>> save 50K (x86-64 build) of space for module storage. Not much, but
> >>>> already considerable when you only have 16MB of storage.
> >>>>
> >>>> I personally like Johannes' suggestion to enable these symbols by
> >>>> default to avoid bothering PC users with such negligible things for
> >>>> them. One thing that makes me doubtful is whether we should hide the
> >>>> debugfs disabling option under the EXPERT. Or it would be an EXPERT
> >>>> option misuse, since the debugfs knobs existence themself does not
> >>>> affect regular WWAN device use.
> >>>>
> >>>> Leon, would it be Ok with you to add these options to the kernel
> >>>> configuration and enable them by default?
> >>>
> >>> I didn't block your previous proposal either. Just pointed that your
> >>> description doesn't correlate with the actual rationale for the patches.
> >>>
> >>> Instead of security claims, just use your OpenWrt case as a base for
> >>> the commit message, which is very reasonable and valuable case.
> >>
> >> Sure. Previous messages were too shallow and unclear. Thanks for
> >> pointing me to this issue. I will improve them based on the feedback
> >> received.
> >>
> >> I still think we need separate options for the subsystem and for the
> >> driver (see the rationale below). And I doubt, should I place the
> >> detailed description of the OpenWrt case in each commit message, or it
> >> would be sufficient to place it in a cover letter and add a shorter
> >> version to each commit message. On the one hand, the cover letter
> >> would not show up in the git log. On the other hand, it is not
> >> genteelly to blow up each commit message with the duplicated story.
> >
> > I didn't check who is going to apply your patches, but many maintainers
> > use cover letter as a description for merge commit. I would write about
> > OpenWrt in the cover letter only.
> >
> >>> However you should ask yourself if both IOSM_DEBUGFS and WWAN_DEBUGFS
> >>> are needed. You wrote that wwan debugfs is empty without ioasm. Isn't
> >>> better to allow user to select WWAN_DEBUGFS and change iosm code to
> >>> rely on it instead of IOSM_DEBUGFS?
> >>
> >> Yep, WWAN debugfs interface is useless without driver-specific knobs.
> >> At the moment, only the IOSM driver implements the specific debugfs
> >> interface. But a WWAN modem is a complex device with a lot of
> >> features. For example, see a set of debug and test interfaces
> >> implemented in the proposed driver for the Mediatek T7xx chipset [1].
> >> Without general support from the kernel, all of these debug and test
> >> features will most probably be implemented using the debugfs
> >> interface.
> >>
> >> Initially, I also had a plan to implement a single subsystem-wide
> >> option to disable debugfs entirely. But then I considered how many
> >> driver developers would want to create a driver-specific debugfs
> >> interface, and changed my mind in favor of individual options. Just to
> >> avoid an all-or-nothing case.
> >
> > Usually, the answer here is "don't over-engineer". Once such
> > functionality will be needed, it will be implemented pretty easily.
>
> Ironically, I took your "don't over-engineer" argument and started
> removing the IOSM specific configuration option when I realized that
> the IOSM debugfs interface depends on relayfs and so it should select
> the RELAY option. Without the IOSM debugfs option, we should either
> place RELAY selection to an option that enables the driver itself, or
> to the WWAN subsystem debugfs enabling option. The former will cause
> the kernel inflation even with the WWAN debugfs interface disabled.
> The latter will simply be misleading. In the end, I decided to keep
> both config options in the V2.
Something like this will do the trick.
diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
index bdb2b0e46c12..ebb7292421a1 100644
--- a/drivers/net/wwan/Kconfig
+++ b/drivers/net/wwan/Kconfig
@@ -85,6 +85,7 @@ config IOSM
tristate "IOSM Driver for Intel M.2 WWAN Device"
depends on INTEL_IOMMU
select NET_DEVLINK
+ select RELAY if WWAN_DEBUGFS
help
This driver enables Intel M.2 WWAN Device communication.
>
> --
> Sergey
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
2021-12-02 12:03 ` Leon Romanovsky
@ 2021-12-02 22:42 ` Sergey Ryazanov
0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ryazanov @ 2021-12-02 22:42 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Johannes Berg, David S . Miller, Jakub Kicinski, netdev,
M Chetan Kumar, Intel Corporation, Loic Poulain
On Thu, Dec 2, 2021 at 3:03 PM Leon Romanovsky <leon@kernel.org> wrote:
> On Thu, Dec 02, 2021 at 01:03:33AM +0300, Sergey Ryazanov wrote:
>> Ironically, I took your "don't over-engineer" argument and started
>> removing the IOSM specific configuration option when I realized that
>> the IOSM debugfs interface depends on relayfs and so it should select
>> the RELAY option. Without the IOSM debugfs option, we should either
>> place RELAY selection to an option that enables the driver itself, or
>> to the WWAN subsystem debugfs enabling option. The former will cause
>> the kernel inflation even with the WWAN debugfs interface disabled.
>> The latter will simply be misleading. In the end, I decided to keep
>> both config options in the V2.
>
> Something like this will do the trick.
>
> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> index bdb2b0e46c12..ebb7292421a1 100644
> --- a/drivers/net/wwan/Kconfig
> +++ b/drivers/net/wwan/Kconfig
> @@ -85,6 +85,7 @@ config IOSM
> tristate "IOSM Driver for Intel M.2 WWAN Device"
> depends on INTEL_IOMMU
> select NET_DEVLINK
> + select RELAY if WWAN_DEBUGFS
> help
> This driver enables Intel M.2 WWAN Device communication.
Yes! This is exactly what I need to finally solve this puzzle. Thank you!
--
Sergey
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-12-02 22:42 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 2/5] net: wwan: iosm: allow trace port be uninitialized Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 3/5] net: wwan: iosm: move debugfs knobs into a subdir Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 4/5] net: wwan: iosm: make debugfs optional Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
2021-11-28 17:01 ` Johannes Berg
2021-11-28 23:45 ` Sergey Ryazanov
2021-11-29 6:40 ` Leon Romanovsky
2021-11-29 23:44 ` Sergey Ryazanov
2021-11-30 10:05 ` Leon Romanovsky
2021-12-01 22:03 ` Sergey Ryazanov
2021-12-02 12:03 ` Leon Romanovsky
2021-12-02 22:42 ` Sergey Ryazanov
2021-11-28 17:04 ` kernel test robot
2021-11-28 17:04 ` kernel test robot
2021-11-28 17:05 ` Johannes Berg
2021-11-28 22:57 ` Sergey Ryazanov
2021-11-28 18:27 ` [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Leon Romanovsky
2021-11-30 8:03 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.