LinuxLists.cc - [PATCH 0/8] IB: cleanup debugfs usage

2019-01-22 15:19:35

Subject: [PATCH 0/8] IB: cleanup debugfs usage

When calling debugfs code, there is no need to ever check the return
value of the call, as no logic should ever change if a call works
properly or not. Fix up a bunch of infiniband-specific code to not care
about the results of debugfs.

Greg Kroah-Hartman (8):
infiniband: cxgb4: no need to check return value of debugfs_create functions
infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro
infiniband: hfi1: no need to check return value of debugfs_create functions
infiniband: qib: no need to check return value of debugfs_create functions
infiniband: mlx5: no need to check return value of debugfs_create functions
infiniband: ocrdma: no need to check return value of debugfs_create functions
infiniband: usnic: no need to check return value of debugfs_create functions
infiniband: ipoib: no need to check return value of debugfs_create functions

drivers/infiniband/hw/cxgb4/device.c | 8 +--
drivers/infiniband/hw/hfi1/debugfs.c | 52 +++++++---------
drivers/infiniband/hw/hfi1/debugfs.h | 12 ----
drivers/infiniband/hw/hfi1/fault.c | 53 ++++++----------
drivers/infiniband/hw/mlx5/cong.c | 15 ++---
drivers/infiniband/hw/mlx5/main.c | 8 +--
drivers/infiniband/hw/mlx5/mlx5_ib.h | 8 +--
drivers/infiniband/hw/mlx5/mr.c | 51 +++-------------
drivers/infiniband/hw/ocrdma/ocrdma_stats.c | 67 +++++++--------------
drivers/infiniband/hw/qib/qib_debugfs.c | 26 +++-----
drivers/infiniband/hw/usnic/usnic_debugfs.c | 26 --------
drivers/infiniband/ulp/ipoib/ipoib.h | 4 +-
drivers/infiniband/ulp/ipoib/ipoib_fs.c | 7 +--
drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 +-
14 files changed, 94 insertions(+), 247 deletions(-)

--
2.20.1

2019-01-22 15:19:49

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 2/8] infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro

The macro was just making things harder to follow, and audit, so remove
it and call debugfs_create_file() directly. Also, the macro did not
need to warn about the call failing as no one should ever care about any
debugfs functions failing.

Cc: Mike Marciniszyn <[email protected]>
Cc: Dennis Dalessandro <[email protected]>
Cc: Doug Ledford <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/infiniband/hw/hfi1/debugfs.c | 50 ++++++++++++----------------
drivers/infiniband/hw/hfi1/debugfs.h | 12 -------
drivers/infiniband/hw/hfi1/fault.c | 3 +-
3 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 0a557795563c..52c00c67056f 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -1167,6 +1167,7 @@ void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
char link[10];
struct hfi1_devdata *dd = dd_from_dev(ibd);
struct hfi1_pportdata *ppd;
+ struct dentry *root;
int unit = dd->unit;
int i, j;

