2023-01-12 08:22:35

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check()

VC_UNI_SCREEN_DEBUG is always defined as 0, so this code is never
executed. Drop it along with VC_UNI_SCREEN_DEBUG.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/vt/vt.c | 40 ----------------------------------------
1 file changed, 40 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 981d2bfcf9a5..4b804665c51f 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -323,8 +323,6 @@ void schedule_console_callback(void)
#define get_vc_uniscr(vc) vc->vc_uni_screen
#endif

-#define VC_UNI_SCREEN_DEBUG 0
-
typedef uint32_t char32_t;

/*
@@ -580,43 +578,6 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
}
}

-/* this is for validation and debugging only */
-static void vc_uniscr_debug_check(struct vc_data *vc)
-{
- struct uni_screen *uniscr = get_vc_uniscr(vc);
- unsigned short *p;
- int x, y, mask;
-
- if (!VC_UNI_SCREEN_DEBUG || !uniscr)
- return;
-
- WARN_CONSOLE_UNLOCKED();
-
- /*
- * Make sure our unicode screen translates into the same glyphs
- * as the actual screen. This is brutal indeed.
- */
- p = (unsigned short *)vc->vc_origin;
- mask = vc->vc_hi_font_mask | 0xff;
- for (y = 0; y < vc->vc_rows; y++) {
- char32_t *line = uniscr->lines[y];
- for (x = 0; x < vc->vc_cols; x++) {
- u16 glyph = scr_readw(p++) & mask;
- char32_t uc = line[x];
- int tc = conv_uni_to_pc(vc, uc);
- if (tc == -4)
- tc = conv_uni_to_pc(vc, 0xfffd);
- if (tc == -4)
- tc = conv_uni_to_pc(vc, '?');
- if (tc != glyph)
- pr_err_ratelimited(
- "%s: mismatch at %d,%d: glyph=%#x tc=%#x\n",
- __func__, x, y, glyph, tc);
- }
- }
-}
-
-
static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
enum con_scroll dir, unsigned int nr)
{
@@ -2959,7 +2920,6 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co
goto rescan_last_byte;
}
con_flush(vc, &draw);
- vc_uniscr_debug_check(vc);
console_conditional_schedule();
notify_update(vc);
console_unlock();
--
2.39.0


2023-01-12 08:24:00

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE()

No need to panic in vc_uniscr_copy_line(), just warn. This should never
happen though, as vc_uniscr_check() is supposed to be called before
vc_uniscr_copy_line(). And the former checks vc->vc_uni_lines already.

In any case, use _ONCE as vc_uniscr_copy_line() is called repeatedly for
each line. So don't flood the logs, just in case.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/vt/vt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 119b3eafef59..db72375141b0 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -535,7 +535,8 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
int offset = row * vc->vc_size_row + col * 2;
unsigned long pos;

- BUG_ON(!uni_lines);
+ if (WARN_ON_ONCE(!uni_lines))
+ return;

pos = (unsigned long)screenpos(vc, offset, viewed);
if (pos >= vc->vc_origin && pos < vc->vc_scr_end) {
--
2.39.0

2023-01-12 08:53:13

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 09/11] tty: vt: separate array juggling to juggle_array()

The algorithm used for scrolling is the array juggling. It has
complexity O(N) and space complexity O(1). I.e. quite fast w/o
requirements for temporary storage.

Move the algorithm to a separate function so it is obvious what it is.
It is almost generic (except the array type), so if anyone else wants
array rotation, feel free to make it generic and move it to include/.

And rename all the variables from i, j, k, sz, d, and so on to something
saner.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/vt/vt.c | 52 ++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 74db07b32abe..7cda18b7ee3d 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -398,40 +398,44 @@ static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
}

+/* juggling array rotation algorithm (complexity O(N), size complexity O(1)) */
+static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
+{
+ unsigned int gcd_idx;
+
+ for (gcd_idx = 0; gcd_idx < gcd(nr, size); gcd_idx++) {
+ u32 *gcd_idx_val = array[gcd_idx];
+ unsigned int dst_idx = gcd_idx;
+
+ while (1) {
+ unsigned int src_idx = (dst_idx + nr) % size;
+ if (src_idx == gcd_idx)
+ break;
+
+ array[dst_idx] = array[src_idx];
+ dst_idx = src_idx;
+ }
+
+ array[dst_idx] = gcd_idx_val;
+ }
+}
+
static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
enum con_scroll dir, unsigned int nr)
{
u32 **uni_lines = vc->vc_uni_lines;
- unsigned int i, j, k, sz, d, clear;
+ unsigned int size = b - t;

if (!uni_lines)
return;

- sz = b - t;
- clear = b - nr;
- d = nr;
-
if (dir == SM_DOWN) {
- clear = t;
- d = sz - nr;
- }
-
- for (i = 0; i < gcd(d, sz); i++) {
- u32 *tmp = uni_lines[t + i];
- j = i;
- while (1) {
- k = j + d;
- if (k >= sz)
- k -= sz;
- if (k == i)
- break;
- uni_lines[t + j] = uni_lines[t + k];
- j = k;
- }
- uni_lines[t + j] = tmp;
+ juggle_array(&uni_lines[top], size, size - nr);
+ vc_uniscr_clear_lines(vc, t, nr);
+ } else {
+ juggle_array(&uni_lines[top], size, nr);
+ vc_uniscr_clear_lines(vc, b - nr, nr);
}
-
- vc_uniscr_clear_lines(vc, clear, nr);
}

