All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrzej Hajda <a.hajda@samsung.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, Marek Szyprowski <m.szyprowski@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linux-kernel@vger.kernel.org, andi@firstfloor.org, andi@lisas.de,
	Mike Turquette <mturquette@linaro.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH 0/5] kstrdup optimization
Date: Wed, 14 Jan 2015 09:06:20 +0100	[thread overview]
Message-ID: <54B6237C.5090500@samsung.com> (raw)
In-Reply-To: <20150113153731.43eefac721964d165396e5af@linux-foundation.org>

On 01/14/2015 12:37 AM, Andrew Morton wrote:
> On Mon, 12 Jan 2015 10:18:38 +0100 Andrzej Hajda <a.hajda@samsung.com> wrote:
>
>> Hi,
>>
>> kstrdup if often used to duplicate strings where neither source neither
>> destination will be ever modified. In such case we can just reuse the source
>> instead of duplicating it. The problem is that we must be sure that
>> the source is non-modifiable and its life-time is long enough.
>>
>> I suspect the good candidates for such strings are strings located in kernel
>> .rodata section, they cannot be modifed because the section is read-only and
>> their life-time is equal to kernel life-time.
>>
>> This small patchset proposes alternative version of kstrdup - kstrdup_const,
>> which returns source string if it is located in .rodata otherwise it fallbacks
>> to kstrdup.
>> To verify if the source is in .rodata function checks if the address is between
>> sentinels __start_rodata, __end_rodata. I guess it should work with all
>> architectures.
>>
>> The main patch is accompanied by four patches constifying kstrdup for cases
>> where situtation described above happens frequently.
>>
>> As I have tested the patchset on mobile platform (exynos4210-trats) it saves
>> 3272 string allocations. Since minimal allocation is 32 or 64 bytes depending
>> on Kconfig options the patchset saves respectively about 100KB or 200KB of memory.
> That's a lot of memory.  I wonder where it's all going to.  sysfs,
> probably?

Stats from tested platform.
By caller:
  2260 __kernfs_new_node
    631 clk_register+0xc8/0x1b8
    318 clk_register+0x34/0x1b8
      51 kmem_cache_create
      12 alloc_vfsmnt

By string (with count >= 5):
    883 power
    876 subsystem
    135 parameters
    132 device
     61 iommu_group
     44 sclk_mpll
     42 aclk100
     41 driver
     36 sclk_vpll
     35 none
     34 sclk_epll
     34 aclk160
     32 sclk_hdmi24m
     31 xxti
     31 xusbxti
     31 sclk_usbphy0
     30 sclk_hdmiphy
     28 bdi
     28 aclk133
     14 sclk_apll
     14 aclk200
      9 module
      9 fin_pll
      5 div_core2
   


>
> What the heck does (the cheerily undocumented) KERNFS_STATIC_NAME do
> and can we remove it if this patchset is in place?
>
>

The only call path when this flag is set starts from
sysfs_add_file_mode_ns function.
But I guess this function can be called also for non-const names.

Regards
Andrzej


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

WARNING: multiple messages have this Message-ID (diff)
From: Andrzej Hajda <a.hajda@samsung.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, Marek Szyprowski <m.szyprowski@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linux-kernel@vger.kernel.org, andi@firstfloor.org, andi@lisas.de,
	Mike Turquette <mturquette@linaro.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH 0/5] kstrdup optimization
Date: Wed, 14 Jan 2015 09:06:20 +0100	[thread overview]
Message-ID: <54B6237C.5090500@samsung.com> (raw)
In-Reply-To: <20150113153731.43eefac721964d165396e5af@linux-foundation.org>

On 01/14/2015 12:37 AM, Andrew Morton wrote:
> On Mon, 12 Jan 2015 10:18:38 +0100 Andrzej Hajda <a.hajda@samsung.com> wrote:
>
>> Hi,
>>
>> kstrdup if often used to duplicate strings where neither source neither
>> destination will be ever modified. In such case we can just reuse the source
>> instead of duplicating it. The problem is that we must be sure that
>> the source is non-modifiable and its life-time is long enough.
>>
>> I suspect the good candidates for such strings are strings located in kernel
>> .rodata section, they cannot be modifed because the section is read-only and
>> their life-time is equal to kernel life-time.
>>
>> This small patchset proposes alternative version of kstrdup - kstrdup_const,
>> which returns source string if it is located in .rodata otherwise it fallbacks
>> to kstrdup.
>> To verify if the source is in .rodata function checks if the address is between
>> sentinels __start_rodata, __end_rodata. I guess it should work with all
>> architectures.
>>
>> The main patch is accompanied by four patches constifying kstrdup for cases
>> where situtation described above happens frequently.
>>
>> As I have tested the patchset on mobile platform (exynos4210-trats) it saves
>> 3272 string allocations. Since minimal allocation is 32 or 64 bytes depending
>> on Kconfig options the patchset saves respectively about 100KB or 200KB of memory.
> That's a lot of memory.  I wonder where it's all going to.  sysfs,
> probably?

