Messages in this thread | | | Subject | Re: [PATCH] PCI: endpoint: Use 'bitmap_zalloc()' when applicable | From | Christophe JAILLET <> | Date | Sun, 7 Nov 2021 07:52:44 +0100 |
| |
Le 06/11/2021 à 23:01, Krzysztof Wilczyński a écrit : > Hi Christophe, > >> 'mem->bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify code, >> improve the semantic and avoid some open-coded arithmetic in allocator >> arguments. >> >> Also change the corresponding 'kfree()' into 'bitmap_free()' to keep >> consistency. > > Thank you! > >> Finally, while at it, axe the useless 'bitmap' variable and use >> 'mem->bitmap' directly. > > Personally, I would keep the bitmap variable - this might be what Bjorn > would also prefer, as I believe he prefers not to store what is a "failed" > state of sorts in a target variable directly, if memory serves me right.
Hi,
mostly a mater of taste. On another similar patch I got the answer in [1] :).
'mem' is kzalloc'ed, so in case of failure, here we are just replacing NULL by NULL.
Let me know the preferred style here and if I should send a V2.
CJ
[1]; https://lore.kernel.org/kernel-janitors/20211028164437.GA4045120@p14s/
> > [...] >> @@ -49,10 +49,8 @@ int pci_epc_multi_mem_init(struct pci_epc *epc, >> unsigned int num_windows) >> { >> struct pci_epc_mem *mem = NULL; >> - unsigned long *bitmap = NULL; >> unsigned int page_shift; >> size_t page_size; >> - int bitmap_size; >> int pages; >> int ret; >> int i; >> @@ -72,7 +70,6 @@ int pci_epc_multi_mem_init(struct pci_epc *epc, >> page_size = PAGE_SIZE; >> page_shift = ilog2(page_size); >> pages = windows[i].size >> page_shift; >> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long); >> >> mem = kzalloc(sizeof(*mem), GFP_KERNEL); >> if (!mem) { >> @@ -81,8 +78,8 @@ int pci_epc_multi_mem_init(struct pci_epc *epc, >> goto err_mem; >> } >> >> - bitmap = kzalloc(bitmap_size, GFP_KERNEL); >> - if (!bitmap) { >> + mem->bitmap = bitmap_zalloc(pages, GFP_KERNEL); >> + if (!mem->bitmap) { >> ret = -ENOMEM; >> kfree(mem); >> i--; >> @@ -92,7 +89,6 @@ int pci_epc_multi_mem_init(struct pci_epc *epc, >> mem->window.phys_base = windows[i].phys_base; >> mem->window.size = windows[i].size; >> mem->window.page_size = page_size; >> - mem->bitmap = bitmap; >> mem->pages = pages; >> mutex_init(&mem->lock); >> epc->windows[i] = mem; >> @@ -106,7 +102,7 @@ int pci_epc_multi_mem_init(struct pci_epc *epc, >> err_mem: >> for (; i >= 0; i--) { >> mem = epc->windows[i]; >> - kfree(mem->bitmap); >> + bitmap_free(mem->bitmap); >> kfree(mem); >> } >> kfree(epc->windows); >> @@ -145,7 +141,7 @@ void pci_epc_mem_exit(struct pci_epc *epc) >> >> for (i = 0; i < epc->num_windows; i++) { >> mem = epc->windows[i]; >> - kfree(mem->bitmap); >> + bitmap_free(mem->bitmap); >> kfree(mem); >> } >> kfree(epc->windows); > > Thank you! > > Reviewed-by: Krzysztof Wilczyński <kw@linux.com> > > Krzysztof >
| |