On Wed, 07 Jun 2006 15:06:55 -0500 Steve Wise <[EMAIL PROTECTED]> wrote:
> > +void c2_free(struct c2_alloc *alloc, u32 obj) > +{ > + spin_lock(&alloc->lock); > + clear_bit(obj, alloc->table); > + spin_unlock(&alloc->lock); > +} The spinlock is unneeded here. What does all the code in this file do, anyway? It looks totally generic (and hence inappropriate for drivers/infiniband/hw/amso1100/) and somewhat similar to idr trees, perhaps. > +int c2_array_set(struct c2_array *array, int index, void *value) > +{ > + int p = (index * sizeof(void *)) >> PAGE_SHIFT; > + > + /* Allocate with GFP_ATOMIC because we'll be called with locks held. */ > + if (!array->page_list[p].page) > + array->page_list[p].page = > + (void **) get_zeroed_page(GFP_ATOMIC); > + > + if (!array->page_list[p].page) > + return -ENOMEM; This _will_ happen under load. What will the result of that be, in the context of thise driver? This function is incorrectly designed - it should receive a gfp_t argument. Because you don't *know* that the caller will always hold a spinlock. And GFP_KERNEL is far, far stronger than GFP_ATOMIC. > +static int c2_alloc_mqsp_chunk(gfp_t gfp_mask, struct sp_chunk **head) > +{ > + int i; > + struct sp_chunk *new_head; > + > + new_head = (struct sp_chunk *) __get_free_page(gfp_mask | GFP_DMA); Why is __GFP_DMA in there? Unless you've cornered the ISA bus infiniband market, it's likely to be wrong. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html