I'm facing with some problems with my list. I'll explain in detail:
I have a struct with fields that represent counters and also I have the list. These two elements are accessed from various points in my code:
The struct and the list are accessed from user context. The counters are incremented and a new element is added to the list. This is done only when a certain process is specified by the user. This code looks like this:
if (trace_enabled) { /*Checks if the current process is the user-specified one*/
process_stats_inc_counter(&my_ps, 1);
attach_to_list(1);
}
Both functions does this work inside a critical section:
down_interruptible(&process_lock);
/*Increment counters*/
up(&process_lock);
down_interruptible(&process_lock);
/*Add element to list*/
up(&process_lock);
When adding the new element to the list, I check if the list is 200 elements long. If it is, I remove the "last" element. This is the only point where I remove an element from outside the clearing function.
When the user specifies the new pid I want to clear these data structures. The counters struct is cleared without problems, but I get an oops when trying to free the list:
down_interruptible(&process_lock);
/*Setting counters to 0
...
*/
list_for_each_entry_safe(p, my_node, &nodeList, list) {
list_del(&my_node->list);
kfree(my_node);
}
up(&process_lock);
If I comment this loop, the module doesn't crash so I think I'm doing (again...) something wrong with the list. This function is called from a procfs callback (where the user specifies the pid to trace).
list_for_each_entry_safe protects the list against removing, but no against adding, so I tried with the semaphore.
Any Ideas?
... my apologies for the length of the mail
On 7/11/06, Greg KH <greg@xxxxxxxxx> wrote:
On Tue, Jul 11, 2006 at 06:09:38PM +0200, Fernando Apestegu?a wrote:
> Hi,
>
> I would like to know how to free a list_head.
>
> I have almost 200 elements and I'm wonder if I must traverse the list for
> all the elements and do:
>
> list_for_each(...){
> my_entry=list_entry(....)
> list_del(my_entry)
> kfree(my_entry)
> }
>
> Should I do this in this way?
No, try this instead:
list_for_each_entry_safe(...) {
list_del(my_entry);
kfree(my_entry);
}
The main point is the _safe usage if you are traversing a list and
deleting items from it at the same time.
Hope this helps,
greg k-h
