All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sven Eckelmann <sven@narfation.org>
To: b.a.t.m.a.n@lists.open-mesh.org
Subject: Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: fix visualization output without neighbors on the primary interface
Date: Mon, 07 May 2012 08:43:06 +0200	[thread overview]
Message-ID: <7772207.d3Er5W7jJA@bentobox> (raw)
In-Reply-To: <4FA75113.40401@universe-factory.net>

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Monday, May 07, 2012 06:35:31 AM Matthias Schiffer wrote:
[....]
> I have some questions about the code though:
> 
> - Is there any reason vis_seq_print_text() allocates a buffer at all instead
> of just printing the data directy into the seq_file? Looking at the
> seq_printf implementation, there doesn't seem to be a problem calling it
> while holding the lock.

This is something which came from an old... old... old implementation. It 
didn't use debugfs and seq_printf and therefore stupid tricks had to be used. 
Actually, the current implementation is broken and has to be changed (but no 
one wanted to touch the vis code).

> - In many places in the vis code
> hlist_for_each_entry_rcu() is used to iterate over the hash lists, even
> though all access to vis_hash is guarded by the vis_hash_lock, so it seems
> to be okay to just use hlist_for_each_entry(). In some functions,
> vis_seq_print_text() being one of them, rcu_read_lock/unlock pairs could be
> removed as well with this change. Do I overlook something?

I think it would be better to reduce the spin-locking and change the code to 
use rcu_read_lock. But maybe we have to think a lot about the data structures 
to generate consistent output... so maybe it is not possible (when also 
wanting it implemented in an efficient way).

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2012-05-07  6:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-05 15:05 [B.A.T.M.A.N.] [PATCH] batman-adv: fix visualization output without neighbors on the primary interface Matthias Schiffer
2012-05-05 15:29 ` Sven Eckelmann
2012-05-05 15:49   ` Matthias Schiffer
2012-05-05 15:51     ` [B.A.T.M.A.N.] [PATCHv2] " Matthias Schiffer
2012-05-06 20:14       ` Simon Wunderlich
2012-05-07  4:35         ` Matthias Schiffer
2012-05-07  6:40           ` Marek Lindner
2012-05-07 11:10             ` Matthias Schiffer
2012-05-07 11:28               ` Sven Eckelmann
2012-05-08  6:04               ` Marek Lindner
2012-05-08 12:51                 ` Matthias Schiffer
2012-05-08 20:52                   ` Guido Iribarren
2012-05-09 11:33                     ` Marek Lindner
2012-05-09 16:10                     ` Martin Hundebøll
2012-05-10 19:47                       ` Matthias Schiffer
2012-05-10 20:19                         ` Marek Lindner
2012-05-10 20:46                           ` Matthias Schiffer
2012-05-07  6:43           ` Sven Eckelmann [this message]
2012-05-07  6:43       ` Marek Lindner

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=7772207.d3Er5W7jJA@bentobox \
    --to=sven@narfation.org \
    --cc=b.a.t.m.a.n@lists.open-mesh.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.