* [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 16:22 ` Steve Wise
2019-01-22 15:17 ` [PATCH 2/8] infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro Greg Kroah-Hartman
` (7 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Steve Wise
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 <swise@chelsio.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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) {
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions
2019-01-22 15:17 ` [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-01-22 16:22 ` Steve Wise
2019-01-22 16:30 ` Greg Kroah-Hartman
0 siblings, 1 reply; 14+ messages in thread
From: Steve Wise @ 2019-01-22 16:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma
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 <swise@chelsio.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: linux-rdma@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 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 <swise@opengridcomputing.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions
2019-01-22 16:22 ` Steve Wise
@ 2019-01-22 16:30 ` Greg Kroah-Hartman
0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 16:30 UTC (permalink / raw)
To: Steve Wise; +Cc: Doug Ledford, Jason Gunthorpe, linux-kernel, linux-rdma
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 <swise@chelsio.com>
> > Cc: Doug Ledford <dledford@redhat.com>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: linux-rdma@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 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 <swise@opengridcomputing.com>
Thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/8] infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 3/8] infiniband: hfi1: no need to check return value of debugfs_create functions Greg Kroah-Hartman
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Mike Marciniszyn,
Dennis Dalessandro
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 <mike.marciniszyn@intel.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/8] infiniband: hfi1: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 2/8] infiniband: hfi1: drop crazy DEBUGFS_SEQ_FILE_CREATE() macro Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 4/8] infiniband: qib: " Greg Kroah-Hartman
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Mike Marciniszyn,
Dennis Dalessandro
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 <mike.marciniszyn@intel.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/8] infiniband: qib: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (2 preceding siblings ...)
2019-01-22 15:17 ` [PATCH 3/8] infiniband: hfi1: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 5/8] infiniband: mlx5: " Greg Kroah-Hartman
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Mike Marciniszyn,
Dennis Dalessandro
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 <mike.marciniszyn@intel.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/8] infiniband: mlx5: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (3 preceding siblings ...)
2019-01-22 15:17 ` [PATCH 4/8] infiniband: qib: " Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 17:42 ` Leon Romanovsky
2019-01-22 15:17 ` [PATCH 6/8] infiniband: ocrdma: " Greg Kroah-Hartman
` (3 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Leon Romanovsky
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 <leon@kernel.org>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/8] infiniband: mlx5: no need to check return value of debugfs_create functions
2019-01-22 15:17 ` [PATCH 5/8] infiniband: mlx5: " Greg Kroah-Hartman
@ 2019-01-22 17:42 ` Leon Romanovsky
0 siblings, 0 replies; 14+ messages in thread
From: Leon Romanovsky @ 2019-01-22 17:42 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Doug Ledford, Jason Gunthorpe, linux-kernel, linux-rdma
[-- Attachment #1: Type: text/plain, Size: 823 bytes --]
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 <leon@kernel.org>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: linux-rdma@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 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 <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/8] infiniband: ocrdma: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (4 preceding siblings ...)
2019-01-22 15:17 ` [PATCH 5/8] infiniband: mlx5: " Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-22 15:17 ` [PATCH 7/8] infiniband: usnic: " Greg Kroah-Hartman
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Selvin Xavier,
Devesh Sharma, Leon Romanovsky, Yuval Shaia
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: Selvin Xavier <selvin.xavier@broadcom.com>
Cc: Devesh Sharma <devesh.sharma@broadcom.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Yuval Shaia <yuval.shaia@oracle.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/infiniband/hw/ocrdma/ocrdma_stats.c | 67 +++++++--------------
1 file changed, 22 insertions(+), 45 deletions(-)
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
index 6be0ea109138..a902942adb5d 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
@@ -767,88 +767,65 @@ void ocrdma_add_port_stats(struct ocrdma_dev *dev)
/* Create post stats base dir */
dev->dir = debugfs_create_dir(pci_name(pdev), ocrdma_dbgfs_dir);
- if (!dev->dir)
- goto err;
dev->rsrc_stats.type = OCRDMA_RSRC_STATS;
dev->rsrc_stats.dev = dev;
- if (!debugfs_create_file("resource_stats", S_IRUSR, dev->dir,
- &dev->rsrc_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("resource_stats", S_IRUSR, dev->dir,
+ &dev->rsrc_stats, &ocrdma_dbg_ops);
dev->rx_stats.type = OCRDMA_RXSTATS;
dev->rx_stats.dev = dev;
- if (!debugfs_create_file("rx_stats", S_IRUSR, dev->dir,
- &dev->rx_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("rx_stats", S_IRUSR, dev->dir, &dev->rx_stats,
+ &ocrdma_dbg_ops);
dev->wqe_stats.type = OCRDMA_WQESTATS;
dev->wqe_stats.dev = dev;
- if (!debugfs_create_file("wqe_stats", S_IRUSR, dev->dir,
- &dev->wqe_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("wqe_stats", S_IRUSR, dev->dir, &dev->wqe_stats,
+ &ocrdma_dbg_ops);
dev->tx_stats.type = OCRDMA_TXSTATS;
dev->tx_stats.dev = dev;
- if (!debugfs_create_file("tx_stats", S_IRUSR, dev->dir,
- &dev->tx_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("tx_stats", S_IRUSR, dev->dir, &dev->tx_stats,
+ &ocrdma_dbg_ops);
dev->db_err_stats.type = OCRDMA_DB_ERRSTATS;
dev->db_err_stats.dev = dev;
- if (!debugfs_create_file("db_err_stats", S_IRUSR, dev->dir,
- &dev->db_err_stats, &ocrdma_dbg_ops))
- goto err;
-
+ debugfs_create_file("db_err_stats", S_IRUSR, dev->dir,
+ &dev->db_err_stats, &ocrdma_dbg_ops);
dev->tx_qp_err_stats.type = OCRDMA_TXQP_ERRSTATS;
dev->tx_qp_err_stats.dev = dev;
- if (!debugfs_create_file("tx_qp_err_stats", S_IRUSR, dev->dir,
- &dev->tx_qp_err_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("tx_qp_err_stats", S_IRUSR, dev->dir,
+ &dev->tx_qp_err_stats, &ocrdma_dbg_ops);
dev->rx_qp_err_stats.type = OCRDMA_RXQP_ERRSTATS;
dev->rx_qp_err_stats.dev = dev;
- if (!debugfs_create_file("rx_qp_err_stats", S_IRUSR, dev->dir,
- &dev->rx_qp_err_stats, &ocrdma_dbg_ops))
- goto err;
-
+ debugfs_create_file("rx_qp_err_stats", S_IRUSR, dev->dir,
+ &dev->rx_qp_err_stats, &ocrdma_dbg_ops);
dev->tx_dbg_stats.type = OCRDMA_TX_DBG_STATS;
dev->tx_dbg_stats.dev = dev;
- if (!debugfs_create_file("tx_dbg_stats", S_IRUSR, dev->dir,
- &dev->tx_dbg_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("tx_dbg_stats", S_IRUSR, dev->dir,
+ &dev->tx_dbg_stats, &ocrdma_dbg_ops);
dev->rx_dbg_stats.type = OCRDMA_RX_DBG_STATS;
dev->rx_dbg_stats.dev = dev;
- if (!debugfs_create_file("rx_dbg_stats", S_IRUSR, dev->dir,
- &dev->rx_dbg_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("rx_dbg_stats", S_IRUSR, dev->dir,
+ &dev->rx_dbg_stats, &ocrdma_dbg_ops);
dev->driver_stats.type = OCRDMA_DRV_STATS;
dev->driver_stats.dev = dev;
- if (!debugfs_create_file("driver_dbg_stats", S_IRUSR, dev->dir,
- &dev->driver_stats, &ocrdma_dbg_ops))
- goto err;
+ debugfs_create_file("driver_dbg_stats", S_IRUSR, dev->dir,
+ &dev->driver_stats, &ocrdma_dbg_ops);
dev->reset_stats.type = OCRDMA_RESET_STATS;
dev->reset_stats.dev = dev;
- if (!debugfs_create_file("reset_stats", 0200, dev->dir,
- &dev->reset_stats, &ocrdma_dbg_ops))
- goto err;
-
-
- return;
-err:
- debugfs_remove_recursive(dev->dir);
- dev->dir = NULL;
+ debugfs_create_file("reset_stats", 0200, dev->dir, &dev->reset_stats,
+ &ocrdma_dbg_ops);
}
void ocrdma_rem_port_stats(struct ocrdma_dev *dev)
{
- if (!dev->dir)
- return;
debugfs_remove_recursive(dev->dir);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 7/8] infiniband: usnic: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (5 preceding siblings ...)
2019-01-22 15:17 ` [PATCH 6/8] infiniband: ocrdma: " Greg Kroah-Hartman
@ 2019-01-22 15:17 ` Greg Kroah-Hartman
2019-01-23 0:16 ` Parvi Kaustubhi (pkaustub)
2019-01-22 15:18 ` [PATCH 8/8] infiniband: ipoib: " Greg Kroah-Hartman
2019-01-23 21:15 ` [PATCH 0/8] IB: cleanup debugfs usage Jason Gunthorpe
8 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:17 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman,
Christian Benvenuti, Nelson Escobar, Parvi Kaustubhi
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 <benve@cisco.com>
Cc: Nelson Escobar <neescoba@cisco.com>
Cc: Parvi Kaustubhi <pkaustub@cisco.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 7/8] infiniband: usnic: no need to check return value of debugfs_create functions
2019-01-22 15:17 ` [PATCH 7/8] infiniband: usnic: " Greg Kroah-Hartman
@ 2019-01-23 0:16 ` Parvi Kaustubhi (pkaustub)
0 siblings, 0 replies; 14+ messages in thread
From: Parvi Kaustubhi (pkaustub) @ 2019-01-23 0:16 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Doug Ledford, Jason Gunthorpe, linux-kernel, linux-rdma,
Christian Benvenuti (benve), Nelson Escobar (neescoba)
usnic driver was tested with this change.
Acked-by: Parvi Kaustubhi <pkaustub@cisco.com>
> On Jan 22, 2019, at 7:17 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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 <benve@cisco.com>
> Cc: Nelson Escobar <neescoba@cisco.com>
> Cc: Parvi Kaustubhi <pkaustub@cisco.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: linux-rdma@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 8/8] infiniband: ipoib: no need to check return value of debugfs_create functions
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (6 preceding siblings ...)
2019-01-22 15:17 ` [PATCH 7/8] infiniband: usnic: " Greg Kroah-Hartman
@ 2019-01-22 15:18 ` Greg Kroah-Hartman
2019-01-23 21:15 ` [PATCH 0/8] IB: cleanup debugfs usage Jason Gunthorpe
8 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-22 15:18 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-kernel, linux-rdma, Greg Kroah-Hartman, Leon Romanovsky,
Kamal Heib, Denis Drozdov, Saeed Mahameed, Erez Shitrit,
Yuval Shaia, Dennis Dalessandro, Alaa Hleihel, Parav Pandit,
Kees Cook, Arseny Maslennikov, Evgenii Smirnov
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: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Kamal Heib <kamalheib1@gmail.com>
Cc: Denis Drozdov <denisd@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Erez Shitrit <erezsh@mellanox.com>
Cc: Yuval Shaia <yuval.shaia@oracle.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Alaa Hleihel <alaa@mellanox.com>
Cc: Parav Pandit <parav@mellanox.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Arseny Maslennikov <ar@cs.msu.ru>
Cc: Evgenii Smirnov <evgenii.smirnov@profitbricks.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 4 ++--
drivers/infiniband/ulp/ipoib/ipoib_fs.c | 7 +------
drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 +---
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 1da119d901a9..5941d660add1 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -781,12 +781,12 @@ static inline void ipoib_cm_handle_tx_wc(struct net_device *dev, struct ib_wc *w
#ifdef CONFIG_INFINIBAND_IPOIB_DEBUG
void ipoib_create_debug_files(struct net_device *dev);
void ipoib_delete_debug_files(struct net_device *dev);
-int ipoib_register_debugfs(void);
+void ipoib_register_debugfs(void);
void ipoib_unregister_debugfs(void);
#else
static inline void ipoib_create_debug_files(struct net_device *dev) { }
static inline void ipoib_delete_debug_files(struct net_device *dev) { }
-static inline int ipoib_register_debugfs(void) { return 0; }
+static inline void ipoib_register_debugfs(void) { }
static inline void ipoib_unregister_debugfs(void) { }
#endif
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_fs.c b/drivers/infiniband/ulp/ipoib/ipoib_fs.c
index 178488028734..64c19f6fa931 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_fs.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_fs.c
@@ -267,14 +267,10 @@ void ipoib_create_debug_files(struct net_device *dev)
snprintf(name, sizeof(name), "%s_mcg", dev->name);
priv->mcg_dentry = debugfs_create_file(name, S_IFREG | S_IRUGO,
ipoib_root, dev, &ipoib_mcg_fops);
- if (!priv->mcg_dentry)
- ipoib_warn(priv, "failed to create mcg debug file\n");
snprintf(name, sizeof(name), "%s_path", dev->name);
priv->path_dentry = debugfs_create_file(name, S_IFREG | S_IRUGO,
ipoib_root, dev, &ipoib_path_fops);
- if (!priv->path_dentry)
- ipoib_warn(priv, "failed to create path debug file\n");
}
void ipoib_delete_debug_files(struct net_device *dev)
@@ -286,10 +282,9 @@ void ipoib_delete_debug_files(struct net_device *dev)
priv->mcg_dentry = priv->path_dentry = NULL;
}
-int ipoib_register_debugfs(void)
+void ipoib_register_debugfs(void)
{
ipoib_root = debugfs_create_dir("ipoib", NULL);
- return ipoib_root ? 0 : -ENOMEM;
}
void ipoib_unregister_debugfs(void)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index d932f99201d1..45ef3b0f03c5 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -2577,9 +2577,7 @@ static int __init ipoib_init_module(void)
*/
BUILD_BUG_ON(IPOIB_CM_COPYBREAK > IPOIB_CM_HEAD_SIZE);
- ret = ipoib_register_debugfs();
- if (ret)
- return ret;
+ ipoib_register_debugfs();
/*
* We create a global workqueue here that is used for all flush
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 0/8] IB: cleanup debugfs usage
2019-01-22 15:17 [PATCH 0/8] IB: cleanup debugfs usage Greg Kroah-Hartman
` (7 preceding siblings ...)
2019-01-22 15:18 ` [PATCH 8/8] infiniband: ipoib: " Greg Kroah-Hartman
@ 2019-01-23 21:15 ` Jason Gunthorpe
8 siblings, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2019-01-23 21:15 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Doug Ledford, linux-kernel, linux-rdma
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
^ permalink raw reply [flat|nested] 14+ messages in thread