This is another part (say part II.) of the previous type unification
across the tty layer[1]. This time, in tty_buffer. Apart from type
changes, this series contains a larger set of refactoring of the code.
Namely, unification of byte stuffing into the tty buffers into a single
function.
[1] https://lore.kernel.org/all/[email protected]/
Jiri Slaby (SUSE) (10):
tty: tty_buffer: switch data type to u8
tty: tty_buffer: use struct_size() in tty_buffer_alloc()
tty: tty_buffer: unify tty_insert_flip_string_{fixed_flag,flags}()
tty: tty_buffer: warn if losing flags in
__tty_insert_flip_string_flags()
tty: tty_buffer: switch insert functions to size_t
tty: tty_buffer: let tty_prepare_flip_string() return size_t
tty: tty_buffer: use __tty_insert_flip_string_flags() in
tty_insert_flip_char()
tty: tty_buffer: better types in __tty_buffer_request_room()
tty: tty_buffer: initialize variables in initializers already
tty: tty_buffer: invert conditions in __tty_buffer_request_room()
Documentation/driver-api/tty/tty_buffer.rst | 7 +-
drivers/tty/tty_buffer.c | 169 ++++++--------------
include/linux/tty_buffer.h | 4 +-
include/linux/tty_flip.h | 64 ++++++--
4 files changed, 111 insertions(+), 133 deletions(-)
--
2.41.0
The same as in the previous patch, tty_prepare_flip_string() accepts
size_t as an size argument. It returns the same size (or less). It is
unexpected that it returns a signed value and can confuse users to check
for negative values.
Instead, return the same size_t as accepted to make clear we return
values >= 0, where zero in fact means failure.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 5 +++--
include/linux/tty_flip.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 598891e53031..4f84466498f7 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -383,9 +383,9 @@ EXPORT_SYMBOL(__tty_insert_flip_char);
* Returns: the length available and buffer pointer (@chars) to the space which
* is now allocated and accounted for as ready for normal characters.
*/
-int tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size)
+size_t tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size)
{
- int space = __tty_buffer_request_room(port, size, false);
+ size_t space = __tty_buffer_request_room(port, size, false);
if (likely(space)) {
struct tty_buffer *tb = port->buf.tail;
@@ -395,6 +395,7 @@ int tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size)
memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space);
tb->used += space;
}
+
return space;
}
EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 569747364ae5..efd03d9c11f8 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -13,7 +13,7 @@ int tty_buffer_request_room(struct tty_port *port, size_t size);
size_t __tty_insert_flip_string_flags(struct tty_port *port, const u8 *chars,
const u8 *flags, bool mutable_flags,
size_t size);
-int tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size);
+size_t tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size);
void tty_flip_buffer_push(struct tty_port *port);
int __tty_insert_flip_char(struct tty_port *port, u8 ch, u8 flag);
--
2.41.0
It makes the code both more compact, and more understandable.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 414bb7f9155f..44c0adaec850 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -262,17 +262,10 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size,
bool flags)
{
struct tty_bufhead *buf = &port->buf;
- struct tty_buffer *b, *n;
- size_t left;
- bool change;
+ struct tty_buffer *n, *b = buf->tail;
+ size_t left = (b->flags ? 1 : 2) * b->size - b->used;
+ bool change = !b->flags && flags;
- b = buf->tail;
- if (!b->flags)
- left = 2 * b->size - b->used;
- else
- left = b->size - b->used;
-
- change = !b->flags && flags;
if (change || left < size) {
/* This is the slow path - looking for new buffers to use */
n = tty_buffer_alloc(port, size);
--
2.41.0
Use __tty_insert_flip_string_flags() for the slow path of
tty_insert_flip_char(). The former is generic enough, so there is no
reason to reimplement the injection once again.
So now we have a single function stuffing into tty buffers.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
Documentation/driver-api/tty/tty_buffer.rst | 3 ++-
drivers/tty/tty_buffer.c | 26 ---------------------
include/linux/tty_flip.h | 12 +++++++---
3 files changed, 11 insertions(+), 30 deletions(-)
diff --git a/Documentation/driver-api/tty/tty_buffer.rst b/Documentation/driver-api/tty/tty_buffer.rst
index 774dc119c312..4b5ca1776d4f 100644
--- a/Documentation/driver-api/tty/tty_buffer.rst
+++ b/Documentation/driver-api/tty/tty_buffer.rst
@@ -15,11 +15,12 @@ Flip Buffer Management
======================
.. kernel-doc:: drivers/tty/tty_buffer.c
- :identifiers: tty_prepare_flip_string __tty_insert_flip_char
+ :identifiers: tty_prepare_flip_string
tty_flip_buffer_push tty_ldisc_receive_buf
.. kernel-doc:: include/linux/tty_flip.h
:identifiers: tty_insert_flip_string_fixed_flag tty_insert_flip_string_flags
+ tty_insert_flip_char
----
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 4f84466498f7..e162318d6c31 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -343,32 +343,6 @@ size_t __tty_insert_flip_string_flags(struct tty_port *port, const u8 *chars,
}
EXPORT_SYMBOL(__tty_insert_flip_string_flags);
-/**
- * __tty_insert_flip_char - add one character to the tty buffer
- * @port: tty port
- * @ch: character
- * @flag: flag byte
- *
- * Queue a single byte @ch to the tty buffering, with an optional flag. This is
- * the slow path of tty_insert_flip_char().
- */
-int __tty_insert_flip_char(struct tty_port *port, u8 ch, u8 flag)
-{
- struct tty_buffer *tb;
- bool flags = flag != TTY_NORMAL;
-
- if (!__tty_buffer_request_room(port, 1, flags))
- return 0;
-
- tb = port->buf.tail;
- if (tb->flags)
- *flag_buf_ptr(tb, tb->used) = flag;
- *char_buf_ptr(tb, tb->used++) = ch;
-
- return 1;
-}
-EXPORT_SYMBOL(__tty_insert_flip_char);
-
/**
* tty_prepare_flip_string - make room for characters
* @port: tty port
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index efd03d9c11f8..af4fce98f64e 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -15,7 +15,6 @@ size_t __tty_insert_flip_string_flags(struct tty_port *port, const u8 *chars,
size_t size);
size_t tty_prepare_flip_string(struct tty_port *port, u8 **chars, size_t size);
void tty_flip_buffer_push(struct tty_port *port);
-int __tty_insert_flip_char(struct tty_port *port, u8 ch, u8 flag);
/**
* tty_insert_flip_string_fixed_flag - add characters to the tty buffer
@@ -55,7 +54,14 @@ static inline size_t tty_insert_flip_string_flags(struct tty_port *port,
return __tty_insert_flip_string_flags(port, chars, flags, true, size);
}
-
+/**
+ * tty_insert_flip_char - add one character to the tty buffer
+ * @port: tty port
+ * @ch: character
+ * @flag: flag byte
+ *
+ * Queue a single byte @ch to the tty buffering, with an optional flag.
+ */
static inline size_t tty_insert_flip_char(struct tty_port *port, u8 ch, u8 flag)
{
struct tty_buffer *tb = port->buf.tail;
@@ -68,7 +74,7 @@ static inline size_t tty_insert_flip_char(struct tty_port *port, u8 ch, u8 flag)
*char_buf_ptr(tb, tb->used++) = ch;
return 1;
}
- return __tty_insert_flip_char(port, ch, flag);
+ return __tty_insert_flip_string_flags(port, &ch, &flag, false, 1);
}
static inline size_t tty_insert_flip_string(struct tty_port *port,
--
2.41.0
* use bool for 'change' as it holds a result of a boolean.
* use size_t for 'left', so it is the same as 'size' which it is
compared to. Both are supposed to contain an unsigned value.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index e162318d6c31..414bb7f9155f 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -263,7 +263,8 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size,
{
struct tty_bufhead *buf = &port->buf;
struct tty_buffer *b, *n;
- int left, change;
+ size_t left;
+ bool change;
b = buf->tail;
if (!b->flags)
--
2.41.0
Now, that tty_buffer::data has the right type, use struct_size() for
size calculation. struct_size() makes the code less error-prone and more
readable.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 684d099cbe11..c94df1a2d7f8 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -177,8 +177,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
*/
if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit)
return NULL;
- p = kmalloc(sizeof(struct tty_buffer) + 2 * size,
- GFP_ATOMIC | __GFP_NOWARN);
+ p = kmalloc(struct_size(p, data, 2 * size), GFP_ATOMIC | __GFP_NOWARN);
if (p == NULL)
return NULL;
--
2.41.0
We are used to handle "bad" states in the 'if's in the kernel. Refactor
(invert the two conditions in) __tty_buffer_request_room(), so that the
code returns from the fast paths immediately instead of postponing to
the heavy end of the function.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 44 ++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 44c0adaec850..5f6d0cf67571 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -266,28 +266,28 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size,
size_t left = (b->flags ? 1 : 2) * b->size - b->used;
bool change = !b->flags && flags;
- if (change || left < size) {
- /* This is the slow path - looking for new buffers to use */
- n = tty_buffer_alloc(port, size);
- if (n != NULL) {
- n->flags = flags;
- buf->tail = n;
- /*
- * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
- * ensures they see all buffer data.
- */
- smp_store_release(&b->commit, b->used);
- /*
- * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
- * ensures the latest commit value can be read before the head
- * is advanced to the next buffer.
- */
- smp_store_release(&b->next, n);
- } else if (change)
- size = 0;
- else
- size = left;
- }
+ if (!change && left >= size)
+ return size;
+
+ /* This is the slow path - looking for new buffers to use */
+ n = tty_buffer_alloc(port, size);
+ if (n == NULL)
+ return change ? 0 : left;
+
+ n->flags = flags;
+ buf->tail = n;
+ /*
+ * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
+ * ensures they see all buffer data.
+ */
+ smp_store_release(&b->commit, b->used);
+ /*
+ * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
+ * ensures the latest commit value can be read before the head
+ * is advanced to the next buffer.
+ */
+ smp_store_release(&b->next, n);
+
return size;
}
--
2.41.0
And add a WARN_ON_ONCE(need_flags) to make sure we are not losing flags
in __tty_insert_flip_string_flags().
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/tty_buffer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 94a88dc05a54..c101b4ab737e 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -325,6 +325,9 @@ int __tty_insert_flip_string_flags(struct tty_port *port, const u8 *chars,
flags += space;
} else if (tb->flags) {
memset(flag_buf_ptr(tb, tb->used), flags[0], space);
+ } else {
+ /* tb->flags should be available once requested */
+ WARN_ON_ONCE(need_flags);
}
tb->used += space;
--
2.41.0