LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: Pekka J Enberg <penberg@cs.helsinki.fi>
Cc: Netfilter Development Mailinglist 
	<netfilter-devel@vger.kernel.org>,
	clameter@sgi.com, joe@perches.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH] netfilter: replace horrible hack with ksize()
Date: Thu, 06 Mar 2008 15:35:57 +0100	[thread overview]
Message-ID: <47D0014D.1060901@trash.net> (raw)
In-Reply-To: <Pine.LNX.4.64.0803061618090.4378@sbz-30.cs.Helsinki.FI>

Pekka J Enberg wrote:
> On Thu, 6 Mar 2008, Pekka J Enberg wrote:
>>>> -	if (newlen >= ct->ext->real_len) {
>>>> +	if (newlen >= ksize(ct->ext)) {
>>> This needs to look at the currently allocated size, otherwise
>>> it will always realloc when adding new extensions after having
>>> used up ksize(ct->ext) space.
>> Lets say you
>>
>>   p = kmalloc(8, ...);
>>
>> Then ksize(p) will return the currently allocated size which is 32 bytes 
>> when page size is 4 KB, and not 8 bytes. So it should be equivalent of 
>> what the current code does.
>>
>> What am I missing here?
> 
> Ok, it's not equivalent. We have two sizes: object size (8 bytes) and 
> buffer size (32 bytes) here. In netfilter, ->real_len is same as object 
> size, not buffer size as ksize() is.
> 
> But now I am officially even more confused, why does the netfilter code 
> decided whether to reallocate based on _object size_ and not _buffer size_ 
> (as krealloc() does, for example)?


It decides to reallocate when the remaining space isn't enough
to hold the new data. NF_CT_EXT_MIN_SIZE is used to make sure it
doesn't allocate anything smaller than the minimum slab size and
hopefully avoid reallocations in the future. Unless I'm
misunderstanding what ksize() does, the easiest way to get
rid of this would be to replace NF_CT_EXT_MIN_SIZE by ksize(0).

Even better would be to not only avoid waste on the first allocation,
but also on reallocations. This would look something like this:

__nf_ct_ext_add():
{
         struct nf_ct_ext *new;
-       int i, newlen, newoff;
+	int i, newlen, newoff, reallen;
...
         if (newlen >= ct->ext->real_len) {
+		reallen = ksize(newlen);
-               new = kmalloc(newlen, gfp);
+		new = kmalloc(reallen, gfp);
                 if (!new)
                         return NULL;
...
-               new->real_len = newlen;
+		new->real_len = reallen;
                 ct->ext = new;
         }


  reply	other threads:[~2008-03-06 14:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-05 21:20 Pekka J Enberg
2008-03-05 21:56 ` Christoph Lameter
2008-03-05 21:57   ` Pekka Enberg
2008-03-05 22:19   ` David Miller
2008-03-06 14:03 ` Patrick McHardy
2008-03-06 14:14   ` Pekka J Enberg
2008-03-06 14:20     ` Pekka J Enberg
2008-03-06 14:35       ` Patrick McHardy [this message]
2008-03-06 14:41         ` Pekka J Enberg
2008-03-06 14:51           ` Patrick McHardy
2008-03-06 15:49             ` Pekka Enberg
2008-03-10 17:57               ` Patrick McHardy
2008-03-06 16:41     ` Christoph Lameter

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=47D0014D.1060901@trash.net \
    --to=kaber@trash.net \
    --cc=clameter@sgi.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=penberg@cs.helsinki.fi \
    --subject='Re: [PATCH] netfilter: replace horrible hack with ksize()' \
    /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

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