linux-kernel - RE: [PATCH rdma-next 4/8] IB/core: Skip device which doesn't have necessary capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BY5PR12MB4322E477DA2334233EAD5173DC759@BY5PR12MB4322.namprd12.prod.outlook.com>
Date:   Wed, 7 Apr 2021 15:06:35 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jason Gunthorpe <jgg@...dia.com>, Leon Romanovsky <leon@...nel.org>
CC:     Doug Ledford <dledford@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Dennis Dalessandro <dennis.dalessandro@...nelisnetworks.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Karsten Graul <kgraul@...ux.ibm.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        Mike Marciniszyn <mike.marciniszyn@...nelisnetworks.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "rds-devel@....oracle.com" <rds-devel@....oracle.com>,
        Santosh Shilimkar <santosh.shilimkar@...cle.com>
Subject: RE: [PATCH rdma-next 4/8] IB/core: Skip device which doesn't have
 necessary capabilities



> From: Jason Gunthorpe <jgg@...dia.com>
> Sent: Tuesday, April 6, 2021 9:17 PM
> 
> On Mon, Apr 05, 2021 at 08:49:56AM +0300, Leon Romanovsky wrote:
> > @@ -2293,6 +2295,17 @@ static void ib_sa_event(struct ib_event_handler
> *handler,
> >  	}
> >  }
> >
> > +static bool ib_sa_client_supported(struct ib_device *device) {
> > +	unsigned int i;
> > +
> > +	rdma_for_each_port(device, i) {
> > +		if (rdma_cap_ib_sa(device, i))
> > +			return true;
> > +	}
> > +	return false;
> > +}
> 
> This is already done though:
It is but, ib_sa_device() allocates ib_sa_device worth of struct for each port without checking the rdma_cap_ib_sa().
This results into allocating 40 * 512 = 20480 rounded of to power of 2 to 32K bytes of memory for the rdma device with 512 ports.
Other modules are also similarly wasting such memory.

> 
> 	for (i = 0; i <= e - s; ++i) {
> 		spin_lock_init(&sa_dev->port[i].ah_lock);
> 		if (!rdma_cap_ib_sa(device, i + 1))
> 			continue;
> [..]
> 
> 	if (!count) {
> 		ret = -EOPNOTSUPP;
> 		goto free;
> 
> Why does it need to be duplicated? The other patches are all basically like
> that too.
> 
> The add_one function should return -EOPNOTSUPP if it doesn't want to run
> on this device and any supported checks should just be at the front - this is
> how things work right now
> 
I am ok to fold this check at the beginning of add callback.
When 512 to 1K RoCE devices are used, they do not have SA, CM, CMA etc caps on and all the client needs to go through refcnt + xa + sem and unroll them.
Is_supported() routine helps to cut down all of it. I didn't calculate the usec saved with it.

Please let me know.