> -----Original Message----- > From: Christoph Hellwig [mailto:h...@lst.de] > Sent: Wednesday, July 29, 2020 12:23 AM > To: Song Bao Hua (Barry Song) <song.bao....@hisilicon.com> > Cc: Christoph Hellwig <h...@lst.de>; m.szyprow...@samsung.com; > robin.mur...@arm.com; w...@kernel.org; ganapatrao.kulka...@cavium.com; > catalin.mari...@arm.com; io...@lists.linux-foundation.org; Linuxarm > <linux...@huawei.com>; linux-arm-ker...@lists.infradead.org; > linux-kernel@vger.kernel.org; Zengtao (B) <prime.z...@hisilicon.com>; > huangdaode <huangda...@huawei.com>; Jonathan Cameron > <jonathan.came...@huawei.com>; Nicolas Saenz Julienne > <nsaenzjulie...@suse.de>; Steve Capper <steve.cap...@arm.com>; Andrew > Morton <a...@linux-foundation.org>; Mike Rapoport <r...@linux.ibm.com> > Subject: Re: [PATCH v4 1/2] dma-direct: provide the ability to reserve > per-numa CMA > > On Tue, Jul 28, 2020 at 12:19:03PM +0000, Song Bao Hua (Barry Song) wrote: > > I am sorry I haven't got your point yet. Do you mean something like the > below? > > > > arch/arm64/Kconfig: > > config CMDLINE > > string "Default kernel command string" > > - default "" > > + default "pernuma_cma=16M" > > help > > Provide a set of default command-line options at build time by > > entering them here. As a minimum, you should specify the the > > root device (e.g. root=/dev/nfs). > > Yes. > > > A background of the current code is that Linux distributions can usually use > arch/arm64/configs/defconfig > > directly to build kernel. cmdline can be easily ignored during the > > generation > of Linux distributions. > > I've not actually heard of a distro shipping defconfig yet.. > > > > > > if a way to expose this in the device tree might be useful, but people > > > more familiar with the device tree and the arm code will have to chime > > > in on that. > > > > Not sure if it is an useful user case as we are using ACPI but not device > > tree > since it is an ARM64 > > server with NUMA. > > Well, than maybe ACPI experts need to chime in on this. > > > > This seems to have lost the dma_contiguous_default_area NULL check. > > > > cma_alloc() is doing the check by returning NULL if cma is NULL. > > > > struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align, > > bool no_warn) > > { > > ... > > if (!cma || !cma->count) > > return NULL; > > } > > > > But I agree here the code can check before calling cma_alloc_aligned. > > Oh, indeed. Please split the removal of the NULL check in to a prep > patch then. Do you mean removing the NULL check in cma_alloc()? If so, it seems lot of places need to be changed: struct page *dma_alloc_from_contiguous(struct device *dev, size_t count, unsigned int align, bool no_warn) { if (align > CONFIG_CMA_ALIGNMENT) align = CONFIG_CMA_ALIGNMENT; + code to check dev_get_cma_area(dev) is not NULL return cma_alloc(dev_get_cma_area(dev), count, align, no_warn); } bool dma_release_from_contiguous(struct device *dev, struct page *pages, int count) { + code to check dev_get_cma_area(dev) is not NULL return cma_release(dev_get_cma_area(dev), pages, count); } bool cma_release(struct cma *cma, const struct page *pages, unsigned int count) { unsigned long pfn; + do we need to remove this !cma too if we remove it in cma_alloc()? if (!cma || !pages) return false; ... } And some other places where cma_alloc() and cma_release() are called: arch/powerpc/kvm/book3s_hv_builtin.c drivers/dma-buf/heaps/cma_heap.c drivers/s390/char/vmcp.c drivers/staging/android/ion/ion_cma_heap.c mm/hugetlb.c it seems many code were written with the assumption that cma_alloc/release will check if cma is null so they don't check it before calling cma_alloc(). And I am not sure if kernel robot will report error like pointer reference before checking it if !cma is removed in cma_alloc(). Thanks Barry