@@ -1174,31 +1175,26 @@ void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
return;
snprintf(name, sizeof(name), "%s_%d", class_name(), unit);
snprintf(link, sizeof(link), "%d", unit);
- ibd->hfi1_ibdev_dbg = debugfs_create_dir(name, hfi1_dbg_root);
- if (!ibd->hfi1_ibdev_dbg) {
- pr_warn("create of %s failed\n", name);
- return;
- }
+ root = debugfs_create_dir(name, hfi1_dbg_root);
+ ibd->hfi1_ibdev_dbg = root;
+
ibd->hfi1_ibdev_link =
debugfs_create_symlink(link, hfi1_dbg_root, name);
- if (!ibd->hfi1_ibdev_link) {
- pr_warn("create of %s symlink failed\n", name);
- return;
- }
- DEBUGFS_SEQ_FILE_CREATE(opcode_stats, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(tx_opcode_stats, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(ctx_stats, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(qp_stats, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(sdes, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(rcds, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(pios, ibd->hfi1_ibdev_dbg, ibd);
- DEBUGFS_SEQ_FILE_CREATE(sdma_cpu_list, ibd->hfi1_ibdev_dbg, ibd);
+
+ debugfs_create_file("opcode_stats", 0444, root, ibd, &_opcode_stats_file_ops);
+ debugfs_create_file("tx_opcode_stats", 0444, root, ibd, &_tx_opcode_stats_file_ops);
+ debugfs_create_file("ctx_stats", 0444, root, ibd, &_ctx_stats_file_ops);
+ debugfs_create_file("qp_stats", 0444, root, ibd, &_qp_stats_file_ops);
+ debugfs_create_file("sdes", 0444, root, ibd, &_sdes_file_ops);
+ debugfs_create_file("rcds", 0444, root, ibd, &_rcds_file_ops);
+ debugfs_create_file("pios", 0444, root, ibd, &_pios_file_ops);
+ debugfs_create_file("sdma_cpu_list", 0444, root, ibd, &_sdma_cpu_list_file_ops);
+
/* dev counter files */
for (i = 0; i < ARRAY_SIZE(cntr_ops); i++)
- DEBUGFS_FILE_CREATE(cntr_ops[i].name,
- ibd->hfi1_ibdev_dbg,
- dd,
- &cntr_ops[i].ops, S_IRUGO);
+ debugfs_create_file(cntr_ops[i].name, 0444, root, dd,
+ &cntr_ops[i].ops);
+
/* per port files */
for (ppd = dd->pport, j = 0; j < dd->num_pports; j++, ppd++)
for (i = 0; i < ARRAY_SIZE(port_cntr_ops); i++) {
@@ -1206,12 +1202,10 @@ void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
sizeof(name),
port_cntr_ops[i].name,
j + 1);
- DEBUGFS_FILE_CREATE(name,
- ibd->hfi1_ibdev_dbg,
- ppd,
- &port_cntr_ops[i].ops,
+ debugfs_create_file(name,
!port_cntr_ops[i].ops.write ?
- S_IRUGO : S_IRUGO | S_IWUSR);
+ S_IRUGO : S_IRUGO | S_IWUSR,
+ root, ppd, &port_cntr_ops[i].ops);
}

hfi1_fault_init_debugfs(ibd);
@@ -1343,8 +1337,8 @@ void hfi1_dbg_init(void)
hfi1_dbg_root = debugfs_create_dir(DRIVER_NAME, NULL);
if (!hfi1_dbg_root)
pr_warn("init of debugfs failed\n");
- DEBUGFS_SEQ_FILE_CREATE(driver_stats_names, hfi1_dbg_root, NULL);
- DEBUGFS_SEQ_FILE_CREATE(driver_stats, hfi1_dbg_root, NULL);
+ debugfs_create_file("driver_stats_names", 0444, hfi1_dbg_root, NULL, &_driver_stats_names_file_ops);
+ debugfs_create_file("driver_stats", 0444, hfi1_dbg_root, NULL, &_driver_stats_file_ops);
}

void hfi1_dbg_exit(void)
diff --git a/drivers/infiniband/hw/hfi1/debugfs.h b/drivers/infiniband/hw/hfi1/debugfs.h
index d5d824459fcc..57e582caa5eb 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.h
+++ b/drivers/infiniband/hw/hfi1/debugfs.h
@@ -49,16 +49,6 @@

struct hfi1_ibdev;

-#define DEBUGFS_FILE_CREATE(name, parent, data, ops, mode) \
-do { \
- struct dentry *ent; \
- const char *__name = name; \
- ent = debugfs_create_file(__name, mode, parent, \
- data, ops); \
- if (!ent) \
- pr_warn("create of %s failed\n", __name); \
-} while (0)
-
#define DEBUGFS_SEQ_FILE_OPS(name) \
static const struct seq_operations _##name##_seq_ops = { \
.start = _##name##_seq_start, \
@@ -89,8 +79,6 @@ static const struct file_operations _##name##_file_ops = { \
.release = seq_release \
}

-#define DEBUGFS_SEQ_FILE_CREATE(name, parent, data) \
- DEBUGFS_FILE_CREATE(#name, parent, data, &_##name##_file_ops, 0444)

ssize_t hfi1_seq_read(struct file *file, char __user *buf, size_t size,
loff_t *ppos);
diff --git a/drivers/infiniband/hw/hfi1/fault.c b/drivers/infiniband/hw/hfi1/fault.c
index e2290f32c8d9..dd09b8544568 100644
--- a/drivers/infiniband/hw/hfi1/fault.c
+++ b/drivers/infiniband/hw/hfi1/fault.c
@@ -278,7 +278,8 @@ int hfi1_fault_init_debugfs(struct hfi1_ibdev *ibd)
return -ENOENT;
}

- DEBUGFS_SEQ_FILE_CREATE(fault_stats, ibd->fault->dir, ibd);
+ debugfs_create_file("fault_stats", 0444, ibd->fault->dir, ibd,
+ &_fault_stats_file_ops);
if (!debugfs_create_bool("enable", 0600, ibd->fault->dir,
&ibd->fault->enable))
goto fail;
--
2.20.1

2019-01-22 15:20:10

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 5/8] infiniband: mlx5: no need to check return value of debugfs_create functions

When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.

Cc: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/infiniband/hw/mlx5/cong.c | 15 +++-----
drivers/infiniband/hw/mlx5/main.c | 8 ++---
drivers/infiniband/hw/mlx5/mlx5_ib.h | 8 +----
drivers/infiniband/hw/mlx5/mr.c | 51 +++++-----------------------
4 files changed, 18 insertions(+), 64 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cong.c b/drivers/infiniband/hw/mlx5/cong.c
index 7e4e358a4fd8..8ba439fabf7f 100644
--- a/drivers/infiniband/hw/mlx5/cong.c
+++ b/drivers/infiniband/hw/mlx5/cong.c
@@ -389,19 +389,19 @@ void mlx5_ib_cleanup_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)
dev->port[port_num].dbg_cc_params = NULL;
}

