On Wed 2022-10-19 17:01:24, John Ogness wrote:
> Replace the open coded single linked list with a hlist so a conversion
> to SRCU protected list walks can reuse the existing primitives.
>
> Signed-off-by: John Ogness <john.ogness@xxxxxxxxxxxxx>
Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
Just one nit below.
> @@ -3229,32 +3244,30 @@ int unregister_console(struct console *console)
> if (res > 0)
> return 0;
>
> - res = -ENODEV;
> console_lock();
> - if (console_drivers == console) {
> - console_drivers=console->next;
> - res = 0;
> - } else {
> - for_each_console(con) {
> - if (con->next == console) {
> - con->next = console->next;
> - res = 0;
> - break;
> - }
> - }
> +
> + /* Disable it unconditionally */
> + console->flags &= ~CON_ENABLED;
> +
> + if (hlist_unhashed(&console->node)) {
> + res = -ENODEV;
> + goto out_unlock;
Nit: It might make sense to replace this with:
console_unlock();
return -ENODEV;
This is the only code path using the extra goto target.
It is just an idea. I do not resist on this change.
> }
>
> - if (res)
> - goto out_disable_unlock;
> + hlist_del_init(&console->node);
>
> /*
> + * <HISTORICAL>
> * If this isn't the last console and it has CON_CONSDEV set, we
> * need to set it on the next preferred console.
> + * </HISTORICAL>
> + *
> + * The above makes no sense as there is no guarantee that the next
> + * console has any device attached. Oh well....
> */
> - if (console_drivers != NULL && console->flags & CON_CONSDEV)
> - console_drivers->flags |= CON_CONSDEV;
> + if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV)
> + console_first()->flags |= CON_CONSDEV;
>
> - console->flags &= ~CON_ENABLED;
> console_unlock();
> console_sysfs_notify();
>
> @@ -3263,10 +3276,8 @@ int unregister_console(struct console *console)
>
> return res;
>
> -out_disable_unlock:
> - console->flags &= ~CON_ENABLED;
> +out_unlock:
> console_unlock();
> -
> return res;
> }
> EXPORT_SYMBOL(unregister_console);
Best Regards,
Petr
