* [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects
@ 2019-02-08 16:22 Parav Pandit
2019-02-08 17:30 ` David Ahern
0 siblings, 1 reply; 4+ messages in thread
From: Parav Pandit @ 2019-02-08 16:22 UTC (permalink / raw)
To: netdev, davem; +Cc: parav
Add WARN_ON to make sure that all sub objects of a devlink device are
cleanedup before freeing the devlink device.
This helps to catch any driver bugs.
Signed-off-by: Parav Pandit <parav@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/devlink.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index cd0d393..5e2ef5a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4229,6 +4229,13 @@ void devlink_unregister(struct devlink *devlink)
*/
void devlink_free(struct devlink *devlink)
{
+ WARN_ON(!list_empty(&devlink->port_list));
+ WARN_ON(!list_empty(&devlink->sb_list));
+ WARN_ON(!list_empty(&devlink->dpipe_table_list));
+ WARN_ON(!list_empty(&devlink->resource_list));
+ WARN_ON(!list_empty(&devlink->param_list));
+ WARN_ON(!list_empty(&devlink->region_list));
+
kfree(devlink);
}
EXPORT_SYMBOL_GPL(devlink_free);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects
2019-02-08 16:22 [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects Parav Pandit
@ 2019-02-08 17:30 ` David Ahern
2019-02-08 18:01 ` Parav Pandit
0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2019-02-08 17:30 UTC (permalink / raw)
To: Parav Pandit, netdev, davem
On 2/8/19 8:22 AM, Parav Pandit wrote:
> Add WARN_ON to make sure that all sub objects of a devlink device are
> cleanedup before freeing the devlink device.
> This helps to catch any driver bugs.
>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> net/core/devlink.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index cd0d393..5e2ef5a 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -4229,6 +4229,13 @@ void devlink_unregister(struct devlink *devlink)
> */
> void devlink_free(struct devlink *devlink)
> {
> + WARN_ON(!list_empty(&devlink->port_list));
> + WARN_ON(!list_empty(&devlink->sb_list));
> + WARN_ON(!list_empty(&devlink->dpipe_table_list));
> + WARN_ON(!list_empty(&devlink->resource_list));
> + WARN_ON(!list_empty(&devlink->param_list));
> + WARN_ON(!list_empty(&devlink->region_list));
> +
> kfree(devlink);
> }
> EXPORT_SYMBOL_GPL(devlink_free);
>
reporter_list was just added which brings up the maintenance question:
If you are going to do this you might want a comment in
include/net/devlink.h to remind folks to update this function as relevant.
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects
2019-02-08 17:30 ` David Ahern
@ 2019-02-08 18:01 ` Parav Pandit
2019-02-08 21:09 ` Parav Pandit
0 siblings, 1 reply; 4+ messages in thread
From: Parav Pandit @ 2019-02-08 18:01 UTC (permalink / raw)
To: David Ahern, netdev, davem
> -----Original Message-----
> From: David Ahern <dsahern@gmail.com>
> Sent: Friday, February 8, 2019 11:30 AM
> To: Parav Pandit <parav@mellanox.com>; netdev@vger.kernel.org;
> davem@davemloft.net
> Subject: Re: [PATCH net-next] devlink: Add WARN_ON to catch errors of not
> cleaning devlink objects
>
> On 2/8/19 8:22 AM, Parav Pandit wrote:
> > Add WARN_ON to make sure that all sub objects of a devlink device are
> > cleanedup before freeing the devlink device.
> > This helps to catch any driver bugs.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> > net/core/devlink.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > cd0d393..5e2ef5a 100644
> > --- a/net/core/devlink.c
> > +++ b/net/core/devlink.c
> > @@ -4229,6 +4229,13 @@ void devlink_unregister(struct devlink *devlink)
> > */
> > void devlink_free(struct devlink *devlink) {
> > + WARN_ON(!list_empty(&devlink->port_list));
> > + WARN_ON(!list_empty(&devlink->sb_list));
> > + WARN_ON(!list_empty(&devlink->dpipe_table_list));
> > + WARN_ON(!list_empty(&devlink->resource_list));
> > + WARN_ON(!list_empty(&devlink->param_list));
> > + WARN_ON(!list_empty(&devlink->region_list));
> > +
> > kfree(devlink);
> > }
> > EXPORT_SYMBOL_GPL(devlink_free);
> >
>
> reporter_list was just added which brings up the maintenance question:
> If you are going to do this you might want a comment in
> include/net/devlink.h to remind folks to update this function as relevant.
I see. Make sense. Adding reporter_list and updating devlink.h, too for comment in v1.
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects
2019-02-08 18:01 ` Parav Pandit
@ 2019-02-08 21:09 ` Parav Pandit
0 siblings, 0 replies; 4+ messages in thread
From: Parav Pandit @ 2019-02-08 21:09 UTC (permalink / raw)
To: Parav Pandit, David Ahern, netdev, davem
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Parav Pandit
> Sent: Friday, February 8, 2019 12:01 PM
> To: David Ahern <dsahern@gmail.com>; netdev@vger.kernel.org;
> davem@davemloft.net
> Subject: RE: [PATCH net-next] devlink: Add WARN_ON to catch errors of not
> cleaning devlink objects
>
>
>
> > -----Original Message-----
> > From: David Ahern <dsahern@gmail.com>
> > Sent: Friday, February 8, 2019 11:30 AM
> > To: Parav Pandit <parav@mellanox.com>; netdev@vger.kernel.org;
> > davem@davemloft.net
> > Subject: Re: [PATCH net-next] devlink: Add WARN_ON to catch errors of
> > not cleaning devlink objects
> >
> > On 2/8/19 8:22 AM, Parav Pandit wrote:
> > > Add WARN_ON to make sure that all sub objects of a devlink device
> > > are cleanedup before freeing the devlink device.
> > > This helps to catch any driver bugs.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > > ---
> > > net/core/devlink.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > > cd0d393..5e2ef5a 100644
> > > --- a/net/core/devlink.c
> > > +++ b/net/core/devlink.c
> > > @@ -4229,6 +4229,13 @@ void devlink_unregister(struct devlink
> *devlink)
> > > */
> > > void devlink_free(struct devlink *devlink) {
> > > + WARN_ON(!list_empty(&devlink->port_list));
> > > + WARN_ON(!list_empty(&devlink->sb_list));
> > > + WARN_ON(!list_empty(&devlink->dpipe_table_list));
> > > + WARN_ON(!list_empty(&devlink->resource_list));
> > > + WARN_ON(!list_empty(&devlink->param_list));
> > > + WARN_ON(!list_empty(&devlink->region_list));
> > > +
> > > kfree(devlink);
> > > }
> > > EXPORT_SYMBOL_GPL(devlink_free);
> > >
> >
> > reporter_list was just added which brings up the maintenance question:
> > If you are going to do this you might want a comment in
> > include/net/devlink.h to remind folks to update this function as relevant.
> I see. Make sense. Adding reporter_list and updating devlink.h, too for
> comment in v1.
I think its little too much to add such a generic comment in devlink.h for lists.
My tree was bit old and missed out the reporter_list entry because we first test on internal trees.
I updated my tree to avoid such problem in future.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-08 21:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 16:22 [PATCH net-next] devlink: Add WARN_ON to catch errors of not cleaning devlink objects Parav Pandit
2019-02-08 17:30 ` David Ahern
2019-02-08 18:01 ` Parav Pandit
2019-02-08 21:09 ` Parav Pandit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).