linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marciniszyn, Mike" <mike.marciniszyn@intel.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: "dledford@redhat.com" <dledford@redhat.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	"Wan, Kaike" <kaike.wan@intel.com>,
	"Dan Carpenter (dan.carpenter@oracle.com)"
	<dan.carpenter@oracle.com>,
	"Dalessandro, Dennis" <dennis.dalessandro@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: [PATCH for-rc 1/2] IB/hfi1: Restore kfree in dummy_netdev cleanup
Date: Thu, 25 Jun 2020 18:11:36 +0000	[thread overview]
Message-ID: <BY5PR11MB3958E9C1B56B55741BA95B6986920@BY5PR11MB3958.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20200624185342.GM6578@ziepe.ca>

> 
> Gross - make sure your rework allocates netdevs using alloc_netdev.
> 

I don't think that is the correct solution.

All other users of the dummy net device embed it in other structures:

init_dummy_netdev(&mal->dummy_dev);
init_dummy_netdev(&eth->dummy_dev);
init_dummy_netdev(&ar->napi_dev);
init_dummy_netdev(&irq_grp->napi_ndev);
init_dummy_netdev(&wil->napi_ndev);
init_dummy_netdev(&trans_pcie->napi_dev);
init_dummy_netdev(&dev->napi_dev);
init_dummy_netdev(&bus->mux_dev);

There is NO explicit free of the dummy since its lifetime is controlled by its parent struct.

The fault with the current AIP use of the dummy netdev is that it confuses an is-a with a has-a relationship.

This code highlights the confusion by extending the allocation to include the real rx data:

	const int netdev_size = sizeof(*dd->dummy_netdev) +
		sizeof(struct hfi1_netdev_priv);
	<snip>
	dd->dummy_netdev = kcalloc_node(1, netdev_size, GFP_KERNEL, dd->node);

and this kludgy code to find the rx data given a net_device:

static inline
struct hfi1_netdev_priv *hfi1_netdev_priv(struct net_device *dev)
{
        return (struct hfi1_netdev_priv *)&dev[1];
}

The solution is to embed the dummy net_device in a renamed hfi1_netdev_rx struct like other use cases.

The lifetime of that struct IS controlled properly and a kfree() of the renamed struct would include the embedded dummy.

Mike



  reply	other threads:[~2020-06-25 18:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 20:32 [PATCH for-rc 0/2] Fixes for module unload Dennis Dalessandro
2020-06-23 20:32 ` [PATCH for-rc 1/2] IB/hfi1: Restore kfree in dummy_netdev cleanup Dennis Dalessandro
2020-06-24 18:53   ` Jason Gunthorpe
2020-06-25 18:11     ` Marciniszyn, Mike [this message]
2020-06-23 20:32 ` [PATCH for-rc 2/2] IB/hfi1: Fix module use count flaw due to leftover module put calls Dennis Dalessandro
2020-06-24 19:01 ` [PATCH for-rc 0/2] Fixes for module unload Jason Gunthorpe
2020-06-25 11:46   ` Dennis Dalessandro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BY5PR11MB3958E9C1B56B55741BA95B6986920@BY5PR11MB3958.namprd11.prod.outlook.com \
    --to=mike.marciniszyn@intel.com \
    --cc=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=dennis.dalessandro@intel.com \
    --cc=dledford@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=kaike.wan@intel.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='RE: [PATCH for-rc 1/2] IB/hfi1: Restore kfree in dummy_netdev cleanup' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).