Stats from tested platform.
By caller:
  2260 __kernfs_new_node
    631 clk_register+0xc8/0x1b8
    318 clk_register+0x34/0x1b8
      51 kmem_cache_create
      12 alloc_vfsmnt

By string (with count >= 5):
    883 power
    876 subsystem
    135 parameters
    132 device
     61 iommu_group
     44 sclk_mpll
     42 aclk100
     41 driver
     36 sclk_vpll
     35 none
     34 sclk_epll
     34 aclk160
     32 sclk_hdmi24m
     31 xxti
     31 xusbxti
     31 sclk_usbphy0
     30 sclk_hdmiphy
     28 bdi
     28 aclk133
     14 sclk_apll
     14 aclk200
      9 module
      9 fin_pll
      5 div_core2
   


>
> What the heck does (the cheerily undocumented) KERNFS_STATIC_NAME do
> and can we remove it if this patchset is in place?
>
>

The only call path when this flag is set starts from
sysfs_add_file_mode_ns function.
But I guess this function can be called also for non-const names.

Regards
Andrzej



  reply	other threads:[~2015-01-14  8:06 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-12  9:18 [PATCH 0/5] kstrdup optimization Andrzej Hajda
2015-01-12  9:18 ` Andrzej Hajda
2015-01-12  9:18 ` [PATCH 1/5] mm/util: add kstrdup_const Andrzej Hajda
2015-01-12  9:18   ` Andrzej Hajda
2015-01-12 17:13   ` Joe Perches
2015-01-12 17:13     ` Joe Perches
2015-01-12  9:18 ` [PATCH 2/5] kernfs: convert node name allocation to kstrdup_const Andrzej Hajda
2015-01-12  9:18   ` Andrzej Hajda
2015-01-14 14:13   ` Tejun Heo
2015-01-14 14:13     ` Tejun Heo
2015-01-14 14:37   ` [PATCH 2.5/5] kernfs: remove KERNFS_STATIC_NAME Tejun Heo
2015-01-14 14:37     ` Tejun Heo
2015-01-12  9:18 ` [PATCH 3/5] clk: convert clock name allocations to kstrdup_const Andrzej Hajda
2015-01-12  9:18   ` Andrzej Hajda
2015-01-12 23:11   ` Mike Turquette
2015-01-13  7:57     ` Andrzej Hajda
2015-01-13  7:57       ` Andrzej Hajda
2015-01-12  9:18 ` [PATCH 4/5] mm/slab: convert cache " Andrzej Hajda
2015-01-12  9:18   ` Andrzej Hajda
2015-01-12  9:18 ` [PATCH 5/5] fs/namespace: convert devname allocation " Andrzej Hajda
2015-01-12  9:18   ` Andrzej Hajda
2015-01-12 20:45 ` [PATCH 0/5] kstrdup optimization Geert Uytterhoeven
2015-01-12 20:45   ` Geert Uytterhoeven
2015-01-13 23:48   ` Andrew Morton
2015-01-13 23:48     ` Andrew Morton
2015-01-14  0:10   ` Craig Milo Rogers
2015-01-14  0:10     ` Craig Milo Rogers
2015-01-14  0:17     ` Andrew Morton
2015-01-14  0:17       ` Andrew Morton
2015-01-13 23:37 ` Andrew Morton
2015-01-13 23:37   ` Andrew Morton
2015-01-14  8:06   ` Andrzej Hajda [this message]
2015-01-14  8:06     ` Andrzej Hajda
2015-01-14 14:12   ` Tejun Heo
2015-01-14 14:12     ` Tejun Heo

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=54B6237C.5090500@samsung.com \
    --to=a.hajda@samsung.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=andi@lisas.de \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mturquette@linaro.org \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.