On Tue, 2011-02-01 at 22:43 -0800, Mike Christie wrote:
> On 01/28/2011 06:04 PM, Robert Love wrote:
> > +static int fcoe_transport_create(const char *buffer, struct kernel_param *kp)
> > +{
> > + int rc = -ENODEV;
> > + struct net_device *netdev = NULL;
> > + struct fcoe_transport *ft = NULL;
> > + enum fip_state fip_mode = (enum fip_state)(long)kp->arg;
> > +
> > + if (!mutex_trylock(&ft_mutex))
> > + return restart_syscall();
>
> I was just wondering what is the reason for this. I see it in other
> network related sysfs code but no where else. Is there some networky reason?
I think this may be unnecessary. We initially added a rtnl_trylock in
fcoe_create/destroy (fcoe.ko's sysfs entry-points) with change
34ce27bcf96f5f366e1fa8c4729ffc8a55de4cc3. I cannot think of a good
reason why this is a mutex_trylock/restart_syscall and not just a simple
mutex_lock.
>
> > +
> > +#ifdef CONFIG_LIBFCOE_MODULE
> > + /*
> > + * Make sure the module has been initialized, and is not about to be
> > + * removed. Module parameter sysfs files are writable before the
> > + * module_init function is called and after module_exit.
> > + */
> > + if (THIS_MODULE->state != MODULE_STATE_LIVE)
> > + goto out_nodev;
> > +#endif
>
> It seems that this would be a problem for every driver that uses module
> params. Should the module code be getting a module refcount or doing
> this test?
I think the problem is that the sysfs entries are created and are
writable before the module is truly initialized, so we could start using
uninitialized variables. It looks like a global solution would be to not
add sysfs files (or at least don't make them writable) until the module
initialization has completed. I have no idea how feasible this is.
//Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
