linux-kernel - Re: [PATCH 25/62] infiniband/cxgb4: convert to idr

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: <510E71BD.1010909@opengridcomputing.com>
Date:	Sun, 03 Feb 2013 08:18:37 -0600
From:	Steve Wise <swise@...ngridcomputing.com>
To:	Tejun Heo <tj@...nel.org>
CC:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	rusty@...tcorp.com.au, bfields@...ldses.org,
	skinsbursky@...allels.com, ebiederm@...ssion.com,
	jmorris@...ei.org, axboe@...nel.dk, Steve Wise <swise@...lsio.com>,
	linux-rdma@...r.kernel.org
Subject: Re: [PATCH 25/62] infiniband/cxgb4: convert to idr_alloc()

On 2/2/2013 7:20 PM, Tejun Heo wrote:
> Convert to the much saner new idr interface.
>
> Only compile tested.
>
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Cc: Steve Wise <swise@...lsio.com>
> Cc: linux-rdma@...r.kernel.org
> ---
> This patch depends on an earlier idr changes and I think it would be
> best to route these together through -mm.  Please holler if there's
> any objection.  Thanks.

Is there a git tree somewhere that I can use to test these patches out?

>   drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 27 ++++++++++++++-------------
>   1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> index 9c1644f..7f862da 100644
> --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> @@ -260,20 +260,21 @@ static inline int _insert_handle(struct c4iw_dev *rhp, struct idr *idr,
>   				 void *handle, u32 id, int lock)
>   {
>   	int ret;
> -	int newid;
>   
> -	do {
> -		if (!idr_pre_get(idr, lock ? GFP_KERNEL : GFP_ATOMIC))
> -			return -ENOMEM;
> -		if (lock)
> -			spin_lock_irq(&rhp->lock);
> -		ret = idr_get_new_above(idr, handle, id, &newid);
> -		BUG_ON(!ret && newid != id);
> -		if (lock)
> -			spin_unlock_irq(&rhp->lock);
> -	} while (ret == -EAGAIN);
> -
> -	return ret;
> +	if (lock) {
> +		idr_preload(GFP_KERNEL);
> +		spin_lock_irq(&rhp->lock);
> +	}

The idr_preload() needs to be above the 'if (lock)', no?

> +
> +	ret = idr_alloc(idr, handle, id, id + 1, GFP_ATOMIC);
> +
> +	if (lock) {
> +		spin_unlock_irq(&rhp->lock);
> +		idr_preload_end();
> +	}

And idr_preload_end() should be after the 'if (lock)' block methinks...

> +
> +	BUG_ON(ret == -ENOSPC);
> +	return ret < 0 ? ret : 0;

What would cause ret >  0?

>   }
>   
>   static inline int insert_handle(struct c4iw_dev *rhp, struct idr *idr,

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/