| Message ID | 20190806161128.31232-9-gregkh@linuxfoundation.org |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Networking driver debugfs cleanups | expand |
On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Jesper Dangaard Brouer <hawk@kernel.org> > Cc: John Fastabend <john.fastabend@gmail.com> > Cc: Edwin Peer <edwin.peer@netronome.com> > Cc: Yangtao Li <tiny.windzz@gmail.com> > Cc: Simon Horman <simon.horman@netronome.com> > Cc: oss-drivers@netronome.com > Cc: netdev@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> I take it this is the case since commit ff9fb72bc077 ("debugfs: return error values, not NULL")? I.e. v5.0? It'd be useful to know for backport purposes.
On Tue, Aug 06, 2019 at 09:50:08AM -0700, Jakub Kicinski wrote: > On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > Cc: Jesper Dangaard Brouer <hawk@kernel.org> > > Cc: John Fastabend <john.fastabend@gmail.com> > > Cc: Edwin Peer <edwin.peer@netronome.com> > > Cc: Yangtao Li <tiny.windzz@gmail.com> > > Cc: Simon Horman <simon.horman@netronome.com> > > Cc: oss-drivers@netronome.com > > Cc: netdev@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > I take it this is the case since commit ff9fb72bc077 ("debugfs: return > error values, not NULL")? I.e. v5.0? It'd be useful to know for backport > purposes. You were always safe to ignore debugfs calls before that, but in 5.0 and then 5.2 we got a bit more "robust" with some internal debugfs logic to make it even easier. These can be backported to 2.6.11+ if you really want to, no functionality should change. But why would you want to backport them? This really isn't a "bugfix" for a stable kernel. No one should ever noticed the difference except for less memory being used. thanks, greg k-h
On Tue, 6 Aug 2019 19:00:49 +0200, Greg Kroah-Hartman wrote: > On Tue, Aug 06, 2019 at 09:50:08AM -0700, Jakub Kicinski wrote: > > On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com> > > > Cc: "David S. Miller" <davem@davemloft.net> > > > Cc: Alexei Starovoitov <ast@kernel.org> > > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > > Cc: Jesper Dangaard Brouer <hawk@kernel.org> > > > Cc: John Fastabend <john.fastabend@gmail.com> > > > Cc: Edwin Peer <edwin.peer@netronome.com> > > > Cc: Yangtao Li <tiny.windzz@gmail.com> > > > Cc: Simon Horman <simon.horman@netronome.com> > > > Cc: oss-drivers@netronome.com > > > Cc: netdev@vger.kernel.org > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > > > I take it this is the case since commit ff9fb72bc077 ("debugfs: return > > error values, not NULL")? I.e. v5.0? It'd be useful to know for backport > > purposes. > > You were always safe to ignore debugfs calls before that, but in 5.0 and > then 5.2 we got a bit more "robust" with some internal debugfs logic to > make it even easier. These can be backported to 2.6.11+ if you really > want to, no functionality should change. Oh sorry! I meant vendor out-of-tree driver backport. We all maintain a tarball version of the drivers that compile on old kernels, I was mostly wondering from that perspective. > But why would you want to backport them? This really isn't a "bugfix" > for a stable kernel. No one should ever noticed the difference except > for less memory being used. Right, it wouldn't really help to do an upstream backport.
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c index ab7f2498e1c4..553c708694e8 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c @@ -159,19 +159,13 @@ void nfp_net_debugfs_vnic_add(struct nfp_net *nn, struct dentry *ddir) else strcpy(name, "ctrl-vnic"); nn->debugfs_dir = debugfs_create_dir(name, ddir); - if (IS_ERR_OR_NULL(nn->debugfs_dir)) - return; /* Create queue debugging sub-tree */ queues = debugfs_create_dir("queue", nn->debugfs_dir); - if (IS_ERR_OR_NULL(queues)) - return; rx = debugfs_create_dir("rx", queues); tx = debugfs_create_dir("tx", queues); xdp = debugfs_create_dir("xdp", queues); - if (IS_ERR_OR_NULL(rx) || IS_ERR_OR_NULL(tx) || IS_ERR_OR_NULL(xdp)) - return; for (i = 0; i < min(nn->max_rx_rings, nn->max_r_vecs); i++) { sprintf(name, "%d", i); @@ -190,16 +184,7 @@ void nfp_net_debugfs_vnic_add(struct nfp_net *nn, struct dentry *ddir) struct dentry *nfp_net_debugfs_device_add(struct pci_dev *pdev) { - struct dentry *dev_dir; - - if (IS_ERR_OR_NULL(nfp_dir)) - return NULL; - - dev_dir = debugfs_create_dir(pci_name(pdev), nfp_dir); - if (IS_ERR_OR_NULL(dev_dir)) - return NULL; - - return dev_dir; + return debugfs_create_dir(pci_name(pdev), nfp_dir); } void nfp_net_debugfs_dir_clean(struct dentry **dir)
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: Jakub Kicinski <jakub.kicinski@netronome.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Jesper Dangaard Brouer <hawk@kernel.org> Cc: John Fastabend <john.fastabend@gmail.com> Cc: Edwin Peer <edwin.peer@netronome.com> Cc: Yangtao Li <tiny.windzz@gmail.com> Cc: Simon Horman <simon.horman@netronome.com> Cc: oss-drivers@netronome.com Cc: netdev@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- .../ethernet/netronome/nfp/nfp_net_debugfs.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-)