linux-kernel - Re: [PATCH 1/7] aoe: support more AoE addresses with dynamic block device minor numbers

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] [day] [month] [year] [list]

Message-ID: <EC30E724-B84C-425D-BFF0-90DCC8FB17AB@coraid.com>
Date:	Tue, 2 Oct 2012 19:03:48 -0500
From:	Ed Cashin <ecashin@...aid.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/7] aoe: support more AoE addresses with dynamic block
 device minor numbers

On Oct 2, 2012, at 5:12 PM, Andrew Morton wrote:
...
>> +static int
>> +minor_get(ulong *minor)
>> {
>> -	struct aoedev *d;
>> 	ulong flags;
>> +	ulong n;
>> +	int error = 0;
>> +
>> +	spin_lock_irqsave(&used_minors_lock, flags);
>> +	n = find_first_zero_bit(used_minors, N_DEVS);
>> +	if (n < N_DEVS)
>> +		set_bit(n, used_minors);
>> +	else
>> +		error = -1;
>> +	spin_unlock_irqrestore(&used_minors_lock, flags);
>> +
>> +	*minor = n * AOE_PARTITIONS;
>> +	return error;
>> +}
> 
> - can use the more efficient __set_bit() inside that spinlock.

Thanks for that observation.  Because this operation occurs on target discovery, which is expected to be relatively infrequent, my inclination is to leave it in its atomic form, though, and leave the __set_bit() for another time when optimization is needed.  Like you said, this is a minor point.  I wouldn't mind changing it, though, if you think it's worth me resubmitting the patch.  Just let me know.

> - could avoid setting *minor if we're returning an error.

Yes.  The only caller of aoedev.c:minor_get() handles that correctly.  Again, just let me know if you think this is worth a resubmission of the patch.  Otherwise I'll just make a note to myself to try to avoid setting output parameters on error in the future.

-- 
  Ed Cashin
  ecashin@...aid.com

--
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/