* [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix @ 2020-05-12 14:28 Richard Palethorpe 2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe 2020-05-13 8:23 ` [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Petr Vorel 0 siblings, 2 replies; 6+ messages in thread From: Richard Palethorpe @ 2020-05-12 14:28 UTC (permalink / raw) To: ltp Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- testcases/kernel/pty/pty04.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testcases/kernel/pty/pty04.c b/testcases/kernel/pty/pty04.c index 252babe92..bfda08b2b 100644 --- a/testcases/kernel/pty/pty04.c +++ b/testcases/kernel/pty/pty04.c @@ -92,7 +92,7 @@ static struct ldisc_info ldiscs[] = { {N_SLCAN, "N_SLCAN", CAN_MTU}, }; -static volatile int ptmx, pts, sk, mtu, no_check; +static int ptmx, pts, sk, mtu, no_check; static int set_ldisc(int tty, const struct ldisc_info *ldisc) { @@ -173,13 +173,15 @@ static void write_pty(const struct ldisc_info *ldisc) written = 0; - ret = TST_RETRY_FUNC(try_write(ptmx, data, len, &written), TST_RETVAL_NOTNULL); + ret = TST_RETRY_FUNC(try_write(ptmx, data, len, &written), + TST_RETVAL_NOTNULL); if (ret < 0) tst_brk(TBROK | TERRNO, "Failed 1st write to PTY"); tst_res(TPASS, "Wrote PTY 1"); written = 0; - ret = TST_RETRY_FUNC(try_write(ptmx, data, len, &written), TST_RETVAL_NOTNULL); + ret = TST_RETRY_FUNC(try_write(ptmx, data, len, &written), + TST_RETVAL_NOTNULL); if (ret < 0) tst_brk(TBROK | TERRNO, "Failed 2nd write to PTY"); -- 2.26.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] pty04: Retry reads when short 2020-05-12 14:28 [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Richard Palethorpe @ 2020-05-12 14:28 ` Richard Palethorpe 2020-05-12 15:44 ` Cyril Hrubis 2020-05-12 15:49 ` Petr Vorel 2020-05-13 8:23 ` [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Petr Vorel 1 sibling, 2 replies; 6+ messages in thread From: Richard Palethorpe @ 2020-05-12 14:28 UTC (permalink / raw) To: ltp Even though reads are blocking and packets are flipped into the netdevice buffer whole, it seems read may return before a full packet is read into user land. Retrying read should prevent timeouts and read failures on some machines. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Reported-by: Jan Stancek <jstancek@redhat.com> Cc: Petr Vorel <pvorel@suse.cz> --- NOTE: When running this test repeatedly and in parallel; strange things still happen, see: https://github.com/linux-test-project/ltp/issues/674#issuecomment-625181783 However this hopefully will fix the timeout issues and read failures reported by some testers. testcases/kernel/pty/pty04.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/testcases/kernel/pty/pty04.c b/testcases/kernel/pty/pty04.c index bfda08b2b..9b2911421 100644 --- a/testcases/kernel/pty/pty04.c +++ b/testcases/kernel/pty/pty04.c @@ -288,9 +288,20 @@ static void check_data(const struct ldisc_info *ldisc, tst_res(TINFO, "Will continue test without data checking"); } +static ssize_t try_read(int fd, char *data, ssize_t size, ssize_t *n) +{ + ssize_t ret = read(fd, data, size); + + if (ret < 0) + return -(errno != EAGAIN); + + return (*n += ret) >= size; +} + static void read_netdev(const struct ldisc_info *ldisc) { - int rlen, plen = 0; + int ret, rlen, plen = 0; + ssize_t n; char *data; switch (ldisc->n) { @@ -305,20 +316,27 @@ static void read_netdev(const struct ldisc_info *ldisc) tst_res(TINFO, "Reading from socket %d", sk); - SAFE_READ(1, sk, data, plen); + n = 0; + ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL); + if (ret < 0) + tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen); check_data(ldisc, data, plen); tst_res(TPASS, "Read netdev 1"); - SAFE_READ(1, sk, data, plen); + n = 0; + ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL); + if (ret < 0) + tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen); check_data(ldisc, data, plen); tst_res(TPASS, "Read netdev 2"); TST_CHECKPOINT_WAKE(0); - while((rlen = read(sk, data, plen)) > 0) + while ((rlen = read(sk, data, plen)) > 0) check_data(ldisc, data, rlen); tst_res(TPASS, "Reading data from netdev interrupted by hangup"); + close(sk); tst_free_all(); } @@ -357,6 +375,7 @@ static void cleanup(void) { ioctl(pts, TIOCVHANGUP); ioctl(ptmx, TIOCVHANGUP); + close(sk); tst_reap_children(); } -- 2.26.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] pty04: Retry reads when short 2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe @ 2020-05-12 15:44 ` Cyril Hrubis 2020-05-12 15:49 ` Petr Vorel 1 sibling, 0 replies; 6+ messages in thread From: Cyril Hrubis @ 2020-05-12 15:44 UTC (permalink / raw) To: ltp Hi! > +static ssize_t try_read(int fd, char *data, ssize_t size, ssize_t *n) > +{ > + ssize_t ret = read(fd, data, size); > + > + if (ret < 0) > + return -(errno != EAGAIN); > + > + return (*n += ret) >= size; > +} I had to read this piece twice, but I think it's correct. > static void read_netdev(const struct ldisc_info *ldisc) > { > - int rlen, plen = 0; > + int ret, rlen, plen = 0; > + ssize_t n; > char *data; > > switch (ldisc->n) { > @@ -305,20 +316,27 @@ static void read_netdev(const struct ldisc_info *ldisc) > > tst_res(TINFO, "Reading from socket %d", sk); > > - SAFE_READ(1, sk, data, plen); > + n = 0; > + ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL); > + if (ret < 0) > + tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen); I wonder if a simple loop without exponential backoff would suffice here. A least the code would probably be more readable. > check_data(ldisc, data, plen); > tst_res(TPASS, "Read netdev 1"); > > - SAFE_READ(1, sk, data, plen); > + n = 0; > + ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL); > + if (ret < 0) > + tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen); > check_data(ldisc, data, plen); > tst_res(TPASS, "Read netdev 2"); > > TST_CHECKPOINT_WAKE(0); > - while((rlen = read(sk, data, plen)) > 0) > + while ((rlen = read(sk, data, plen)) > 0) > check_data(ldisc, data, rlen); This should have been part of the previous cleanup patch. > tst_res(TPASS, "Reading data from netdev interrupted by hangup"); > > + close(sk); > tst_free_all(); > } > > @@ -357,6 +375,7 @@ static void cleanup(void) > { > ioctl(pts, TIOCVHANGUP); > ioctl(ptmx, TIOCVHANGUP); > + close(sk); > > tst_reap_children(); > } > -- > 2.26.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] pty04: Retry reads when short 2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe 2020-05-12 15:44 ` Cyril Hrubis @ 2020-05-12 15:49 ` Petr Vorel 2020-05-12 19:44 ` Jan Stancek 1 sibling, 1 reply; 6+ messages in thread From: Petr Vorel @ 2020-05-12 15:49 UTC (permalink / raw) To: ltp Hi Richard, > Even though reads are blocking and packets are flipped into the netdevice > buffer whole, it seems read may return before a full packet is read into user > land. Retrying read should prevent timeouts and read failures on some > machines. > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > Reported-by: Jan Stancek <jstancek@redhat.com> > Cc: Petr Vorel <pvorel@suse.cz> Reviewed-by: Petr Vorel <pvorel@suse.cz> Thanks for taking care of this. It's still possible to reproduce timeout just with higher -i (-i10), but it's an improvement. Kind regards, Petr ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] pty04: Retry reads when short 2020-05-12 15:49 ` Petr Vorel @ 2020-05-12 19:44 ` Jan Stancek 0 siblings, 0 replies; 6+ messages in thread From: Jan Stancek @ 2020-05-12 19:44 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi Richard, > > > Even though reads are blocking and packets are flipped into the netdevice > > buffer whole, it seems read may return before a full packet is read into > > user > > land. Retrying read should prevent timeouts and read failures on some > > machines. > > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > Reported-by: Jan Stancek <jstancek@redhat.com> > > Cc: Petr Vorel <pvorel@suse.cz> > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Thanks for taking care of this. ACK, I'll update github issue when this runs for a while with recent upstream kernels. > It's still possible to reproduce timeout just with higher -i (-i10), > but it's an improvement. > > Kind regards, > Petr > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix 2020-05-12 14:28 [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Richard Palethorpe 2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe @ 2020-05-13 8:23 ` Petr Vorel 1 sibling, 0 replies; 6+ messages in thread From: Petr Vorel @ 2020-05-13 8:23 UTC (permalink / raw) To: ltp Hi Richard, Merged (with while from 2nd patch). Thanks! Kind regards, Petr ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-13 8:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-12 14:28 [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Richard Palethorpe 2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe 2020-05-12 15:44 ` Cyril Hrubis 2020-05-12 15:49 ` Petr Vorel 2020-05-12 19:44 ` Jan Stancek 2020-05-13 8:23 ` [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Petr Vorel
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.