configfs: Create config_item from netconsole
Subsystems (2)
Right now there is a limitation in netconsole, where it is impossible to disable or modify the target created from the command line parameter. (netconsole=...).
"netconsole" cmdline parameter sets the remote IP, and if the remote IP changes, the machine needs to be rebooted (with the new remote IP set in the command line parameter).
I am planning to reuse the dynamic netconsole mechanism for the 'command line' target, i.e., create a cmdline configfs_item for the "command line" target, so, the user can modify the "command line" target in configfs in runtime. Something as :
echo 0 > /sys/kernel/config/netconsole/cmdline/enabled echo <new-IP> > /sys/kernel/config/netconsole/cmdline/remote_ip echo 1 > /sys/kernel/config/netconsole/cmdline/enabled
I didn't find a configfs API to register a configfs_item into a config_group. Basically the make_item() callbacks are called once the inode is created by the user at mkdir(2) time, but now I need to create it at the driver initialization.
Should I create a configfs_register_item() to solve this problem?
Thanks!
Quoted 8 lines · Breno Leitao
Right now there is a limitation in netconsole, where it is impossible to disable or modify the target created from the command line parameter. (netconsole=...).
"netconsole" cmdline parameter sets the remote IP, and if the remote IP changes, the machine needs to be rebooted (with the new remote IP set in the command line parameter).
Quoted 8 lines
I am planning to reuse the dynamic netconsole mechanism for the 'command line' target, i.e., create a
cmdlineconfigfs_itemfor the "command line" target, so, the user can modify the "command line" target in configfs in runtime. Something as :echo 0 >
/sys/kernel/config/netconsole/cmdline/enabledecho <new-IP> >/sys/kernel/config/netconsole/cmdline/remote_ipecho 1 >/sys/kernel/config/netconsole/cmdline/enabled
Note that the netconsole= command line parameter can specify more than one network console, split by semicolons. Anything you do here has to be responsive to this. So you can't create a single cmdline entry.
Quoted 6 lines
I didn't find a configfs API to register a
configfs_iteminto aconfig_group. Basically themake_item()callbacks are called once the inode is created by the user at mkdir(2) time, but now I need to create it at the driver initialization.Should I create a
configfs_register_item()to solve this problem?
It's an express philosophy of configfs that all lifetimes are controlled by userspace, which is why we don't have such a facility. If hch wants to change this, I defer to his judgement. But I don't think it is necessary.
What I would do instead is check whether a mkdir(2) call is trying to reference a command line entry. If so, attach it to the existing entry rather than creating a new one.
Currently, alloc_param_target() initializes netconsole_target.item to zeros. The item is never used by parameter-created targets. Step one would be to give it a name. So in init_netconsole(), right after alloc_param_target(), initialize nt->item just like we do in make_netconsole_target(). So something like:
```
+ #ifdef CONFIG_NETCONOLE_DYNAMIC
+ char target_name[16];
+ int target_count = 0;
+ #endif
while ((target_config = strsep(&input, ";"))) {
nt = alloc_param_target(target_config);
if (IS_ERR(nt)) {
err = PTR_ERR(nt);
goto fail;
}
+ #ifdef CONFIG_NETCONSOLE_DYNAMIC
+ snprintf(target_name, 16, "cmdline", target_count);
+ config_item_init_type_name(&nt->item, target_name,
+ &netconsole_target_type);
+ target_count++;
+ #endif
+
/* Dump existing printks when we register */
if (nt->extended) {
extended = true;
```Then, later in make_netconsole_target(), rather than blindly inserting the new netconsole_target in the list, you can check if the name is already present and use that. Here's some ugly pseudocode:
```
spin_lock_irqsave(&target_list_lock, flags);
list_for_each_entry(tmp, &target_list, list) {
if (!strcmp(tmp->item.name, nt->item.name)) {
existing = tmp;
break;
}
}
if (existing) {
to_free = nt;
nt = existing;
} else {
list_add(&nt->list, &target_list);
}
spin_unlock_irqrestore(&target_list_lock, flags);
if (to_free)
kfree(to_free);
return &nt->item;
}
```In this fashion, each console created on the command line will get a name of cmdline0, cmdline1, etc. They will not be part of the configfs tree. If the user comes along later and says mkdir /sys/kernel/config/netconsole/cmdline1, the existing cmdline1 console will be attached to the configfs tree. The user is then free to disable and reconfigure the device.
Note that this behavior cannot be triggered for netconsoles already created by configfs. If I try to mkdir /sys/kernel/config/netconsole/mydev twice, the second command will get -EEXISTS in filesystem code long before it reaches the netconsole code. Only when someone makes a config item that matches the console names can we traverse this code. Thus, matching the name is safe.
There would, of course, be some other corner cases to handle. Do we allow dynamic names that look like command-line names if no command-line parameter exists? Does rmdir /sys/kernel/config/netconsole/cmdline0 actually delete the command-line console entry, or does it return -EBUSY? And so on.
Thanks,
Joel
Signature · 5 lines
"When ideas fail, words come in very handy."
- Goethe
http://www.jlbec.org/
[email protected]Hello Joel,
Quoted 8 lines · Joel Becker
Quoted 1 line · Breno Leitao
Should I create a
configfs_register_item()to solve this problem?It's an express philosophy of configfs that all lifetimes are controlled by userspace, which is why we don't have such a facility. If hch wants to change this, I defer to his judgement. But I don't think it is necessary.
I am happy the suggestion you gave, so, no need to change this philosophy.
Quoted 6 lines
In this fashion, each console created on the command line will get a name of
cmdline0,cmdline1, etc. They will not be part of the configfs tree. If the user comes along later and saysmkdir /sys/kernel/config/netconsole/cmdline1, the existingcmdline1console will be attached to the configfs tree. The user is then free to disable and reconfigure the device.
I am happy with this approach.
Quoted 3 lines
There would, of course, be some other corner cases to handle. Do we allow dynamic names that look like command-line names if no command-line parameter exists?
I'd say so, I think cmdline targets shouldn't be special if there are not netconsole targets defined at boot time.
Quoted 3 lines
Does
rmdir /sys/kernel/config/netconsole/cmdline0actually delete the command-line console entry, or does it return-EBUSY? And so on.
I've looked at the code and tested, it cmdline0could be removed as any other target entry. On top of that, the user can create a new cmdline0 entry, and, it will be set with the default values for the attributes.
I am planning to send a patch based on what you suggested, and we can continue the discuss there.
Thanks for the _very_ detailed feedback!
Breno