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
signature.asc
Description: This is a digitally signed message part.
