> + for (i = 0; i < buffer->page_count; i++) {
> + buffer->pages[i] = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
> + if (buffer->pages[i] == NULL)
> + goto out_pages;
> +
> + address = dma_map_page(card->device, buffer->pages[i],
> + 0, PAGE_SIZE, direction);
> + if (dma_mapping_error(address)) {
> + __free_page(buffer->pages[i]);
> + goto out_pages;
> + }
Are you sure using streaming dma mapping is safe here? I don't see
actual user in this patch, but doing the proper ownership protocol
for them is quite difficult if you reuse them, and allocating them
in kernelspace usually means you want to keep reusing them.
> +#include <linux/kthread.h>
You don't actually seem to use this one ..
> +#include <asm/uaccess.h>
.. or this one ..
> +#include <asm/semaphore.h>
.. or this one.
> + retval = fw_core_add_address_handler(&topology_map,
> + &topology_map_region);
> + BUG_ON(retval < 0);
> +
> + retval = fw_core_add_address_handler(®isters,
> + ®isters_region);
> + BUG_ON(retval < 0);
> +
> + /* Add the vendor textual descriptor. */
> + retval = fw_core_add_descriptor(&vendor_id_descriptor);
> + BUG_ON(retval < 0);
> + retval = fw_core_add_descriptor(&model_id_descriptor);
> + BUG_ON(retval < 0);
These kinds of bug checks look wrong. Either the operations
can't fail in which case they should not return an error value
or you should handle them properly.
Both the previous and this patch contain quite a lot of GFP_ATOMIC
allocation which are a sign of not having a very good layering.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