static void vc_uniscr_copy_area(u32 **dst_lines,
--
2.39.0

2023-01-12 09:10:59

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check()

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> VC_UNI_SCREEN_DEBUG is always defined as 0, so this code is never
> executed. Drop it along with VC_UNI_SCREEN_DEBUG.
>
> Signed-off-by: Jiri Slaby (SUSE) <[email protected]>

Reviewed-by: Ilpo J?rvinen <[email protected]>

--
i.

2023-01-12 10:27:41

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE()

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> No need to panic in vc_uniscr_copy_line(), just warn. This should never
> happen though, as vc_uniscr_check() is supposed to be called before
> vc_uniscr_copy_line(). And the former checks vc->vc_uni_lines already.
>
> In any case, use _ONCE as vc_uniscr_copy_line() is called repeatedly for
> each line. So don't flood the logs, just in case.
>
> Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
> ---
> drivers/tty/vt/vt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 119b3eafef59..db72375141b0 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -535,7 +535,8 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
> int offset = row * vc->vc_size_row + col * 2;
> unsigned long pos;
>
> - BUG_ON(!uni_lines);
> + if (WARN_ON_ONCE(!uni_lines))
> + return;
>
> pos = (unsigned long)screenpos(vc, offset, viewed);
> if (pos >= vc->vc_origin && pos < vc->vc_scr_end) {
>

Reviewed-by: Ilpo J?rvinen <[email protected]>

--
i.

2023-01-12 10:31:02

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 09/11] tty: vt: separate array juggling to juggle_array()

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> The algorithm used for scrolling is the array juggling. It has
> complexity O(N) and space complexity O(1). I.e. quite fast w/o
> requirements for temporary storage.
>
> Move the algorithm to a separate function so it is obvious what it is.
> It is almost generic (except the array type), so if anyone else wants
> array rotation, feel free to make it generic and move it to include/.
>
> And rename all the variables from i, j, k, sz, d, and so on to something
> saner.
>
> Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
> ---
> drivers/tty/vt/vt.c | 52 ++++++++++++++++++++++++---------------------
> 1 file changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 74db07b32abe..7cda18b7ee3d 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -398,40 +398,44 @@ static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
> memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
> }
>
> +/* juggling array rotation algorithm (complexity O(N), size complexity O(1)) */
> +static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
> +{
> + unsigned int gcd_idx;
> +
> + for (gcd_idx = 0; gcd_idx < gcd(nr, size); gcd_idx++) {
> + u32 *gcd_idx_val = array[gcd_idx];
> + unsigned int dst_idx = gcd_idx;
> +
> + while (1) {
> + unsigned int src_idx = (dst_idx + nr) % size;
> + if (src_idx == gcd_idx)
> + break;
> +
> + array[dst_idx] = array[src_idx];
> + dst_idx = src_idx;
> + }
> +
> + array[dst_idx] = gcd_idx_val;
> + }
> +}
> +
> static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
> enum con_scroll dir, unsigned int nr)
> {
> u32 **uni_lines = vc->vc_uni_lines;
> - unsigned int i, j, k, sz, d, clear;
> + unsigned int size = b - t;
>
> if (!uni_lines)
> return;
>
> - sz = b - t;
> - clear = b - nr;
> - d = nr;
> -
> if (dir == SM_DOWN) {
> - clear = t;
> - d = sz - nr;
> - }
> -
> - for (i = 0; i < gcd(d, sz); i++) {
> - u32 *tmp = uni_lines[t + i];
> - j = i;
> - while (1) {
> - k = j + d;
> - if (k >= sz)
> - k -= sz;
> - if (k == i)
> - break;
> - uni_lines[t + j] = uni_lines[t + k];
> - j = k;
> - }
> - uni_lines[t + j] = tmp;
> + juggle_array(&uni_lines[top], size, size - nr);

top? Should be t I think.

> + vc_uniscr_clear_lines(vc, t, nr);
> + } else {
> + juggle_array(&uni_lines[top], size, nr);

Ditto.

> + vc_uniscr_clear_lines(vc, b - nr, nr);
> }
> -
> - vc_uniscr_clear_lines(vc, clear, nr);
> }
>
> static void vc_uniscr_copy_area(u32 **dst_lines,
>



--
i.