On Thu, 2015-11-26 at 01:19 +0100, Steinar H. Gunderson wrote:
Hi,
just two small nits to pick:
> +static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct usb_memory *usbm = NULL;
> + struct usb_dev_state *ps = file->private_data;
> + size_t size = vma->vm_end - vma->vm_start;
> + void *mem;
> + unsigned long flags;
> + dma_addr_t dma_handle;
> + int ret;
> +
> + ret = try_module_get(THIS_MODULE);
> + if (ret)
> + return ret;
> +
> + ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
> + if (ret) {
> + module_put(THIS_MODULE);
> + return ret;
> + }
Could you do the error handling in a unified manner with "goto"?
> +
> + usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
> + if (!usbm) {
> + module_put(THIS_MODULE);
> + usbfs_decrease_memory_usage(
> + size + sizeof(struct usb_memory));
> + return -ENOMEM;
> + }
> +
> + mem = usb_alloc_coherent(ps->dev, size, GFP_KERNEL, &dma_handle);
Shouldn't this be GFP_USER to let limits apply?
> + if (!mem) {
> + module_put(THIS_MODULE);
> + usbfs_decrease_memory_usage(
> + size + sizeof(struct usb_memory));
> + kfree(usbm);
> + return -ENOMEM;
> + }
> +
> + memset(mem, 0, size);
> +
> + usbm->mem = mem;
> + usbm->dma_handle = dma_handle;
> + usbm->size = size;
> + usbm->ps = ps;
> + usbm->vm_start = vma->vm_start;
> + usbm->vma_use_count = 1;
> + INIT_LIST_HEAD(&usbm->memlist);
> +
> + if (remap_pfn_range(vma, vma->vm_start,
> + virt_to_phys(usbm->mem) >> PAGE_SHIFT,
> + size, vma->vm_page_prot) < 0) {
> + dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
> + return -EAGAIN;
> + }
> +
> + vma->vm_flags |= VM_IO;
> + vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
> + vma->vm_ops = &usbdev_vm_ops;
> + vma->vm_private_data = usbm;
> +
> + spin_lock_irqsave(&ps->lock, flags);
> + list_add_tail(&usbm->memlist, &ps->memory_list);
> + spin_unlock_irqrestore(&ps->lock, flags);
> +
> + return 0;
> +}
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