-int mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)
+void mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)
{
struct mlx5_ib_dbg_cc_params *dbg_cc_params;
struct mlx5_core_dev *mdev;
int i;

if (!mlx5_debugfs_root)
- goto out;
+ return;

/* Takes a 1-based port number */
mdev = mlx5_ib_get_native_port_mdev(dev, port_num + 1, NULL);
if (!mdev)
- goto out;
+ return;

if (!MLX5_CAP_GEN(mdev, cc_query_allowed) ||
!MLX5_CAP_GEN(mdev, cc_modify_allowed))
@@ -415,8 +415,6 @@ int mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)

dbg_cc_params->root = debugfs_create_dir("cc_params",
mdev->priv.dbg_root);
- if (!dbg_cc_params->root)
- goto err;

for (i = 0; i < MLX5_IB_DBG_CC_MAX; i++) {
dbg_cc_params->params[i].offset = i;
@@ -427,14 +425,11 @@ int mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)
0600, dbg_cc_params->root,
&dbg_cc_params->params[i],
&dbg_cc_fops);
- if (!dbg_cc_params->params[i].dentry)
- goto err;
}

put_mdev:
mlx5_ib_put_native_port_mdev(dev, port_num + 1);
-out:
- return 0;
+ return;

err:
mlx5_ib_warn(dev, "cong debugfs failure\n");
@@ -445,5 +440,5 @@ int mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num)
* We don't want to fail driver if debugfs failed to initialize,
* so we are not forwarding error to the user.
*/
- return 0;
+ return;
}
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 94fe253d4956..58e9adc94984 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -5508,9 +5508,7 @@ static bool mlx5_ib_bind_slave_port(struct mlx5_ib_dev *ibdev,
mpi->mdev_events.notifier_call = mlx5_ib_event_slave_port;
mlx5_notifier_register(mpi->mdev, &mpi->mdev_events);

- err = mlx5_ib_init_cong_debugfs(ibdev, port_num);
- if (err)
- goto unbind;
+ mlx5_ib_init_cong_debugfs(ibdev, port_num);

return true;

@@ -6183,8 +6181,8 @@ void mlx5_ib_stage_counters_cleanup(struct mlx5_ib_dev *dev)

static int mlx5_ib_stage_cong_debugfs_init(struct mlx5_ib_dev *dev)
{
- return mlx5_ib_init_cong_debugfs(dev,
- mlx5_core_native_port_num(dev->mdev) - 1);
+ mlx5_ib_init_cong_debugfs(dev, mlx5_core_native_port_num(dev->mdev) - 1);
+ return 0;
}

static void mlx5_ib_stage_cong_debugfs_cleanup(struct mlx5_ib_dev *dev)
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index b06d3b1efea8..b75afc4dd5f6 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -623,7 +623,6 @@ struct mlx5_cache_ent {
spinlock_t lock;

- struct dentry *dir;
char name[4];
u32 order;
u32 xlt;
@@ -635,11 +634,6 @@ struct mlx5_cache_ent {
u32 miss;
u32 limit;

- struct dentry *fsize;
- struct dentry *fcur;
- struct dentry *fmiss;
- struct dentry *flimit;
-
struct mlx5_ib_dev *dev;
struct work_struct work;
struct delayed_work dwork;
@@ -1254,7 +1248,7 @@ __be16 mlx5_get_roce_udp_sport(struct mlx5_ib_dev *dev,
const struct ib_gid_attr *attr);

void mlx5_ib_cleanup_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num);
-int mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num);
+void mlx5_ib_init_cong_debugfs(struct mlx5_ib_dev *dev, u8 port_num);

/* GSI QP helper functions */
struct ib_qp *mlx5_ib_gsi_create_qp(struct ib_pd *pd,
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index fd6ea1f75085..7832da4971b0 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -610,52 +610,27 @@ static void mlx5_mr_cache_debugfs_cleanup(struct mlx5_ib_dev *dev)
dev->cache.root = NULL;
}

-static int mlx5_mr_cache_debugfs_init(struct mlx5_ib_dev *dev)
+static void mlx5_mr_cache_debugfs_init(struct mlx5_ib_dev *dev)
{
struct mlx5_mr_cache *cache = &dev->cache;
struct mlx5_cache_ent *ent;
+ struct dentry *dir;
int i;

if (!mlx5_debugfs_root || dev->rep)
- return 0;
+ return;

cache->root = debugfs_create_dir("mr_cache", dev->mdev->priv.dbg_root);
- if (!cache->root)
- return -ENOMEM;

for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++) {
ent = &cache->ent[i];
sprintf(ent->name, "%d", ent->order);
- ent->dir = debugfs_create_dir(ent->name, cache->root);
- if (!ent->dir)
- goto err;
-
- ent->fsize = debugfs_create_file("size", 0600, ent->dir, ent,
- &size_fops);
- if (!ent->fsize)
- goto err;
-
- ent->flimit = debugfs_create_file("limit", 0600, ent->dir, ent,
- &limit_fops);
- if (!ent->flimit)
- goto err;
-
- ent->fcur = debugfs_create_u32("cur", 0400, ent->dir,
- &ent->cur);
- if (!ent->fcur)
- goto err;
-
- ent->fmiss = debugfs_create_u32("miss", 0600, ent->dir,
- &ent->miss);
- if (!ent->fmiss)
- goto err;
+ dir = debugfs_create_dir(ent->name, cache->root);
+ debugfs_create_file("size", 0600, dir, ent, &size_fops);
+ debugfs_create_file("limit", 0600, dir, ent, &limit_fops);
+ debugfs_create_u32("cur", 0400, dir, &ent->cur);
+ debugfs_create_u32("miss", 0600, dir, &ent->miss);
}
-
- return 0;
-err:
- mlx5_mr_cache_debugfs_cleanup(dev);
-
- return -ENOMEM;
}

