2020-01-30 15:28:43

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v4 5/6] console: Introduce ->exit() callback

Some consoles might require special operations on unregistering.
For instance, serial console, when registered in the kernel,
keeps power on for entire time, until it gets unregistered.
Example of use:

->setup(console):
pm_runtime_get(...);

->exit(console):
pm_runtime_put(...);

For such cases to have a balance we would provide ->exit() callback.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v4: Don't rely on CON_ENABLED at all (Sergey, Petr), update commit message (Petr)
include/linux/console.h | 1 +
kernel/printk/printk.c | 4 ++++
2 files changed, 5 insertions(+)

diff --git a/include/linux/console.h b/include/linux/console.h
index f33016b3a401..54759ad0c36b 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -148,6 +148,7 @@ struct console {
struct tty_driver *(*device)(struct console *, int *);
void (*unblank)(void);
int (*setup)(struct console *, char *);
+ void (*exit)(struct console *);
int (*match)(struct console *, char *name, int idx, char *options);
short flags;
short index;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 932345e6cd71..0117d4d92a8e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2850,6 +2850,10 @@ int unregister_console(struct console *console)
console->flags &= ~CON_ENABLED;
console_unlock();
console_sysfs_notify();
+
+ if (!res && console->exit)
+ console->exit(console);
+
return res;
}
EXPORT_SYMBOL(unregister_console);
--
2.24.1


2020-01-31 01:35:29

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] console: Introduce ->exit() callback

On (20/01/30 17:25), Andy Shevchenko wrote:
[..]
> diff --git a/include/linux/console.h b/include/linux/console.h
> index f33016b3a401..54759ad0c36b 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -148,6 +148,7 @@ struct console {
> struct tty_driver *(*device)(struct console *, int *);
> void (*unblank)(void);
> int (*setup)(struct console *, char *);
> + void (*exit)(struct console *);

Should console ->exit() be able to return an error code?

> int (*match)(struct console *, char *name, int idx, char *options);
> short flags;
> short index;
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 932345e6cd71..0117d4d92a8e 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2850,6 +2850,10 @@ int unregister_console(struct console *console)
> console->flags &= ~CON_ENABLED;
> console_unlock();
> console_sysfs_notify();
> +
> + if (!res && console->exit)
> + console->exit(console);
> +
> return res;
> }

I would probably push it a bit further (I posted this snippet in another
thread). If console is not on the list then there is nothing for us to do
and sysfs notify is pointless.

---

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0117d4d92a8e..7116e421210b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2837,7 +2837,13 @@ int unregister_console(struct console *console)
}
}

- if (!res && (console->flags & CON_EXTENDED))
+ if (res) {
+ console->flags &= ~CON_ENABLED;
+ console_unlock();
+ return res;
+ }
+
+ if (console->flags & CON_EXTENDED)
nr_ext_console_drivers--;

/*
@@ -2851,7 +2857,7 @@ int unregister_console(struct console *console)
console_unlock();
console_sysfs_notify();

- if (!res && console->exit)
+ if (console->exit)
console->exit(console);

return res;

2020-01-31 11:29:35

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] console: Introduce ->exit() callback

On Fri, Jan 31, 2020 at 10:31:54AM +0900, Sergey Senozhatsky wrote:
> On (20/01/30 17:25), Andy Shevchenko wrote:
> [..]
> > diff --git a/include/linux/console.h b/include/linux/console.h
> > index f33016b3a401..54759ad0c36b 100644
> > --- a/include/linux/console.h
> > +++ b/include/linux/console.h
> > @@ -148,6 +148,7 @@ struct console {
> > struct tty_driver *(*device)(struct console *, int *);
> > void (*unblank)(void);
> > int (*setup)(struct console *, char *);
> > + void (*exit)(struct console *);
>

> Should console ->exit() be able to return an error code?

Let's do it!

> > int (*match)(struct console *, char *name, int idx, char *options);
> > short flags;
> > short index;
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 932345e6cd71..0117d4d92a8e 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2850,6 +2850,10 @@ int unregister_console(struct console *console)
> > console->flags &= ~CON_ENABLED;
> > console_unlock();
> > console_sysfs_notify();
> > +
> > + if (!res && console->exit)
> > + console->exit(console);
> > +
> > return res;
> > }
>
> I would probably push it a bit further (I posted this snippet in another
> thread). If console is not on the list then there is nothing for us to do
> and sysfs notify is pointless.

I didn't see post in the other thread, but I suppose that this snipped is
for patch 4 in the series, correct?

>
> ---
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 0117d4d92a8e..7116e421210b 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2837,7 +2837,13 @@ int unregister_console(struct console *console)
> }
> }
>
> - if (!res && (console->flags & CON_EXTENDED))
> + if (res) {
> + console->flags &= ~CON_ENABLED;
> + console_unlock();
> + return res;
> + }
> +
> + if (console->flags & CON_EXTENDED)
> nr_ext_console_drivers--;
>
> /*
> @@ -2851,7 +2857,7 @@ int unregister_console(struct console *console)
> console_unlock();
> console_sysfs_notify();
>
> - if (!res && console->exit)
> + if (console->exit)

> console->exit(console);
>
> return res;

...then something like

return console->exit(console);

return 0;

...or...

res = console->exit(console);

Which one is preferable?

--
With Best Regards,
Andy Shevchenko


2020-02-01 01:09:20

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] console: Introduce ->exit() callback

On (20/01/31 13:27), Andy Shevchenko wrote:
[..]
> >
> > I would probably push it a bit further (I posted this snippet in another
> > thread). If console is not on the list then there is nothing for us to do
> > and sysfs notify is pointless.
>
> I didn't see post in the other thread, but I suppose that this snipped is
> for patch 4 in the series, correct?

No worries! Yes, for v4.

[..]
> > - if (!res && console->exit)
> > + if (console->exit)
>
> > console->exit(console);
> >
> > return res;
>
> ...then something like
>
> return console->exit(console);
>
> return 0;
>
> ...or...
>
> res = console->exit(console);
>
> Which one is preferable?

I don't really have preferences here, so up to you guys.

-ss

2020-02-03 15:04:39

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] console: Introduce ->exit() callback

On Sat, Feb 01, 2020 at 10:08:04AM +0900, Sergey Senozhatsky wrote:
> On (20/01/31 13:27), Andy Shevchenko wrote:

> > > I would probably push it a bit further (I posted this snippet in another
> > > thread). If console is not on the list then there is nothing for us to do
> > > and sysfs notify is pointless.
> >
> > I didn't see post in the other thread, but I suppose that this snipped is
> > for patch 4 in the series, correct?
>
> No worries! Yes, for v4.

I guess it was v5 be mentioned above, nevertheless, just sent v5.

--
With Best Regards,
Andy Shevchenko