All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Timo Teräs" <timo.teras@iki.fi>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH] xfrm: cache bundle lookup results in flow cache
Date: Tue, 23 Mar 2010 11:19:05 +0200	[thread overview]
Message-ID: <4BA88789.7060104@iki.fi> (raw)
In-Reply-To: <20100323074252.GA27623@gondor.apana.org.au>

Herbert Xu wrote:
> On Tue, Mar 23, 2010 at 09:28:33AM +0200, Timo Teräs wrote:
>> So, should I go ahead and do the virtualization of the
>> getters and putters?
> 
> If we're going to have indirect calls per packet then let's at
> least minimise them.  I think one should be enough, i.e., just
> add a ->stale() function pointer to be used in flow_cache_lookup.

Normally, only the getter is called per-packet. So I'm thinking
to have get() that would check for entry being stale or not,
and bump up the refcount.

The resolver can just swap the old entry and call appropriate
_put/_get, so we can avoid virtual calls there.

Thinking more about the flushing of the flow cache. It's basically
only needed to flush before the policies are garbage collected.
This is strictly needed so we can do the atomic_dec() without
turning policy's refcount to zero and causing a leak.

I'm now thinking if it'd be worth doing virtual _put(). This
way we would not need flushing at all for in policy garbage
collector (we could kill flow_cache_flush). We could also
call dst_release immediately in the flow cache _put, which
would release the memory faster. This way we would not need
at all any periodic xfrm_dst checker as flow cache is
regenerated regularly.

The code paths that would require calling the virtual put
are:
- randomization of flow cache (every 10 mins currently)
  from flow_cache_lookup() with bh disabled
- flow cache going too full, from flow_cache_lookup()
  with bh disabled
- cpu notifier

Do you think the virtual _put doing more work would be
too slow?

In that case the plain atomic_dec sounds ok, but we'd then
need to periodically check through the global bundle list
for garbage collection.

Cheers,
  Timo

  reply	other threads:[~2010-03-23  9:19 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-15 12:20 [PATCH] xfrm: cache bundle lookup results in flow cache Timo Teras
2010-03-17 13:07 ` Herbert Xu
2010-03-17 14:16   ` Timo Teräs
2010-03-17 14:58     ` Herbert Xu
2010-03-17 15:56       ` Timo Teräs
2010-03-17 16:32         ` Timo Teräs
2010-03-18 19:30         ` Timo Teräs
2010-03-19  0:31           ` Herbert Xu
2010-03-19  5:48             ` Timo Teräs
2010-03-19  6:03               ` Herbert Xu
2010-03-19  6:21                 ` Timo Teräs
2010-03-19  7:17                   ` Herbert Xu
2010-03-19  7:27                   ` Timo Teräs
2010-03-19  0:32           ` Herbert Xu
2010-03-19  7:20 ` Herbert Xu
2010-03-19  7:48   ` Timo Teräs
2010-03-19  8:29     ` Herbert Xu
2010-03-19  8:37       ` Timo Teräs
2010-03-19  8:47         ` Herbert Xu
2010-03-19  9:12           ` Timo Teräs
2010-03-19  9:32             ` Herbert Xu
2010-03-19  9:53               ` Timo Teräs
2010-03-20 15:17                 ` Herbert Xu
2010-03-20 16:26                   ` Timo Teräs
2010-03-21  0:46                     ` Herbert Xu
2010-03-21  7:34                       ` Timo Teräs
2010-03-21  8:31                         ` Timo Teräs
2010-03-22  3:52                           ` Herbert Xu
2010-03-22 18:03                             ` Timo Teräs
2010-03-23  7:28                               ` Timo Teräs
2010-03-23  7:42                                 ` Herbert Xu
2010-03-23  9:19                                   ` Timo Teräs [this message]
2010-03-23  9:41                                     ` Herbert Xu
2010-03-22  1:26                   ` David Miller
2010-03-22  1:28                   ` David Miller
2010-03-22  1:32                     ` Herbert Xu
2010-03-22  1:36                       ` David Miller
2010-03-22  1:40                         ` Herbert Xu
2010-03-22  3:12                           ` David Miller
2010-03-22  3:52                             ` Herbert Xu
2010-03-22 18:31                               ` Timo Teräs

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=4BA88789.7060104@iki.fi \
    --to=timo.teras@iki.fi \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@vger.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.