On Tue, Jul 09, 2019 at 09:38:42AM +0300, Leon Romanovsky wrote:
> On Mon, Jul 08, 2019 at 05:20:23PM -0300, Jason Gunthorpe wrote:
> > On Thu, Jul 04, 2019 at 04:04:02PM +0300, Leon Romanovsky wrote:
> > > -int rdma_nl_unicast(struct sk_buff *skb, u32 pid)
> > > +int rdma_nl_unicast(struct net *net, struct sk_buff *skb, u32 pid)
> > > {
> > > + struct rdma_dev_net *rnet = net_generic(net, rdma_dev_net_id);
> >
> > This should be a proper type safe accessor in all places
>
> "const struct net *net" and not "struct net *net"?
No,
static inline struct rdma_dev_net *rdma_get_net(struct net *net);
> >
> > > -void rdma_nl_exit(void)
> > > +void rdma_nl_net_exit(struct rdma_dev_net *rnet)
> > > {
> > > - int idx;
> > > -
> > > - for (idx = 0; idx < RDMA_NL_NUM_CLIENTS; idx++)
> > > - rdma_nl_unregister(idx);
> >
> > There should be a WARN_ON during the module unload that no NL clients
> > are still registered
>
> IMHO, the usage of WARN_ON is overrated.
If there is to be any code at all I want to see WARN_ON's for things
that can't happen. Not pr_debug, not nonsense loops like the above.
Jason
