Re: [PATCH v2 1/3] tty: serdev: Export functions to pause receive_buf callback calls
From: Gary Guo
Date: Wed Sep 23 2026 - 09:57:45 EST
On Wed Sep 23, 2026 at 2:31 PM BST, Markus Probst wrote:
> On Wed, 2026-09-23 at 12:35 +0200, Greg Kroah-Hartman wrote:
>> On Sun, Sep 20, 2026 at 02:29:58PM +0000, Markus Probst wrote:
>> > These functions will be used to simply the serdev rust abstraction. It
>> > also contributes to the fixing of 2 race conditions in the serdev rust
>> > abstraction.
>> >
>> > Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
>> > ---
>> > drivers/tty/serdev/core.c | 50 ++++++++++++++++++++++++++++++++++++-
>> > drivers/tty/serdev/serdev-ttyport.c | 38 ++++++++++++++++++++++++++++
>> > include/linux/serdev.h | 6 +++++
>> > 3 files changed, 93 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
>> > index bab1b143b8a6..e8aa89e733bd 100644
>> > --- a/drivers/tty/serdev/serdev-ttyport.c
>> > +++ b/drivers/tty/serdev/serdev-ttyport.c
>> > @@ -7,8 +7,10 @@
>> > #include <linux/tty.h>
>> > #include <linux/tty_driver.h>
>> > #include <linux/poll.h>
>> > +#include "../tty.h"
>> >
>> > #define SERPORT_ACTIVE 1
>> > +#define SERPORT_PAUSE_RX 2
>> >
>> > struct serport {
>> > struct tty_port *port;
>> > @@ -32,6 +34,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
>> > if (!test_bit(SERPORT_ACTIVE, &serport->flags))
>> > return 0;
>> >
>> > + if (test_bit(SERPORT_PAUSE_RX, &serport->flags))
>> > + return 0;
>> > +
>> > + /*
>> > + * Ensure writes by the driver are visible before allowing traffic to resume.
>> > + */
>> > + smp_mb__after_atomic();
>>
>> This scares me. Why not use a real lock?
>>
> I can use locks to make it less "fragile". But I don't I think I need
> them.
>
>> WHat's the issue here, you
>> need this to be "flushed" before this call:
>>
>> > +
>> > ret = serdev_controller_receive_buf(ctrl, cp, count);
>>
>> here?
>>
>> And you just tested a bit, you didn't set a bit, so what are you trying
>> to ensure is written exactly?
> This should be an acquire load operation (paired with the release store
> operation in `ttyport_resume_rx`).
Then you should use `test_bit_acquire`.
However, this isn't sufficient because there's no synchronization between this
bit test with the bit set inside pause_rx.
Best,
Gary