All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/1] Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0
@ 2019-02-25 16:43 Yifeng Li
  2019-02-25 16:43 ` [RFC 1/1] tty: vt.c: " Yifeng Li
  0 siblings, 1 reply; 4+ messages in thread
From: Yifeng Li @ 2019-02-25 16:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Jiri Slaby, Nicolas Pitre, Adam Borowski,
	Mikulas Patocka, Alexander Potapenko, Yifeng Li, Mike Frysinger,
	Daniel Vetter

Previously, in the userspace, it was possible to use the "setterm" command
from util-linux to blank the VT console by default, using the following
command.

# setterm -blank force

According to the man page,

> The force option keeps the screen blank even if a key is pressed.

It was implemented by calling TIOCL_BLANKSCREEN.

	case BLANKSCREEN:
		ioctlarg = TIOCL_BLANKSCREEN;
		if (ioctl(STDIN_FILENO, TIOCLINUX, &ioctlarg))
			warn(_("cannot force blank"));
		break;

However, after Linux 4.12, this command ceased to work anymore, which is
unexpected. By inspecting the kernel source, it shows that the issue was
triggered by the side-effect from commit a4199f5e ("tty: Disable default
console blanking interval").

The console blanking is implemented by function do_blank_screen(). It can
be called by the inactive timer "console_timer" with the time specified by
"blankinterval", or by the IOCTL call TIOCL_BLANKSCREEN. The variable
"blank_state" is used to indicate the current status of console blanking,
it can have three possible values, blank_off, blank_normal_wait, or
blank_vesa_wait.

When do_blank_screen() is called by "console_timer" or TIOCL_BLANKSCREEN:

1. If blank_state == vesa_off_interval, it means the console was already
soft-blanked previously, and now it's the time to use VESA to put the CRT
into a deeper sleep mode. When we're done, blank_state is set to blank_off,
means the screen is now off, return.

2. If blank_state == blank_normal_wait, it means the console was previously on,
waiting to be blanked by the inactive timer. It will set blank_state to
"blank_off", means the screen is now off, and it then blanks the console. The
exception is that, it also checks if VESA powersaving is on, if so, blank_state
will be set to "blank_vesa_wait" instead, and the timer "console_timer" is
reinitialized to call do_blank_screen() again after "vesa_off_interval", for
VESA blanking, see Step 1.

3. If blank_state != blank_normal_wait at this point, it means the screen is
neither on, or VESA blanked - it must be blanked already, so return.

The problem is, "blank_state" will be initialized to "blank_normal_wait" in
con_init(), if AND ONLY IF ("blankinterval" > 0). If "blankinterval" is 0,
"blank_state" will be "blank_off" (== 0), and a call to do_blank_screen() will
always abort, per Step 3. Even if a forced blanking is required from the user
by calling TIOCL_BLANKSCREEN, the console won't be blanked.

In other words, the screen cannot be blanked if autoblanking is disabled,
unless we set a "blankinterval" beforehand, and the confusingly, the ioctl
will not even return an error code.

This behavior is unexpected from a user's point-of-view, since it's not
mentioned in any documentation. The setterm man page suggests it will always
work, and the kernel comments in uapi/linux/tiocl.h says

> #define TIOCL_BLANKSCREEN 14 /* keep screen blank even if a key is pressed */

The absence of an error is also misleading.

Personally, I consider it is either: (a) a bug of the implementation,
"blankinterval" shouldn't be a kill switch for manual console blanking, (b) or a
bug of inadequate documentation.

In the following patch, I suggest making a minor change to the console blanking
logic. We introduce a 4th "blank_state" - "blank_normal_notimer", it indicates
the console can be blanked, but not automatically by a timer. Then, we made a
change to "con_init()" - if (blankinterval == 0), "blank_state" will be set to
"blank_normal_notimer" (we have similar changes to "do_unblank_screen()" and
"poke_blanked_console()"). Finally, we change Step 3 in do_blank_screen() - now
it will return if "blank_state" is neither "blank_normal_wait" nor
"blank_normal_notimer", thus allowing the console to be blanked even if there's
no timed autoblanking.

Dear maintainers, please review if my proposed chance is appropriate.

Thanks,
Tom Li

Yifeng Li (1):
  tty: vt.c: Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval
    == 0

 drivers/tty/vt/vt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC 1/1] tty: vt.c: Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0
  2019-02-25 16:43 [RFC 0/1] Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0 Yifeng Li
@ 2019-02-25 16:43 ` Yifeng Li
  2019-02-25 18:29   ` Nicolas Pitre
  0 siblings, 1 reply; 4+ messages in thread
From: Yifeng Li @ 2019-02-25 16:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Jiri Slaby, Nicolas Pitre, Adam Borowski,
	Mikulas Patocka, Alexander Potapenko, Yifeng Li, Mike Frysinger,
	Daniel Vetter

In vt.c, "blank_state" will be initialized to "blank_normal_wait" in
con_init() if AND ONLY IF ("blankinterval" > 0). If "blankinterval" is 0,
"blank_state" will be "blank_off" (== 0), and a call to do_blank_screen()
will always abort. Even if a forced blanking is required from the user by
calling TIOCL_BLANKSCREEN, the console won't be blanked.

In this commit,  we introduce a 4th "blank_state" - "blank_normal_notimer",
it indicates the console can be blanked, but not automatically by a timer.
Then, we made a change to "con_init()" - if (blankinterval == 0),
"blank_state" will be set to "blank_normal_notimer" (we have similar
changes to "do_unblank_screen()" and "poke_blanked_console()"). Finally,
we change do_blank_screen() - now it will return if "blank_state" is
neither "blank_normal_wait" nor "blank_normal_notimer", thus allowing
the console to be blanked even if there's no timed autoblanking.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 drivers/tty/vt/vt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 41ec8e5010f3..ec84bc2868e1 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -238,6 +238,7 @@ enum {
 	blank_off = 0,
 	blank_normal_wait,
 	blank_vesa_wait,
+	blank_normal_notimer, /* no auto blanking, only manual blanking */
 };
 
 /*
@@ -3334,6 +3335,8 @@ static int __init con_init(void)
 	if (blankinterval) {
 		blank_state = blank_normal_wait;
 		mod_timer(&console_timer, jiffies + (blankinterval * HZ));
+	} else {
+		blank_state = blank_normal_notimer;
 	}
 
 	for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
@@ -4168,7 +4171,8 @@ void do_blank_screen(int entering_gfx)
 		return;
 	}
 
-	if (blank_state != blank_normal_wait)
+	if (blank_state != blank_normal_wait &&
+			blank_state != blank_normal_notimer)
 		return;
 	blank_state = blank_off;
 
@@ -4232,6 +4236,8 @@ void do_unblank_screen(int leaving_gfx)
 	if (blankinterval) {
 		mod_timer(&console_timer, jiffies + (blankinterval * HZ));
 		blank_state = blank_normal_wait;
+	} else {
+		blank_state = blank_normal_notimer;
 	}
 
 	console_blanked = 0;
@@ -4293,7 +4299,8 @@ void poke_blanked_console(void)
 	else if (blankinterval) {
 		mod_timer(&console_timer, jiffies + (blankinterval * HZ));
 		blank_state = blank_normal_wait;
-	}
+	} else if (!blankinterval)
+		blank_state = blank_normal_notimer;
 }
 
 /*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC 1/1] tty: vt.c: Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0
  2019-02-25 16:43 ` [RFC 1/1] tty: vt.c: " Yifeng Li
