lkml.org 
[lkml]   [2007]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
DateTue, 19 Jun 2007 19:04:19 +0900
FromKeiichi KII <>
SubjectRe: [RFC][PATCH -mm take5 4/7] using symlink for the net_device
Hello Satyam,

> and this is why we have to use the dual-list mechanism to react to the net
> device rename. This isn't so obvious, a comment at the point where you
> declare modify_target_list would be nice? (BTW temporary_list would be
> a better name for that, IMO)

All right, my patches are short of comments. So, I will add comments
to the ambiguous codes.

> Ok, so reading through the code makes it obvious that this mutex is used
> to protect against the following race:
>
> Thread #1 Thread #2
> ========= =========
>
> [ NETDEV_CHANGENAME notifier ] [ ioctl(NETCON_REMOVE_TARGET) ]
>
> netconsole_event()
> move from target_list to temp list
> work on temp list
> kobject_unregister()
> -> release_target()
> -> remove_target()
> move back to target_list
>
> Which would mean a deleted/removed target added back => *boom*
>
> But, the race still hasn't been closed properly!
>
> You're taking the mutex only around "work on temp list" which is
> insufficient, you need to ensure atomicity inside netconsole_event()
> _completely_ like this (renaming netdev_change_sem to
> netdev_changename_mtx):

After the target moves from target_list to temporary_list,
the kobject_unregister() of possible raced target isn't called
in ioctl(NETCON_REMOVE_TARGET) because the target_list doesn't contain
the target .

I have the wrong idea?

>> +static char *make_netdev_class_name(char *netdev_name)
>> +{
>> + char *name;
>> +
>> + name = kasprintf(GFP_KERNEL, "net:%s", netdev_name);
>
> Why the "net:" prefix in the filename?

Because I drew upon dev_change_name() method in net/core/dev.c.
The device_rename() in the above function makes use of same prefix
related to netdev.

>> static int setup_target_sysfs(struct netconsole_target *nt)
>> {
>> + int retval = 0;
>> + char *name;
>> +
>> kobject_set_name(&nt->obj, "port%d", nt->id);
>> nt->obj.parent = &netconsole_miscdev.this_device->kobj;
>> nt->obj.ktype = &target_ktype;
>> - return kobject_register(&nt->obj);
>> + retval = kobject_register(&nt->obj);
>> + name = make_netdev_class_name(nt->np.dev_name);
>> + if (!name)
>> + return -ENOMEM;
>
> Just call kasprintf() directly, why the obfuscation?
>

I drew upon dev_change_name() method in net/core/dev.c.

Thanks
--
Keiichi KII
NEC Corporation OSS Platform Development Division
E-mail: k-keiichi@bx.jp.nec.com


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-06-19 12:07    [from the cache]
©2003-2010