All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Kellermann <max@duempel.org>
To: pablo@netfilter.org, netfilter-devel@vger.kernel.org
Subject: conntrackd: questions about the new alarm implementation
Date: Mon, 21 Jan 2008 07:20:59 +0100	[thread overview]
Message-ID: <20080121062059.GA3995@swift.blarg.de> (raw)

Hi Pablo,

I have had a look at your new alarm.c.  I have a few questions about
it:

- Please explain why you now have 2048 (!) alarm queues, where the
  correct one is determined by hashing the alarm struct.  I fail to
  imagine how this hashing might be useful.  I can only see that it
  makes the code more complex and 2048 times slower - except for
  add_alarm(), which becomes a little bit faster, but there are only
  few add_alarm() invocations compared with get_next_alarm_run() and
  do_alarm_run().

- __run() is again bugged regarding due alarms: when an alarm is due,
  all readied file descriptors are ignored (l 193).  This is
  automatically recovered in the next select() call, so you didn't
  notice it.  Why do you keep screwing up this piece of code over and
  over? ;-)

- why this overcomplicated new __run() prototype with an int pointer
  parameter?  Its returned value is never used.  Its input value is
  only used to set next_alarm to NULL - why not just pass NULL as
  next_alarm in the first place (my patches did exactly that)?  Why
  does __run() have a return value?

Max


             reply	other threads:[~2008-01-21  6:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-21  6:20 Max Kellermann [this message]
2008-01-21 12:55 ` conntrackd: questions about the new alarm implementation Pablo Neira Ayuso
2008-01-21 13:05   ` Max Kellermann
2008-01-21 13:18     ` Pablo Neira Ayuso
2008-01-21 13:33       ` Max Kellermann
2008-01-21 13:52         ` Pablo Neira Ayuso

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=20080121062059.GA3995@swift.blarg.de \
    --to=max@duempel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.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.