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] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 22 Jul 2010 13:46:00 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Michał Nazarewicz <m.nazarewicz@...sung.com>
Cc:	Marek Szyprowski <m.szyprowski@...sung.com>,
	'Daniel Walker' <dwalker@...eaurora.org>, linux-mm@...ck.org,
	Pawel Osciak <p.osciak@...sung.com>,
	'Xiaolin Zhang' <xiaolin.zhang@...el.com>,
	'Hiremath Vaibhav' <hvaibhav@...com>,
	'Robert Fekete' <robert.fekete@...ricsson.com>,
	'Marcus Lorentzon' <marcus.xm.lorentzon@...ricsson.com>,
	linux-kernel@...r.kernel.org,
	'Kyungmin Park' <kyungmin.park@...sung.com>,
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/4] mm: cma: Contiguous Memory Allocator added

On Thu, Jul 22, 2010 at 01:30:52PM +0200, Michał Nazarewicz wrote:

> The first one, I believe, should be there as to specify the regions
> that are to be reserved.  Drivers and platform will still be able to
> add their own regions but I believe that in vest majority of cases,
> it will be enough to just pass the list of region on a command line.

The command line is a real pain for stuff like this since it's not
usually committed to revision control so robustly and it's normally more
painful to change the bootloader to pass the desired command line in
than it is to change either the kernel or userspace (some bootloaders
are just completely unconfigurable without reflashing, and if your only
recovery mechanism is JTAG that can be a bit of a concern).

> Alternatively, instead of the textual description of platform could
> provide an array of regions it want reserved.  It would remove like
> 50 lines of code from CMA core (in the version I have on my drive at
> least, where part of the syntax was simplified) however it would
> remove the possibility to easily change the configuration from
> command line (ie. no need to recompile which is handy when you need
> to optimise this and test various configurations) and would add more
> code to the platform initialisation code, ie: instead of:

> 	cma_defaults("reg1=20M;reg2=20M", NULL);

> one would have to define an array with the regions descriptors.
> Personally, I don't see much benefits from this.

I think it'd be vastly more legible, especially if the list of regions
gets large.  I had thought the only reason for the text format was to
put it onto the command line.

> I agree that parsing it is not nice but thanks to it, all you need to
> do in the driver is:

> 	cma_alloc(dev, "a", ...)
> 	cma_alloc(dev, "b", ...)
> 	cma_alloc(dev, "f", ...)

> Without cma_map you'd have to pass names of the region to the driver
> and make the driver use those.

I agree that a mapping facility for the names is essential, especially
if drivers need to share regions.

> What I'm trying to say is that I'm trying to move complexity out of
> the drivers into the framework (as I believe that's what frameworks
> are for).

It sounds like apart from the way you're passing the configuration in
you're doing roughly what I'd suggest.  I'd expect that in a lot of
cases the map could be satisfied from the default region so there'd be
no need to explicitly set one up.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