linux-kernel - Re: [RFCv5 3/9] mm: cma: Added SysFS support

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:	Mon, 6 Sep 2010 23:08:18 -0700
From:	Greg KH <greg@...ah.com>
To:	Micha?? Nazarewicz <m.nazarewicz@...sung.com>
Cc:	Hans Verkuil <hverkuil@...all.nl>,
	Peter Zijlstra <peterz@...radead.org>,
	Daniel Walker <dwalker@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	Jonathan Corbet <corbet@....net>, Mel Gorman <mel@....ul.ie>,
	Pawel Osciak <p.osciak@...sung.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	linux-kernel@...r.kernel.org,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	linux-mm@...ck.org, Kyungmin Park <kyungmin.park@...sung.com>,
	Minchan Kim <minchan.kim@...il.com>,
	Zach Pfeffer <zpfeffer@...eaurora.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org
Subject: Re: [RFCv5 3/9] mm: cma: Added SysFS support

On Tue, Sep 07, 2010 at 07:31:30AM +0200, Micha?? Nazarewicz wrote:
> Hello Greg,
>
> Thanks for reviewing the sysfs part.  Actually, I was never really sure
> if I shouldn't rather put this code to debugfs and you got me convinced
> that I should.  Sysfs somehow looked more appealing from kernel's API
> point of view -- things seem to be more organised in sysfs than in
> debugfs.  It seems I'll have to port it to debugfs after all

Yes, debugfs looks like a much better place for this.  You can do
whatever you want in debugfs as long as you follow the one rule for it:
	There are no rules for debugfs.

> Nonetheless, a few responses to your comments:
>
>> On Mon, Sep 06, 2010 at 08:33:53AM +0200, Michal Nazarewicz wrote:
>>> +		The "allocators" file list all registered allocators.
>>> +		Allocators with no name are listed as a single minus
>>> +		sign.
>
> On Mon, 06 Sep 2010 23:07:47 +0200, Greg KH <greg@...ah.com> wrote:
>> So this returns more than one value?
>
> Aren't thing like cpufreq governors listed in a single sysfs file?

Yeah, but I don't like it :)

> I remember there was such a file somewhere.  Has that been made
> deprecated? I cannot seem to find any information on that.

It's best if you really don't do this, but it does happen as it is the
best way to show the information.  If that's the case, fine.

>>> +		The "regions" directory list all reserved regions.
>>
>> Same here?
>
> regions is actually a directory with subdirectories for each
> region. ;)

Ah.

>>> +static ssize_t cma_sysfs_region_name_show(struct cma_region *reg, char *page)
>>> +{
>>> +	return reg->name ? snprintf(page, PAGE_SIZE, "%s\n", reg->name) : 0;
>>> +}
>
>> Is a name field ever really going to be bigger than a page?
>
> I prefer being on the safe side -- I have no idea what user will provide
> as region name so I assume as little as possible.

By "user" do you mean userspace, or another kernel driver file?  If it's
a kernel driver, you can assume that they will be sane.  if userspace,
assume it's insane and do some checking of the name before you use it.

> For numeric values you are right that snprintf() is a bit paranoid,
> still I see no good reason why not to use it.

Same goes for no good reason to use it :)

thanks,

greg k-h
--
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/