All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: The list for a Better Approach To Mobile Ad-hoc Networking
	<b.a.t.m.a.n@lists.open-mesh.net>
Subject: Re: [B.A.T.M.A.N.] [PATCH] Removing the big batman lock
Date: Mon, 30 Nov 2009 11:09:02 +0100	[thread overview]
Message-ID: <20091130100902.GE10269@lunn.ch> (raw)
In-Reply-To: <20091129200950.GA19275@pandem0nium>

On Sun, Nov 29, 2009 at 09:09:50PM +0100, Simon Wunderlich wrote:

> I did some testing, including loading, unloading, killing individual
> nodes etc, which seems to be clean so far. However there might be 
> more race conditions introduced by this large patch, and i'm therefore
> requesting a careful review before committing.

Hi Simon

I've not done a careful review yet, just a quick look.

Two idea for improvements:

In the purge code, add some debug code which looks at the refcount
value. If it is not between 1 and 5, prinkt() a warning what there is
probably a missing refcount operation. Since the purge code does not
run very often, it should not add much overhead, yet it is still a
useful debug tool.

The following bit of code happens quite a lot:

while (NULL != (orig_node = orig_hash_iterate(&hashit, orig_node))) {


There is also a comment about having to free hashit, if you don't
iterate to the end of the hash. How about refactoring this, more like
the linux list.h.

Make hashit a stack variable, with an init macro:

#define HASHIT(name) struct hash_it_t name = { .index = -1, .bucket = NULL, \
	                                       .prev_bucket=NULL,           \
 					       .first_bucket=NULL }

and a macro for iterating over the hash
    
    HASHIT(hashit);

    orig_hash_for_each(orig_node, hashit) {

    foo(orig_node);
    }


    Andrew

  reply	other threads:[~2009-11-30 10:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-29 20:09 [B.A.T.M.A.N.] [PATCH] Removing the big batman lock Simon Wunderlich
2009-11-30 10:09 ` Andrew Lunn [this message]
2009-12-13 21:17   ` Simon Wunderlich
2009-12-03  1:31 ` Linus Lüssing
2009-12-03  6:00   ` Andrew Lunn
2009-12-04 17:39     ` Sven Eckelmann
2009-12-03 12:13 ` [B.A.T.M.A.N.] [PATCHv2] [batman-adv] " Simon Wunderlich
2009-12-03 13:11   ` Andrew Lunn
2009-12-03 13:21     ` Sven Eckelmann
2009-12-04 13:55   ` Sven Eckelmann
2009-12-05 15:27   ` [B.A.T.M.A.N.] [PATCHv3] " Simon Wunderlich

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=20091130100902.GE10269@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=b.a.t.m.a.n@lists.open-mesh.net \
    /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.