>>> Ingo Molnar <[email protected]> 16.05.06 17:05 >>>
>* Jan Beulich <[email protected]> wrote:
>> +#ifdef CONFIG_STACK_UNWIND
>> +#include <asm/unwind.h>
>> +#else
>> +#include <asm-generic/unwind.h>
>> +#endif
>
>this wants to become include/linux/unwind.h?
Not really, at least not until IA64 and PARISC get adopted to the same (architecture independent) interface.
>> +#ifdef MODULE_UNWIND_INFO
>> +#include <asm/unwind.h>
>> +#endif
>
>this too could then include <linux/unwind.h>
As above.
>> +DEFINE_SPINLOCK(table_lock);
>
>static?
Oh, yes.
>> +static struct unwind_table *
>> +find_table(unsigned long pc)
>> +{
>> + int old_removals;
>> + struct unwind_table *table = NULL;
>> +
>> + do {
>> + if (table)
>> + atomic_dec(&table->users);
>> + old_removals = atomic_read(&removals);
>
>racy? wants to become rcu?
I don't think so. As far as I can tell, this isn't going to be a problem, it may just result in an extra, normally
unneeded, re-run of the loop.
>> + spin_lock(&table_lock);
>
>spin_lock_irq?
Why?
>> + if (init_only && table == last_table) {
>> + table->init.pc = 0;
>> + table->init.range = 0;
>> + return;
>> + }
>
>SMP and PREEMPT unsafe.
I don't think so, given that this can be called only from the module loader. As Andi pointed out elsewhere, it may even
be unnecessary to do the locking at all.
>> + spin_lock(&table_lock);
>
>spin_lock_irq().
Again, why?
>> + if (table) {
>> + while (atomic_read(&table->users) || atomic_read(&lookups))
>> + msleep(1);
>> + kfree(table);
>> + }
>
>ugh!
???
>> +//todo case DW_CFA_def_cfa_expression:
>> +//todo case DW_CFA_expression:
>> +//todo case DW_CFA_val_expression:
>
>hm?
This means what it says - it needs to be done, and I have no clear understanding of how these expressions are to be
treated, as I've never seen them in use anywhere.
>> +{
>> + info->task = current;
>> + arch_unwind_init_running(info, callback, arg);
>> + return 0;
>
>newline before the return. (this happens in a couple of other places
>too)
Surely can do that, although I don't see why this should be needed in functions this small.
Jan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
