linux-kernel - RE: [PATCH v4 1/2] dma-direct: provide the ability to reserve per-numa CMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <B926444035E5E2439431908E3842AFD25C8B1D@DGGEMI525-MBS.china.huawei.com>
Date:   Wed, 29 Jul 2020 11:21:09 +0000
From:   "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
To:     Christoph Hellwig <hch@....de>
CC:     "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "will@...nel.org" <will@...nel.org>,
        "ganapatrao.kulkarni@...ium.com" <ganapatrao.kulkarni@...ium.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        Linuxarm <linuxarm@...wei.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Zengtao (B)" <prime.zeng@...ilicon.com>,
        huangdaode <huangdaode@...wei.com>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Steve Capper <steve.capper@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mike Rapoport <rppt@...ux.ibm.com>
Subject: RE: [PATCH v4 1/2] dma-direct: provide the ability to reserve
 per-numa CMA



> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@....de]
> Sent: Wednesday, July 29, 2020 12:23 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@...ilicon.com>
> Cc: Christoph Hellwig <hch@....de>; m.szyprowski@...sung.com;
> robin.murphy@....com; will@...nel.org; ganapatrao.kulkarni@...ium.com;
> catalin.marinas@....com; iommu@...ts.linux-foundation.org; Linuxarm
> <linuxarm@...wei.com>; linux-arm-kernel@...ts.infradead.org;
> linux-kernel@...r.kernel.org; Zengtao (B) <prime.zeng@...ilicon.com>;
> huangdaode <huangdaode@...wei.com>; Jonathan Cameron
> <jonathan.cameron@...wei.com>; Nicolas Saenz Julienne
> <nsaenzjulienne@...e.de>; Steve Capper <steve.capper@....com>; Andrew
> Morton <akpm@...ux-foundation.org>; Mike Rapoport <rppt@...ux.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