Messages in this thread | | | Date | Tue, 14 Apr 2009 12:58:23 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] dma-debug: add dma_debug_resize_entries() to adjust the number of dma_debug_entries |
| |
* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> +int dma_debug_resize_entries(u32 num_entries) > +{ > + int i, delta, ret = 0; > + unsigned long flags; > + struct dma_debug_entry *entry; > + > + spin_lock_irqsave(&free_entries_lock, flags); > + > + if (nr_total_entries < num_entries) { > + delta = num_entries - nr_total_entries; > + > + for (i = 0; i < delta; i++) { > + entry = kzalloc(sizeof(*entry), GFP_ATOMIC); > + if (!entry) > + break;
hm, using GFP_ATOMIC within a spinlock is not a very nice thing to do in general. While this is boot-only and the GFP_ATOMIC will likely succeed, this could become non-boot functionality and then it's exposed to the momentary VM pressure situation that might make GFP_ATOMIC fail.
Please fix this to be GFP_KERNEL.
a small detail:
> + delta = nr_total_entries - num_entries; > + > + for (i = 0; i < delta && !list_empty(&free_entries); i++) { > + entry = __dma_entry_alloc(); > + kfree(entry); > + } > + > + nr_total_entries -= i; > + }
Can i != delta ever happen? It would suggest the list length being out of sync with the nr_total_entries counter.
Ingo
| |