On Wed, 3 Oct 2012 15:18:43 -0400 Alexandre Bounine <alexandre.boun...@idt.com> wrote:
> Replace the single global destination ID counter with per-net allocation > mechanism to allow independent destID management for each available RapidIO > network. Using bitmap based mechanism instead of counters allows > destination ID release and reuse in systems that support hot-swap. > > > ... > > +static u16 rio_destid_alloc(struct rio_net *net) > +{ > + int destid; > + struct rio_id_table *idtab = &net->destid_table; > + > + spin_lock(&idtab->lock); > + destid = find_next_zero_bit(idtab->table, idtab->max, idtab->next); > + if (destid >= idtab->max) > + destid = find_first_zero_bit(idtab->table, idtab->max); > + > + if (destid < idtab->max) { > + idtab->next = destid + 1; > + if (idtab->next >= idtab->max) > + idtab->next = 0; > + set_bit(destid, idtab->table); > + destid += idtab->start; > + } else > + destid = RIO_INVALID_DESTID; > + > + spin_unlock(&idtab->lock); > + return (u16)destid; > +} This is round-robin rather than the simpler first-fit, and this reader doesn't know why. Suggest the addition of a code comment explaining this decision. > +/* > + * rio_destid_reserve - Reserve the specivied destID > + * net: RIO network > + * destid: destID to reserve > + * > + * Tries to reserve the specified destID. > + * Returns 0 if successfull. > + */ > +static int rio_destid_reserve(struct rio_net *net, u16 destid) > +{ > + int oldbit; > + struct rio_id_table *idtab = &net->destid_table; > + > + destid -= idtab->start; > + spin_lock(&idtab->lock); > + oldbit = test_and_set_bit(destid, idtab->table); > + spin_unlock(&idtab->lock); > + return oldbit; > +} > + > +/* > + * rio_destid_free - free a previously allocated destID > + * net: RIO network > + * destid: destID to free > + * > + * Makes the specified destID available for use. > + */ > +static void rio_destid_free(struct rio_net *net, u16 destid) > +{ > + struct rio_id_table *idtab = &net->destid_table; > + > + destid -= idtab->start; > + spin_lock(&idtab->lock); > + clear_bit(destid, idtab->table); > + spin_unlock(&idtab->lock); > +} > + > +/* > + * rio_destid_first - return first destID in use > + * net: RIO network > + */ > +static u16 rio_destid_first(struct rio_net *net) > +{ > + int destid; > + struct rio_id_table *idtab = &net->destid_table; > + > + spin_lock(&idtab->lock); > + destid = find_first_bit(idtab->table, idtab->max); > + if (destid >= idtab->max) > + destid = RIO_INVALID_DESTID; > + else > + destid += idtab->start; > + spin_unlock(&idtab->lock); > + return (u16)destid; > +} > + > +/* > + * rio_destid_next - return next destID in use > + * net: RIO network > + * from: destination ID from which search shall continue > + */ All these code comments look like kerneldoc, but they aren't. kerneldoc uses /** and identifiers have a leading `@'. And that's OK - one doesn't *have* to use kerneldoc. But a lot of drivers/rapidio/rio-scan.c is already using kerneldoc so the inconsistency is odd. > > ... > > -static struct rio_net __devinit *rio_alloc_net(struct rio_mport *port) > +static struct rio_net __devinit *rio_alloc_net(struct rio_mport *port, > + int do_enum, u16 start) > { > struct rio_net *net; > > net = kzalloc(sizeof(struct rio_net), GFP_KERNEL); > + if (net && do_enum) { > + net->destid_table.table = kzalloc( > + BITS_TO_LONGS(RIO_MAX_ROUTE_ENTRIES(port->sys_size)) * > + sizeof(long), > + GFP_KERNEL); kcalloc() would be idiomatic here. > + if (net->destid_table.table == NULL) { > + pr_err("RIO: failed to allocate destID table\n"); > + kfree(net); > + net = NULL; > + } else { > + net->destid_table.start = start; > + net->destid_table.next = 0; > + net->destid_table.max = > + RIO_MAX_ROUTE_ENTRIES(port->sys_size); > + spin_lock_init(&net->destid_table.lock); > + } > + } > + > > ... > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev