-----Original Message-----
From: Christoph Hellwig [mailto:hch@lst.de]
Sent: Wednesday, July 29, 2020 12:23 AM
To: Song Bao Hua (Barry Song) <redacted>
Cc: Christoph Hellwig <hch@lst.de>; m.szyprowski@samsung.com;
robin.murphy@arm.com; will@kernel.org; ganapatrao.kulkarni@cavium.com;
catalin.marinas@arm.com; iommu@lists.linux-foundation.org; Linuxarm
[off-list ref]; linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org; Zengtao (B) [off-list ref];
huangdaode [off-list ref]; Jonathan Cameron
[off-list ref]; Nicolas Saenz Julienne
[off-list ref]; Steve Capper [off-list ref]; Andrew
Morton [off-list ref]; Mike Rapoport [off-list ref]
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:

quoted

I am sorry I haven't got your point yet. Do you mean something like the

below?

quoted

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.

quoted

A background of the current code is that Linux distributions can usually use

arch/arm64/configs/defconfig

quoted

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..

quoted

quoted

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

quoted

server with NUMA.

Well, than maybe ACPI experts need to chime in on this.

quoted

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel