Hey Andrew,
all list-adds and list-removes do a kref_get or kref_put respectively, but that probably was not very clear. As soon as a refence is created (by linking into the hash or a list), we should also kref_get() it.
Please find attached a new version of this patch with separate send_list_add/remove functions which include the put/get functions. I've also added the kref_put/get call to the send_list loop again. It should look more balanced now. Tested in my 9 qemu setup again, no memory leaks or crashes but i only did a very quick test (15 minutes)
best regards, Simon
On Mon, Mar 01, 2010 at 06:59:55AM +0100, Andrew Lunn wrote:
Hi Simon
@@ -503,15 +550,23 @@ unsigned long flags;
spin_lock_irqsave(&vis_hash_lock, flags);
- purge_vis_packets();
- if (generate_vis_packet() == 0)
if (generate_vis_packet() == 0) { /* schedule if generation was successful */
kref_get(&my_vis_info->refcount);list_add_tail(&my_vis_info->send_list, &send_list);
}
list_for_each_entry_safe(info, temp, &send_list, send_list) {
spin_unlock_irqrestore(&vis_hash_lock, flags);send_vis_packet(info);spin_lock_irqsave(&vis_hash_lock, flags);list_del_init(&info->send_list);
send_vis_packet(info);
kref_put(&info->refcount, free_info);Although you say this works, i just looks wrong. It looks unbalanced. Where is the kref_put for my_vis_info->refcount? There is a kref_put inside what looks like a loop, but no kref_get inside the loop.
Can you explain this is more detail? Can we make it look correctly balanced?
Thanks Andrew