Messages in this thread | | | Date | Tue, 2 Oct 2012 14:12:30 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 1/7] aoe: support more AoE addresses with dynamic block device minor numbers |
| |
On Mon, 1 Oct 2012 18:59:12 -0700 Ed Cashin <ecashin@coraid.com> wrote:
> The ATA over Ethernet protocol uses a major (shelf) and > minor (slot) address to identify a particular storage target. > These changes remove an artificial limitation the aoe driver > imposes on the use of AoE addresses. For example, without these > changes, the slot address has a maximum of 15, but users commonly > use slot numbers much greater than that. > > The AoE shelf and slot address space is often used sparsely. > Instead of using a static mapping between AoE addresses and the > block device minor number, the block device minor numbers are now > allocated on demand. > > ...
Very minor things...
> > ... > > +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.
- could avoid setting *minor if we're returning an error.
> - spin_lock_irqsave(&devlist_lock, flags); > +static void > +minor_free(ulong minor) > +{ > + ulong flags; > > - for (d=devlist; d; d=d->next) > - if (d->aoemajor == maj && d->aoeminor == min) { > - d->ref++; > - break; > - } > + minor /= AOE_PARTITIONS; > + BUG_ON(minor >= N_DEVS); > > - spin_unlock_irqrestore(&devlist_lock, flags); > - return d; > + spin_lock_irqsave(&used_minors_lock, flags); > + BUG_ON(!test_bit(minor, used_minors)); > + clear_bit(minor, used_minors); > + spin_unlock_irqrestore(&used_minors_lock, flags); > }
Could use
BUG_ON(!__test_and_clear_bit(...));
This will work, but I think it's bad form to put an expression with side-effects inside an assert macro.
| |