Hi,
> I'm wondering, doesn't this take quite a long time?
> Too long to hold
> a spinlock for?
>
> Of course we need locking to prevent other module
> loads to modify the
> symbol table while we are checking this one, but to
> prevent the kernel
> from stalling everything else for a long time,
> wouldn't it be better
> to use a semaphore (we can sleep with those -
> right?) and an explicit
> schedule() call in the loop(s)? Or am I completely
> out in outere
> space with that thought?
>
Both at the time of loading a module and while
unloading a
module module_mutex is acquired. In the first place we
wer
e not planning to take the spinlock at all. But saw
that r
esolve_symbol does that and hence wasn't sure whether
the
lock should be acquired.
> Shouldn't this printk() be using KERN_ERROR ?
>
> printk(KERN_ERROR "%s: Duplicate Exported Symbol
> found in %s\n",
>
>
> > + strtab + index, mod->name);
> > + return -ENOEXEC;
> > +duplicate_gpl_sym:
> > + spin_unlock_irq(&modlist_lock);
> > + printk("%s: Duplicate Exported Symbol found in
> %s\n",
> > + strtab + index, mod->name);
> > + return -ENOEXEC;
> > +}
>
> Why 3 different exit paths? and 2 of them are even
> identical. Why not
> something like this instead? :
>
> {
> ...
> if (unlikely(value) {
> ret = -ENOEXEC;
> goto out;
> }
> ...
> out:
> spin_unlock_irq();
> if (ret)
> printk();
> return ret;
> }
>
We will send an updated patch with the modifications.
Thanks,
Anand
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
-
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]
