From:   Jason Gunthorpe <jgg@mellanox.com>
To:     Leon Romanovsky <leon@kernel.org>
CC:     Doug Ledford <dledford@redhat.com>,
        Leon Romanovsky <leonro@mellanox.com>,
        RDMA mailing list <linux-rdma@vger.kernel.org>,
        Daniel Jurgens <danielj@mellanox.com>,
        Danit Goldberg <danitg@mellanox.com>,
        Yishai Hadas <yishaih@mellanox.com>
Subject: Re: [PATCH 2/2] IB/mlx5: Free mpi in mp_slave mode
Thread-Topic: [PATCH 2/2] IB/mlx5: Free mpi in mp_slave mode
Thread-Index: AQHVbFrC1fO1CpNws0Wyj0deQHR6fqcujlCA
Date:   Mon, 16 Sep 2019 17:23:59 +0000
Message-ID: <20190916172353.GD2585@mellanox.com>
References: <20190916064818.19823-1-leon@kernel.org>
 <20190916064818.19823-3-leon@kernel.org>
In-Reply-To: <20190916064818.19823-3-leon@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [142.167.223.10]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 04919a0c-69b4-4a2b-57f2-08d73acaa8ae
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(5600167)(711020)(4605104)(1401327)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020);SRVR:VI1PR05MB4670;
x-ms-traffictypediagnostic: VI1PR05MB4670:|VI1PR05MB4670:
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <VI1PR05MB46701FDC84CE2B21D9FBE61CCF8C0@VI1PR05MB4670.eurprd05.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:6790;
x-forefront-prvs: 0162ACCC24
x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(4636009)(136003)(39860400002)(396003)(376002)(346002)(366004)(199004)(189003)(66446008)(64756008)(66556008)(229853002)(26005)(76176011)(386003)(6916009)(446003)(3846002)(71200400001)(6506007)(476003)(486006)(99286004)(71190400001)(33656002)(7736002)(305945005)(186003)(6116002)(53936002)(107886003)(6246003)(2616005)(1076003)(11346002)(4326008)(316002)(5660300002)(54906003)(8676002)(81156014)(81166006)(66476007)(8936002)(6512007)(66066001)(36756003)(102836004)(256004)(14454004)(86362001)(52116002)(66946007)(6486002)(478600001)(2906002)(6436002)(25786009);DIR:OUT;SFP:1101;SCL:1;SRVR:VI1PR05MB4670;H:VI1PR05MB4141.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1;
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: 1yUSouerJtamlvARYCmb+xtrdlr/w8LtgqhOKyyT3r0JCbGTnMhkUQwKLqAXCO0trQj6VceYRCMkWl/qfE17EokZTXygyBWplSFbE1YWX2WbHfKlgAJPAk90I8eV75VYtsIDuUyqTvnekiXJgpnxyRaxSzIZfLNC3t7iG+bx+Mnw208LDPFsVXhH2dRqQhQPVSzZwmHyyThcW3nodrpIl77tu+Zq/R3in/kRDJ9KpwuI4FtJGBKQOV6sDmDvLcC3bushfD9yJaOaB8oV20QAfMDwgkcU/7DUMrVe3qcgTb2gYvisM4s0OnS08AiU7RnUeeO62CacXvtJNbtm+1G7mHma5lj6MaoPIfb+ioDAkycrOlfr7d9oJUEJvFd7aZsgsJQZWAdiTv9lm+rLBz/p5/oO5ia4O4Oh5b43YkDCmUk=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <01A991971721F64F96A13574181DB306@eurprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 04919a0c-69b4-4a2b-57f2-08d73acaa8ae
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Sep 2019 17:23:59.2042
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: dpIXDvZKUKlhoT9+Y8vr+3V22JLpMyCEkftQSU53TGVF+ccsO6ZYXe3/WpZtitd0o8IcoriHCD70pl8sFr91uw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB4670
Sender: linux-rdma-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-rdma.vger.kernel.org>
X-Mailing-List: linux-rdma@vger.kernel.org

On Mon, Sep 16, 2019 at 09:48:18AM +0300, Leon Romanovsky wrote:
> From: Danit Goldberg <danitg@mellanox.com>
>=20
> ib_add_slave_port() allocates a multiport struct but never frees it.
> Don't leak memory, free the allocated mpi struct during driver unload.
>=20
> Fixes: 32f69e4be269 ("{net, IB}/mlx5: Manage port association for multipo=
rt RoCE")
> Signed-off-by: Danit Goldberg <danitg@mellanox.com>
>  drivers/infiniband/hw/mlx5/main.c | 1 +
>  1 file changed, 1 insertion(+)
>=20
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/ml=
x5/main.c
> index 0569bcab02d4..14807ea8dc3f 100644
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6959,6 +6959,7 @@ static void mlx5_ib_remove(struct mlx5_core_dev *md=
ev, void *context)
>  			mlx5_ib_unbind_slave_port(mpi->ibdev, mpi);
>  		list_del(&mpi->list);
>  		mutex_unlock(&mlx5_ib_multiport_mutex);
> +		kfree(mpi);
>  		return;
>  	}

Personally I think the way this code was written to try to share the
struct mlx5_interface between two completely different usages, with
totally different opaque structs is really obtuse.=20

Two interfaces callback blocks for the two parallel uses would have
been cleaner and might have made this missing kfree clearer.

Jason