linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Joe Perches <joe@perches.com>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH] mm: util: update the kerneldoc for kstrdup_const()
Date: Tue, 30 Jun 2020 16:36:34 +0200	[thread overview]
Message-ID: <9dfad174-4e8b-c733-b529-5c86a34333d4@redhat.com> (raw)
In-Reply-To: <b370f8bfbf2bfc958b15ce6f6d138bec64972183.camel@perches.com>

On 30.06.20 16:14, Joe Perches wrote:
> On Tue, 2020-06-30 at 10:57 +0200, David Hildenbrand wrote:
>> On 29.06.20 21:21, Joe Perches wrote:
>>> On Mon, 2020-06-29 at 12:54 +0200, David Hildenbrand wrote:
>>>> On 28.06.20 19:37, Joe Perches wrote:
>>>>> On Sun, 2020-06-28 at 17:25 +0200, Bartosz Golaszewski wrote:
>>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>
>>>>>> Memory allocated with kstrdup_const() must not be passed to regular
>>>>>> krealloc() as it is not aware of the possibility of the chunk residing
>>>>>> in .rodata. Since there are no potential users of krealloc_const()
>>>>>> at the moment, let's just update the doc to make it explicit.
>>>>>
>>>>> Another option would be to return NULL if it's
>>>>> used from krealloc with a pointer into rodata
>>> []
>>>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>> []
>>>>> @@ -1683,6 +1683,9 @@ static __always_inline void *__do_krealloc(const void *p, size_t new_size,
>>>>>   * @new_size: how many bytes of memory are required.
>>>>>   * @flags: the type of memory to allocate.
>>>>>   *
>>>>> + * If the object pointed to is in rodata (likely from kstrdup_const)
>>>>> + * %NULL is returned.
>>>>> + *
>>> []
>>>> Won't we have similar issues if somebody would do a kfree() instead of a
>>>> kfree_const()? So I think the original patch makes sense.
>>>
>>> Which is why I also suggested making kfree work for
>>> more types of memory freeing earlier this month.
>>>
>>> https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.camel@perches.com/
> []
>> what's the real benefit that is worth spending extra runtime cycles?
> 
> I very much doubt there is an actual instance
> where the runtime cycles matter.  Where could
> there be a fast-path instance of free?

Well, looking at kfree() I can directly spot "unlikely()", which sounds
like somebody cares about branch prediction in the slab.

Once you have cases that can happen equally likely it most certainly
degrades performance. The question is if we care.

Coming back to my question, so the major benefit you see is coding
simplicity, correct?

-- 
Thanks,

David / dhildenb


  reply	other threads:[~2020-06-30 14:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-28 15:25 [PATCH] mm: util: update the kerneldoc for kstrdup_const() Bartosz Golaszewski
2020-06-28 17:37 ` Joe Perches
2020-06-28 18:06   ` Bartosz Golaszewski
2020-06-28 18:20     ` Joe Perches
2020-06-29 10:54   ` David Hildenbrand
2020-06-29 19:21     ` Joe Perches
2020-06-30  8:57       ` David Hildenbrand
2020-06-30 14:14         ` Joe Perches
2020-06-30 14:36           ` David Hildenbrand [this message]
2020-06-30 14:51             ` Joe Perches

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=9dfad174-4e8b-c733-b529-5c86a34333d4@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).