static void delay_time_func(struct timer_list *t)
@@ -669,7 +644,6 @@ int mlx5_mr_cache_init(struct mlx5_ib_dev *dev)
{
struct mlx5_mr_cache *cache = &dev->cache;
struct mlx5_cache_ent *ent;
- int err;
int i;

mutex_init(&dev->slow_path_mutex);
@@ -713,14 +687,7 @@ int mlx5_mr_cache_init(struct mlx5_ib_dev *dev)
queue_work(cache->wq, &ent->work);
}

- err = mlx5_mr_cache_debugfs_init(dev);
- if (err)
- mlx5_ib_warn(dev, "cache debugfs failure\n");
-
- /*
- * We don't want to fail driver if debugfs failed to initialize,
- * so we are not forwarding error to the user.
- */
+ mlx5_mr_cache_debugfs_init(dev);

return 0;
}
--
2.20.1

2019-01-22 15:20:13

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 6/8] infiniband: ocrdma: no need to check return value of debugfs_create functions

2019-01-22 15:20:15

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 8/8] infiniband: ipoib: no need to check return value of debugfs_create functions

2019-01-22 15:20:19

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions

2019-01-22 15:20:57

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 4/8] infiniband: qib: no need to check return value of debugfs_create functions

When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.

Cc: Mike Marciniszyn <[email protected]>
Cc: Dennis Dalessandro <[email protected]>
Cc: Doug Ledford <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/infiniband/hw/qib/qib_debugfs.c | 26 +++++++------------------
1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_debugfs.c b/drivers/infiniband/hw/qib/qib_debugfs.c
index 5ed1ed93380f..8546e69c6a07 100644
--- a/drivers/infiniband/hw/qib/qib_debugfs.c
+++ b/drivers/infiniband/hw/qib/qib_debugfs.c
@@ -66,15 +66,6 @@ static const struct file_operations _##name##_file_ops = { \
.release = seq_release \
};