@ 2019-02-25 18:29   ` Nicolas Pitre
  2019-02-26 11:20     ` Tom Li
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Pitre @ 2019-02-25 18:29 UTC (permalink / raw)
  To: Yifeng Li
  Cc: linux-kernel, Greg Kroah-Hartman, Jiri Slaby, Adam Borowski,
	Mikulas Patocka, Alexander Potapenko, Mike Frysinger,
	Daniel Vetter

On Tue, 26 Feb 2019, Yifeng Li wrote:

> In vt.c, "blank_state" will be initialized to "blank_normal_wait" in
> con_init() if AND ONLY IF ("blankinterval" > 0). If "blankinterval" is 0,
> "blank_state" will be "blank_off" (== 0), and a call to do_blank_screen()
> will always abort. Even if a forced blanking is required from the user by
> calling TIOCL_BLANKSCREEN, the console won't be blanked.
> 
> In this commit,  we introduce a 4th "blank_state" - "blank_normal_notimer",
> it indicates the console can be blanked, but not automatically by a timer.
> Then, we made a change to "con_init()" - if (blankinterval == 0),
> "blank_state" will be set to "blank_normal_notimer" (we have similar
> changes to "do_unblank_screen()" and "poke_blanked_console()"). Finally,
> we change do_blank_screen() - now it will return if "blank_state" is
> neither "blank_normal_wait" nor "blank_normal_notimer", thus allowing
> the console to be blanked even if there's no timed autoblanking.
> 
> Signed-off-by: Yifeng Li <tomli@tomli.me>

Wouldn't this problem be fixed simply by the following:

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9646ff63e7..41b17c4b5a 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4151,8 +4151,6 @@ void do_blank_screen(int entering_gfx)
 		return;
 	}
 
-	if (blank_state != blank_normal_wait)
-		return;
 	blank_state = blank_off;
 
 	/* don't blank graphics */

I just can't find a logical reason for that conditional return.


Nicolas

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC 1/1] tty: vt.c: Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0
  2019-02-25 18:29   ` Nicolas Pitre
@ 2019-02-26 11:20     ` Tom Li
  0 siblings, 0 replies; 4+ messages in thread
From: Tom Li @ 2019-02-26 11:20 UTC (permalink / raw)
  To: Nicolas Pitre, linux-kernel
  Cc: Greg Kroah-Hartman, Jiri Slaby, Adam Borowski, Mikulas Patocka,
	Alexander Potapenko, Yifeng Li, Mike Frysinger, Daniel Vetter

> Wouldn't this problem be fixed simply by the following:
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 9646ff63e7..41b17c4b5a 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4151,8 +4151,6 @@ void do_blank_screen(int entering_gfx)
>  		return;
>  	}
>  
> -	if (blank_state != blank_normal_wait)
> -		return;
>  	blank_state = blank_off;
>  
>  	/* don't blank graphics */
> 
> I just can't find a logical reason for that conditional return.
> 
> 
> Nicolas

I see. I was concerned about the possibly unwanted effect if the function
is reentered, but it seems that there's no logic reason to worry at all.

I'll send a new patch according to your suggestion.

Thanks,
Tom Li

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-02-26 11:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-25 16:43 [RFC 0/1] Fix TIOCL_BLANKSCREEN VT console blanking if blankinterval == 0 Yifeng Li
2019-02-25 16:43 ` [RFC 1/1] tty: vt.c: " Yifeng Li
2019-02-25 18:29   ` Nicolas Pitre
2019-02-26 11:20     ` Tom Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.