Messages in this thread | | | From | Ed Cashin <> | Date | Tue, 2 Oct 2012 19:03:48 -0500 | 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@coraid.com
| |