-#define DEBUGFS_FILE_CREATE(name) \
-do { \
- struct dentry *ent; \
- ent = debugfs_create_file(#name , 0400, ibd->qib_ibdev_dbg, \
- ibd, &_##name##_file_ops); \
- if (!ent) \
- pr_warn("create of " #name " failed\n"); \
-} while (0)
-
static void *_opcode_stats_seq_start(struct seq_file *s, loff_t *pos)
{
struct qib_opcode_stats_perctx *opstats;
@@ -249,17 +240,16 @@ DEBUGFS_FILE(qp_stats)

void qib_dbg_ibdev_init(struct qib_ibdev *ibd)
{
+ struct dentry *root;
char name[10];

snprintf(name, sizeof(name), "qib%d", dd_from_dev(ibd)->unit);
- ibd->qib_ibdev_dbg = debugfs_create_dir(name, qib_dbg_root);
- if (!ibd->qib_ibdev_dbg) {
- pr_warn("create of %s failed\n", name);
- return;
- }
- DEBUGFS_FILE_CREATE(opcode_stats);
- DEBUGFS_FILE_CREATE(ctx_stats);
- DEBUGFS_FILE_CREATE(qp_stats);
+ root = debugfs_create_dir(name, qib_dbg_root);
+ ibd->qib_ibdev_dbg = root;
+
+ debugfs_create_file("opcode_stats", 0400, root, ibd, &_opcode_stats_file_ops);
+ debugfs_create_file("ctx_stats", 0400, root, ibd, &_ctx_stats_file_ops);
+ debugfs_create_file("qp_stats", 0400, root, ibd, &_qp_stats_file_ops);
}

void qib_dbg_ibdev_exit(struct qib_ibdev *ibd)
@@ -274,8 +264,6 @@ void qib_dbg_ibdev_exit(struct qib_ibdev *ibd)
void qib_dbg_init(void)
{
qib_dbg_root = debugfs_create_dir(QIB_DRV_NAME, NULL);
- if (!qib_dbg_root)
- pr_warn("init of debugfs failed\n");
}

void qib_dbg_exit(void)
--
2.20.1

2019-01-22 15:21:05

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 3/8] infiniband: hfi1: no need to check return value of debugfs_create functions

When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.

Cc: Mike Marciniszyn <[email protected]>
Cc: Dennis Dalessandro <[email protected]>
Cc: Doug Ledford <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/infiniband/hw/hfi1/debugfs.c | 2 --
drivers/infiniband/hw/hfi1/fault.c | 50 ++++++++++------------------
2 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 52c00c67056f..961009e66047 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -1335,8 +1335,6 @@ DEBUGFS_FILE_OPS(driver_stats);
void hfi1_dbg_init(void)
{
hfi1_dbg_root = debugfs_create_dir(DRIVER_NAME, NULL);
- if (!hfi1_dbg_root)
- pr_warn("init of debugfs failed\n");
debugfs_create_file("driver_stats_names", 0444, hfi1_dbg_root, NULL, &_driver_stats_names_file_ops);
debugfs_create_file("driver_stats", 0444, hfi1_dbg_root, NULL, &_driver_stats_file_ops);
}
diff --git a/drivers/infiniband/hw/hfi1/fault.c b/drivers/infiniband/hw/hfi1/fault.c
index dd09b8544568..195fe81e13ea 100644
--- a/drivers/infiniband/hw/hfi1/fault.c
+++ b/drivers/infiniband/hw/hfi1/fault.c
@@ -250,6 +250,7 @@ void hfi1_fault_exit_debugfs(struct hfi1_ibdev *ibd)
int hfi1_fault_init_debugfs(struct hfi1_ibdev *ibd)
{
struct dentry *parent = ibd->hfi1_ibdev_dbg;
+ struct dentry *fault_dir;

ibd->fault = kzalloc(sizeof(*ibd->fault), GFP_KERNEL);
if (!ibd->fault)
@@ -269,46 +270,31 @@ int hfi1_fault_init_debugfs(struct hfi1_ibdev *ibd)
bitmap_zero(ibd->fault->opcodes,
sizeof(ibd->fault->opcodes) * BITS_PER_BYTE);

- ibd->fault->dir =
- fault_create_debugfs_attr("fault", parent,
- &ibd->fault->attr);
- if (IS_ERR(ibd->fault->dir)) {
+ fault_dir = fault_create_debugfs_attr("fault", parent,
+ &ibd->fault->attr);
+ if (IS_ERR(fault_dir)) {
kfree(ibd->fault);
ibd->fault = NULL;
return -ENOENT;
}
+ ibd->fault->dir = fault_dir;

- debugfs_create_file("fault_stats", 0444, ibd->fault->dir, ibd,
+ debugfs_create_file("fault_stats", 0444, fault_dir, ibd,
&_fault_stats_file_ops);
- if (!debugfs_create_bool("enable", 0600, ibd->fault->dir,
- &ibd->fault->enable))
- goto fail;
- if (!debugfs_create_bool("suppress_err", 0600,
- ibd->fault->dir,
- &ibd->fault->suppress_err))
- goto fail;
- if (!debugfs_create_bool("opcode_mode", 0600, ibd->fault->dir,
- &ibd->fault->opcode))
- goto fail;
- if (!debugfs_create_file("opcodes", 0600, ibd->fault->dir,
- ibd->fault, &__fault_opcodes_fops))
- goto fail;
- if (!debugfs_create_u64("skip_pkts", 0600,
- ibd->fault->dir,
- &ibd->fault->fault_skip))
- goto fail;
- if (!debugfs_create_u64("skip_usec", 0600,
- ibd->fault->dir,
- &ibd->fault->fault_skip_usec))
- goto fail;
- if (!debugfs_create_u8("direction", 0600, ibd->fault->dir,
- &ibd->fault->direction))
- goto fail;
+ debugfs_create_bool("enable", 0600, fault_dir, &ibd->fault->enable);
+ debugfs_create_bool("suppress_err", 0600, fault_dir,
+ &ibd->fault->suppress_err);
+ debugfs_create_bool("opcode_mode", 0600, fault_dir,
+ &ibd->fault->opcode);
+ debugfs_create_file("opcodes", 0600, fault_dir, ibd->fault,
+ &__fault_opcodes_fops);
+ debugfs_create_u64("skip_pkts", 0600, fault_dir,
+ &ibd->fault->fault_skip);
+ debugfs_create_u64("skip_usec", 0600, fault_dir,
+ &ibd->fault->fault_skip_usec);
+ debugfs_create_u8("direction", 0600, fault_dir, &ibd->fault->direction);

return 0;
-fail:
- hfi1_fault_exit_debugfs(ibd);
- return -ENOMEM;
}

bool hfi1_dbg_fault_suppress_err(struct hfi1_ibdev *ibd)
--
2.20.1

2019-01-22 15:21:08

by Greg Kroah-Hartman

[permalink] [raw]

Subject: [PATCH 7/8] infiniband: usnic: no need to check return value of debugfs_create functions

When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.

Cc: Christian Benvenuti <[email protected]>
Cc: Nelson Escobar <[email protected]>
Cc: Parvi Kaustubhi <[email protected]>
Cc: Doug Ledford <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/infiniband/hw/usnic/usnic_debugfs.c | 26 ---------------------
1 file changed, 26 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_debugfs.c b/drivers/infiniband/hw/usnic/usnic_debugfs.c
index a3115709fb03..e5a3f02fb078 100644
--- a/drivers/infiniband/hw/usnic/usnic_debugfs.c
+++ b/drivers/infiniband/hw/usnic/usnic_debugfs.c
@@ -113,42 +113,21 @@ static const struct file_operations flowinfo_ops = {
void usnic_debugfs_init(void)
{
debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
- if (IS_ERR(debugfs_root)) {
- usnic_err("Failed to create debugfs root dir, check if debugfs is enabled in kernel configuration\n");
- goto out_clear_root;
- }

flows_dentry = debugfs_create_dir("flows", debugfs_root);
- if (IS_ERR_OR_NULL(flows_dentry)) {
- usnic_err("Failed to create debugfs flow dir with err %ld\n",
- PTR_ERR(flows_dentry));
- goto out_free_root;
- }

debugfs_create_file("build-info", S_IRUGO, debugfs_root,
NULL, &usnic_debugfs_buildinfo_ops);
- return;
-
-out_free_root:
- debugfs_remove_recursive(debugfs_root);
-out_clear_root:
- debugfs_root = NULL;
}

void usnic_debugfs_exit(void)
{
- if (!debugfs_root)
- return;
-
debugfs_remove_recursive(debugfs_root);
debugfs_root = NULL;
}

void usnic_debugfs_flow_add(struct usnic_ib_qp_grp_flow *qp_flow)
{
- if (IS_ERR_OR_NULL(flows_dentry))
- return;
-
scnprintf(qp_flow->dentry_name, sizeof(qp_flow->dentry_name),
"%u", qp_flow->flow->flow_id);
qp_flow->dbgfs_dentry = debugfs_create_file(qp_flow->dentry_name,
@@ -156,11 +135,6 @@ void usnic_debugfs_flow_add(struct usnic_ib_qp_grp_flow *qp_flow)
flows_dentry,
qp_flow,
&flowinfo_ops);
- if (IS_ERR_OR_NULL(qp_flow->dbgfs_dentry)) {
- usnic_err("Failed to create dbg fs entry for flow %u with error %ld\n",
- qp_flow->flow->flow_id,
- PTR_ERR(qp_flow->dbgfs_dentry));
- }
}

void usnic_debugfs_flow_remove(struct usnic_ib_qp_grp_flow *qp_flow)
--
2.20.1

2019-01-22 16:25:32

by Steve Wise

[permalink] [raw]

Subject: Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions

Hey Greg,

On 1/22/2019 9:17 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Steve Wise <[email protected]>
> Cc: Doug Ledford <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/infiniband/hw/cxgb4/device.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> index c13c0ba30f63..9c10fff6dcfb 100644
> --- a/drivers/infiniband/hw/cxgb4/device.c
> +++ b/drivers/infiniband/hw/cxgb4/device.c
> @@ -720,11 +720,8 @@ static const struct file_operations ep_debugfs_fops = {
> .read = debugfs_read,
> };
>
> -static int setup_debugfs(struct c4iw_dev *devp)
> +static void setup_debugfs(struct c4iw_dev *devp)
> {
> - if (!devp->debugfs_root)
> - return -1;
> -
> debugfs_create_file_size("qps", S_IWUSR, devp->debugfs_root,
> (void *)devp, &qp_debugfs_fops, 4096);
>
> @@ -740,7 +737,6 @@ static int setup_debugfs(struct c4iw_dev *devp)
> if (c4iw_wr_log)
> debugfs_create_file_size("wr_log", S_IWUSR, devp->debugfs_root,
> (void *)devp, &wr_log_debugfs_fops, 4096);
> - return 0;
> }
>
> void c4iw_release_dev_ucontext(struct c4iw_rdev *rdev,
> @@ -1553,8 +1549,6 @@ static int __init c4iw_init_module(void)
> return err;
>
> c4iw_debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
> - if (!c4iw_debugfs_root)
> - pr_warn("could not create debugfs entry, continuing\n");
>
> reg_workq = create_singlethread_workqueue("Register_iWARP_device");
> if (!reg_workq) {
>

So it is not a problem to call debugfs_create_file_size() when
devp->debugfs_root is NULL?

Acked-by: Steve Wise <[email protected]>

2019-01-22 16:32:50

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions

On Tue, Jan 22, 2019 at 10:22:59AM -0600, Steve Wise wrote:
> Hey Greg,
>
> On 1/22/2019 9:17 AM, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Cc: Steve Wise <[email protected]>
> > Cc: Doug Ledford <[email protected]>
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
> > drivers/infiniband/hw/cxgb4/device.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> > index c13c0ba30f63..9c10fff6dcfb 100644
> > --- a/drivers/infiniband/hw/cxgb4/device.c
> > +++ b/drivers/infiniband/hw/cxgb4/device.c
> > @@ -720,11 +720,8 @@ static const struct file_operations ep_debugfs_fops = {
> > .read = debugfs_read,
> > };
> >
> > -static int setup_debugfs(struct c4iw_dev *devp)
> > +static void setup_debugfs(struct c4iw_dev *devp)
> > {
> > - if (!devp->debugfs_root)
> > - return -1;
> > -
> > debugfs_create_file_size("qps", S_IWUSR, devp->debugfs_root,
> > (void *)devp, &qp_debugfs_fops, 4096);
> >
> > @@ -740,7 +737,6 @@ static int setup_debugfs(struct c4iw_dev *devp)
> > if (c4iw_wr_log)
> > debugfs_create_file_size("wr_log", S_IWUSR, devp->debugfs_root,
> > (void *)devp, &wr_log_debugfs_fops, 4096);
> > - return 0;
> > }
> >
> > void c4iw_release_dev_ucontext(struct c4iw_rdev *rdev,
> > @@ -1553,8 +1549,6 @@ static int __init c4iw_init_module(void)
> > return err;
> >
> > c4iw_debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
> > - if (!c4iw_debugfs_root)
> > - pr_warn("could not create debugfs entry, continuing\n");
> >
> > reg_workq = create_singlethread_workqueue("Register_iWARP_device");
> > if (!reg_workq) {
> >
>
> So it is not a problem to call debugfs_create_file_size() when
> devp->debugfs_root is NULL?

Nope!

>
> Acked-by: Steve Wise <[email protected]>

Thanks,

greg k-h

2019-01-22 17:45:28

by Leon Romanovsky

[permalink] [raw]

Subject: Re: [PATCH 5/8] infiniband: mlx5: no need to check return value of debugfs_create functions

On Tue, Jan 22, 2019 at 04:17:57PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Leon Romanovsky <[email protected]>
> Cc: Doug Ledford <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/cong.c | 15 +++-----
> drivers/infiniband/hw/mlx5/main.c | 8 ++---
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 8 +----
> drivers/infiniband/hw/mlx5/mr.c | 51 +++++-----------------------
> 4 files changed, 18 insertions(+), 64 deletions(-)
>

Thanks,
Acked-by: Leon Romanovsky <[email protected]>

Attachments:

(No filename) (843.00 B)
signature.asc (817.00 B)
Download all attachments

2019-01-23 00:29:19

by Parvi Kaustubhi (pkaustub)

[permalink] [raw]

Subject: Re: [PATCH 7/8] infiniband: usnic: no need to check return value of debugfs_create functions

usnic driver was tested with this change.

Acked-by: Parvi Kaustubhi <[email protected]>

> On Jan 22, 2019, at 7:17 AM, Greg Kroah-Hartman <[email protected]> wrote:
>
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Christian Benvenuti <[email protected]>
> Cc: Nelson Escobar <[email protected]>
> Cc: Parvi Kaustubhi <[email protected]>
> Cc: Doug Ledford <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/infiniband/hw/usnic/usnic_debugfs.c | 26 ---------------------
> 1 file changed, 26 deletions(-)
>
> diff --git a/drivers/infiniband/hw/usnic/usnic_debugfs.c b/drivers/infiniband/hw/usnic/usnic_debugfs.c
> index a3115709fb03..e5a3f02fb078 100644
> --- a/drivers/infiniband/hw/usnic/usnic_debugfs.c
> +++ b/drivers/infiniband/hw/usnic/usnic_debugfs.c
> @@ -113,42 +113,21 @@ static const struct file_operations flowinfo_ops = {
> void usnic_debugfs_init(void)
> {
> debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
> - if (IS_ERR(debugfs_root)) {
> - usnic_err("Failed to create debugfs root dir, check if debugfs is enabled in kernel configuration\n");
> - goto out_clear_root;
> - }
>
> flows_dentry = debugfs_create_dir("flows", debugfs_root);
> - if (IS_ERR_OR_NULL(flows_dentry)) {
> - usnic_err("Failed to create debugfs flow dir with err %ld\n",
> - PTR_ERR(flows_dentry));
> - goto out_free_root;
> - }
>
> debugfs_create_file("build-info", S_IRUGO, debugfs_root,
> NULL, &usnic_debugfs_buildinfo_ops);
> - return;
> -
> -out_free_root:
> - debugfs_remove_recursive(debugfs_root);
> -out_clear_root:
> - debugfs_root = NULL;
> }
>
> void usnic_debugfs_exit(void)
> {
> - if (!debugfs_root)
> - return;
> -
> debugfs_remove_recursive(debugfs_root);
> debugfs_root = NULL;
> }
>
> void usnic_debugfs_flow_add(struct usnic_ib_qp_grp_flow *qp_flow)
> {
> - if (IS_ERR_OR_NULL(flows_dentry))
> - return;
> -
> scnprintf(qp_flow->dentry_name, sizeof(qp_flow->dentry_name),
> "%u", qp_flow->flow->flow_id);
> qp_flow->dbgfs_dentry = debugfs_create_file(qp_flow->dentry_name,
> @@ -156,11 +135,6 @@ void usnic_debugfs_flow_add(struct usnic_ib_qp_grp_flow *qp_flow)
> flows_dentry,
> qp_flow,
> &flowinfo_ops);
> - if (IS_ERR_OR_NULL(qp_flow->dbgfs_dentry)) {
> - usnic_err("Failed to create dbg fs entry for flow %u with error %ld\n",
> - qp_flow->flow->flow_id,
> - PTR_ERR(qp_flow->dbgfs_dentry));
> - }
> }
>
> void usnic_debugfs_flow_remove(struct usnic_ib_qp_grp_flow *qp_flow)
> --
> 2.20.1
>

2019-01-23 21:18:10

by Jason Gunthorpe

[permalink] [raw]

Subject: Re: [PATCH 0/8] IB: cleanup debugfs usage

On Tue, Jan 22, 2019 at 04:17:52PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs code, there is no need to ever check the return
> value of the call, as no logic should ever change if a call works
> properly or not. Fix up a bunch of infiniband-specific code to not care
> about the results of debugfs.
>
> Greg Kroah-Hartman (8):
> infiniband: cxgb4: no need to check return value of debugfs_create functions
> infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro
> infiniband: hfi1: no need to check return value of debugfs_create functions
> infiniband: qib: no need to check return value of debugfs_create functions
> infiniband: mlx5: no need to check return value of debugfs_create functions
> infiniband: ocrdma: no need to check return value of debugfs_create functions
> infiniband: usnic: no need to check return value of debugfs_create functions
> infiniband: ipoib: no need to check return value of debugfs_create functions

Applied to rdma for-next

Thanks,
Jason