Hi Prabhakar-san,
Thank you for the patch!
> From: Lad Prabhakar, Sent: Sunday, April 19, 2020 10:27 PM
<snip>
> @@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> */
> void pci_epc_mem_exit(struct pci_epc *epc)
> {
> - struct pci_epc_mem *mem = epc->mem;
> + struct pci_epc_mem *mem;
> + int i;
>
> + if (!epc->num_windows)
> + return;
> +
> + for (i = 0; i <= epc->num_windows; i++) {
I'm sorry, I overlooked when I reviewed before.
This condition should be "i < epc->num_windows".
> + mem = epc->windows[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->windows);
> +
> + epc->windows = NULL;
> epc->mem = NULL;
> - kfree(mem->bitmap);
> - kfree(mem);
> + epc->num_windows = 0;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>
> @@ -129,31 +168,57 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size)
> {
> - int pageno;
> void __iomem *virt_addr = NULL;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + size_t align_size;
> + int pageno;
> int order;
> + int i;
>
> - size = ALIGN(size, mem->page_size);
> - order = pci_epc_mem_get_order(mem, size);
> -
> - mutex_lock(&mem->lock);
> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
> - if (pageno < 0)
> - goto ret;
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> + mutex_lock(&mem->lock);
> + align_size = ALIGN(size, mem->window.page_size);
> + order = pci_epc_mem_get_order(mem, align_size);
>
> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
> - virt_addr = ioremap(*phys_addr, size);
> - if (!virt_addr)
> - bitmap_release_region(mem->bitmap, pageno, order);
> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
> + order);
> + if (pageno >= 0) {
> + page_shift = ilog2(mem->window.page_size);
> + *phys_addr = mem->window.phys_base +
> + ((phys_addr_t)pageno << page_shift);
> + virt_addr = ioremap(*phys_addr, align_size);
> + if (!virt_addr)
> + bitmap_release_region(mem->bitmap,
> + pageno, order);
I'm sorry here again. But, I think we should call mutex_unlock() and "continue;"
here if ioremap() failed for trying remaining windows. What do you think?
> + mutex_unlock(&mem->lock);
> + return virt_addr;
> + }
> + mutex_unlock(&mem->lock);
> + }
>
> -ret:
> - mutex_unlock(&mem->lock);
> return virt_addr;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
Best regards,
Yoshihiro Shimoda
