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
ecas...@coraid.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
