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