All of lore.kernel.org
 help / color / mirror / Atom feed
From: m.nazarewicz@samsung.com (Michał Nazarewicz)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFCv5 3/9] mm: cma: Added SysFS support
Date: Tue, 07 Sep 2010 07:31:30 +0200	[thread overview]
Message-ID: <op.vindmsj07p4s8u@localhost> (raw)
In-Reply-To: <20100906210747.GA5863@kroah.com>

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

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@kroah.com> wrote:
> So this returns more than one value?

Aren't thing like cpufreq governors listed in a single sysfs file?
I remember there was such a file somewhere.  Has that been made
deprecated? I cannot seem to find any information on that.

>> +		The "regions" directory list all reserved regions.
>
> Same here?

regions is actually a directory with subdirectories for each
region. ;)

>> +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.  For numeric values you
are right that snprintf() is a bit paranoid, still I see no good reason
why not to use it.

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Micha? "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--

WARNING: multiple messages have this Message-ID (diff)
From: "Michał Nazarewicz" <m.nazarewicz@samsung.com>
To: Greg KH <greg@kroah.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	Peter Zijlstra <peterz@infradead.org>,
	Daniel Walker <dwalker@codeaurora.org>,
	Russell King <linux@arm.linux.org.uk>,
	Jonathan Corbet <corbet@lwn.net>, Mel Gorman <mel@csn.ul.ie>,
	Pawel Osciak <p.osciak@samsung.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	linux-kernel@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	linux-mm@kvack.org, Kyungmin Park <kyungmin.park@samsung.com>,
	Minchan Kim <minchan.kim@gmail.com>,
	Zach Pfeffer <zpfeffer@codeaurora.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [RFCv5 3/9] mm: cma: Added SysFS support
Date: Tue, 07 Sep 2010 07:31:30 +0200	[thread overview]
Message-ID: <op.vindmsj07p4s8u@localhost> (raw)
In-Reply-To: <20100906210747.GA5863@kroah.com>

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

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@kroah.com> wrote:
> So this returns more than one value?

Aren't thing like cpufreq governors listed in a single sysfs file?
I remember there was such a file somewhere.  Has that been made
deprecated? I cannot seem to find any information on that.

>> +		The "regions" directory list all reserved regions.
>
> Same here?

regions is actually a directory with subdirectories for each
region. ;)

>> +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.  For numeric values you
are right that snprintf() is a bit paranoid, still I see no good reason
why not to use it.

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--


WARNING: multiple messages have this Message-ID (diff)
From: "Michał Nazarewicz" <m.nazarewicz@samsung.com>
To: Greg KH <greg@kroah.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	Peter Zijlstra <peterz@infradead.org>,
	Daniel Walker <dwalker@codeaurora.org>,
	Russell King <linux@arm.linux.org.uk>,
	Jonathan Corbet <corbet@lwn.net>, Mel Gorman <mel@csn.ul.ie>,
	Pawel Osciak <p.osciak@samsung.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	linux-kernel@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	linux-mm@kvack.org, Kyungmin Park <kyungmin.park@samsung.com>,
	Minchan Kim <minchan.kim@gmail.com>,
	Zach Pfeffer <zpfeffer@codeaurora.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [RFCv5 3/9] mm: cma: Added SysFS support
Date: Tue, 07 Sep 2010 07:31:30 +0200	[thread overview]
Message-ID: <op.vindmsj07p4s8u@localhost> (raw)
In-Reply-To: <20100906210747.GA5863@kroah.com>

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

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@kroah.com> wrote:
> So this returns more than one value?

Aren't thing like cpufreq governors listed in a single sysfs file?
I remember there was such a file somewhere.  Has that been made
deprecated? I cannot seem to find any information on that.

>> +		The "regions" directory list all reserved regions.
>
> Same here?

regions is actually a directory with subdirectories for each
region. ;)

>> +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.  For numeric values you
are right that snprintf() is a bit paranoid, still I see no good reason
why not to use it.

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2010-09-07  5:31 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-06  6:33 [RFCv5 0/9] CMA + VCMM integration Michal Nazarewicz
2010-09-06  6:33 ` Michal Nazarewicz
2010-09-06  6:33 ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 1/9] lib: rbtree: rb_root_init() function added Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 2/9] mm: cma: Contiguous Memory Allocator added Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 3/9] mm: cma: Added SysFS support Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06 21:07   ` Greg KH
2010-09-06 21:07     ` Greg KH
2010-09-06 21:07     ` Greg KH
2010-09-07  5:31     ` Michał Nazarewicz [this message]
2010-09-07  5:31       ` Michał Nazarewicz
2010-09-07  5:31       ` Michał Nazarewicz
2010-09-07  6:08       ` Greg KH
2010-09-07  6:08         ` Greg KH
2010-09-07  6:08         ` Greg KH
2010-09-07  6:55         ` Michał Nazarewicz
2010-09-07  6:55           ` Michał Nazarewicz
2010-09-07  6:55           ` Michał Nazarewicz
2010-09-06  6:33 ` [RFCv5 4/9] mm: cma: Added command line parameters support Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 5/9] mm: cma: Test device and application added Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 6/9] ARM: cma: Added CMA to Aquila, Goni and c210 universal boards Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33 ` [RFCv5 7/9] mm: vcm: Virtual Contiguous Memory framework added Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-21 16:13   ` Konrad Rzeszutek Wilk
2010-09-21 16:13     ` Konrad Rzeszutek Wilk
2010-09-06  6:33 ` [RFCv5 8/9] mm: vcm: Sample driver added Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06 21:10   ` Greg KH
2010-09-06 21:10     ` Greg KH
2010-09-06 21:10     ` Greg KH
2010-09-07  1:58     ` Michał Nazarewicz
2010-09-07  1:58       ` Michał Nazarewicz
2010-09-07  1:58       ` Michał Nazarewicz
2010-09-06  6:33 ` [RFCv5 9/9] mm: vcm: vcm-cma: VCM CMA " Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06  6:33   ` Michal Nazarewicz
2010-09-06 21:09 ` [RFCv5 0/9] CMA + VCMM integration Greg KH
2010-09-06 21:09   ` Greg KH
2010-09-06 21:09   ` Greg KH
2010-09-07  1:40   ` Michał Nazarewicz
2010-09-07  1:40     ` Michał Nazarewicz
2010-09-07  1:40     ` Michał Nazarewicz
2010-09-07  2:34     ` Greg KH
2010-09-07  2:34       ` Greg KH
2010-09-07  2:34       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=op.vindmsj07p4s8u@localhost \
    --to=m.nazarewicz@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.