All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew.lunn@ascom.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.] [batman-adv] VIS startup race condition fix	plus coding style?cleanup
Date: Thu, 16 Jul 2009 09:32:03 +0200	[thread overview]
Message-ID: <20090716073203.GS19071@ma.tech.ascom.ch> (raw)
In-Reply-To: <200907160200.31170.lindner_marek@yahoo.de>

> Sorry, the code review of this patch took a bit more time as it is a quite 
> long one and mixes several topics:
> * coding style cleaning
> * race condition
> * struct changes and their respective code modifications
> * moving a variable to proc.c
> 
> It would be very helpful if you split the patch into smaller parts before I 
> submit them. Is that feasible ?

I will try. It will probably end up in three patches, cleanup, race
fix, variable move. The struct changes are part of the race fix.

> Nevertheless, I dug through and noticed you modified the struct vis_info by 
> adding "struct vis_info_entry entries[130];". This would lead to an allocation 
> of nearly 1KB (130 * 7 Bytes) for every incoming vis packet. Is that 
> intended ? 

Yes, this is intentional, although maybe i've missed something subtle
in the code. The old code allocates one instance of vis_info in
vis_init() by kmalloc()'ing 1000 bytes. In generate_vis_packet() there
are checks to ensure we don't go over this 1000 byte limit. I don't
like this 1000 magic number and decided to clean it up. 

As you said, 130 * 7, is nearly 1K, so the overall memory usage should
not of changed. 

Now, looking deeper, there is something subtle i missed. When sending,
in send_vis_packet(), it calculates the packet length based on the
number of used entries and only sends that many bytes. So in
receive_server_sync_packet() which calls add_packet() it would only
kmalloc() as much space needed for the received packet. My code
allocated the maximum packet size. So i'm wasting space here. I will
rework this code.

> I also applied your coding style patches for bitarray.[ch] and hash.[ch]. 

Thanks

        Andrew

      reply	other threads:[~2009-07-16  7:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-14 16:37 [B.A.T.M.A.N.] [batman-adv] VIS startup race condition fix plus coding style cleanup Andrew Lunn
2009-07-15 18:00 ` Marek Lindner
2009-07-16  7:32   ` Andrew Lunn [this message]

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=20090716073203.GS19071@ma.tech.ascom.ch \
    --to=andrew.lunn@ascom.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.