All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: akpm@linux-foundation.org, feng.tang@intel.com,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [RFC PATCH] devres: avoid over memory allocation with managed memory allocation
Date: Sat, 23 Jul 2022 12:10:48 +0200	[thread overview]
Message-ID: <YtvJKF+2wSD57NbO@kroah.com> (raw)
In-Reply-To: <92ec2f78e8d38f68da95d9250cf3f86b2fbe78ad.1658570017.git.christophe.jaillet@wanadoo.fr>

On Sat, Jul 23, 2022 at 12:04:33PM +0200, Christophe JAILLET wrote:
> On one side, when using devm_kmalloc(), a memory overhead is added in order
> to keep track of the data needed to release the resources automagically.
> 
> On the other side, kmalloc() also rounds-up the required memory size in
> order to ease memory reuse and avoid memory fragmentation.
> 
> Both behavior together can lead to some over memory allocation which can
> be avoided.
> 
> For example:
>   - if 4096 bytes of managed memory is required
>   - "4096 + sizeof(struct devres_node)" bytes are required to the memory
> allocator
>   - 8192 bytes are allocated and nearly half of it is wasted
> 
> In such a case, it would be better to really allocate 4096 bytes of memory
> and record an "action" to perform the kfree() when needed.
> 
> On my 64 bits system:
>    sizeof(struct devres_node) = 40
>    sizeof(struct action_devres) = 16
> 
> So, a devm_add_action() call will allocate 56, rounded up to 64 bytes.
> 
> kmalloc() uses hunks of 8k, 4k, 2k, 1k, 512, 256, 192, 128, 96, 64, 32, 16,
> 8 bytes.
> 
> So in order to save some memory, if the 256 bytes boundary is crossed
> because of the overhead of devm_kmalloc(), 2 distinct memory allocations
> make sense.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is only a RFC to get feed-back on the proposed approach.
> 
> It is compile tested only.
> I don't have numbers to see how much memory could be saved.
> I don't have numbers on the performance impact.
> 
> Should this makes sense to anyone, I would really appreciate getting some
> numbers from others to confirm if it make sense or not.
> 
> 
> The idea of this patch came to me because of a discussion initiated by
> Feng Tang <feng.tang@intel.com>. He proposes to track wasted memory
> allocation in order to give hints on where optimizations can be done.
> 
> My approach is to avoid part of these allocations when due to the usage of
> a devm_ function.
> 
> 
> The drawbacks I see are:
>    - code is more complex
>    - this concurs to memory fragmentation because there will be 2 memory
>      allocations, instead of just 1
>    - this is slower for every memory allocation because of the while loop
>      and tests
>    - the magic 256 constant is maybe not relevant on all systems
>    - some places of the kernel already take advantage of this over memory
>      allocation. So unpredictable impacts can occur somewhere! (see [1],
>      which is part of the [2] thread)
>    - this makes some assumption in devres.c on how memory allocation works,
>      which is not a great idea :(
> 
> The advantages I see:
>    - in some cases, it saves some memory :)
>    - fragmentation is not necessarily an issue, devm_ allocated memory
>      are rarely freed, right?

I think devm_  allocated memory does not happen that much, try it on
your systems and see!

Numbers would be great to have, can you run some benchmarks?  Try it on
a "common" SoC device (raspberry pi?) and a desktop to compare.

thanks,

greg k-h

  reply	other threads:[~2022-07-23 10:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-23 10:04 [RFC PATCH] devres: avoid over memory allocation with managed memory allocation Christophe JAILLET
2022-07-23 10:10 ` Greg Kroah-Hartman [this message]
2022-07-25 13:30 ` [devres] 3d0e198cd7: WARNING:at_drivers/base/devres.c:#devm_kfree kernel test robot
2022-07-25 13:30   ` kernel test robot

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=YtvJKF+2wSD57NbO@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=feng.tang@intel.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rafael@kernel.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.