* [bug report] usb: gadget: u_serial: process RX in workqueue instead of tasklet
@ 2019-06-24 12:32 Dan Carpenter
2019-06-24 13:41 ` mirq-linux
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-06-24 12:32 UTC (permalink / raw)
To: mirq-linux; +Cc: linux-usb
Hello Michał Mirosław,
This is a semi-automatic email about new static checker warnings.
The patch 8b4c62aef6f6: "usb: gadget: u_serial: process RX in
workqueue instead of tasklet" from Dec 16, 2018, leads to the
following Smatch complaint:
drivers/usb/gadget/function/u_serial.c:438 gs_rx_push()
error: we previously assumed 'tty' could be null (see line 373)
drivers/usb/gadget/function/u_serial.c
372 /* leave data queued if tty was rx throttled */
373 if (tty && tty_throttled(tty))
^^^^^^^^^^^^^^^^^^^^^^^^
Other checks for NULL
374 break;
375
376 switch (req->status) {
377 case -ESHUTDOWN:
378 disconnect = true;
379 pr_vdebug("ttyGS%d: shutdown\n", port->port_num);
380 break;
381
382 default:
383 /* presumably a transient fault */
384 pr_warn("ttyGS%d: unexpected RX status %d\n",
385 port->port_num, req->status);
386 /* FALLTHROUGH */
387 case 0:
388 /* normal completion */
389 break;
390 }
391
392 /* push data to (open) tty */
393 if (req->actual && tty) {
394 char *packet = req->buf;
395 unsigned size = req->actual;
396 unsigned n;
397 int count;
398
399 /* we may have pushed part of this packet already... */
400 n = port->n_read;
401 if (n) {
402 packet += n;
403 size -= n;
404 }
405
406 count = tty_insert_flip_string(&port->port, packet,
407 size);
408 if (count)
409 do_push = true;
410 if (count != size) {
411 /* stop pushing; TTY layer can't handle more */
412 port->n_read += count;
413 pr_vdebug("ttyGS%d: rx block %d/%d\n",
414 port->port_num, count, req->actual);
415 break;
416 }
417 port->n_read = 0;
418 }
419
420 list_move(&req->list, &port->read_pool);
421 port->read_started--;
422 }
423
424 /* Push from tty to ldisc; this is handled by a workqueue,
425 * so we won't get callbacks and can hold port_lock
426 */
427 if (do_push)
428 tty_flip_buffer_push(&port->port);
429
430
431 /* We want our data queue to become empty ASAP, keeping data
432 * in the tty and ldisc (not here). If we couldn't push any
433 * this time around, RX may be starved, so wait until next jiffy.
434 *
435 * We may leave non-empty queue only when there is a tty, and
436 * either it is throttled or there is no more room in flip buffer.
437 */
438 if (!list_empty(queue) && !tty_throttled(tty))
^^^^^^^^^^^^^^^^^^^
in the original code there was check for NULL here but the patch removed
it.
439 schedule_delayed_work(&port->push, 1);
440
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] usb: gadget: u_serial: process RX in workqueue instead of tasklet
2019-06-24 12:32 [bug report] usb: gadget: u_serial: process RX in workqueue instead of tasklet Dan Carpenter
@ 2019-06-24 13:41 ` mirq-linux
2019-06-24 14:16 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: mirq-linux @ 2019-06-24 13:41 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-usb
On Mon, Jun 24, 2019 at 03:32:58PM +0300, Dan Carpenter wrote:
> Hello Michał Mirosław,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 8b4c62aef6f6: "usb: gadget: u_serial: process RX in
> workqueue instead of tasklet" from Dec 16, 2018, leads to the
> following Smatch complaint:
[...]
> 431 /* We want our data queue to become empty ASAP, keeping data
> 432 * in the tty and ldisc (not here). If we couldn't push any
> 433 * this time around, RX may be starved, so wait until next jiffy.
> 434 *
> 435 * We may leave non-empty queue only when there is a tty, and
> 436 * either it is throttled or there is no more room in flip buffer.
> 437 */
> 438 if (!list_empty(queue) && !tty_throttled(tty))
> ^^^^^^^^^^^^^^^^^^^
> in the original code there was check for NULL here but the patch removed
> it.
>
> 439 schedule_delayed_work(&port->push, 1);
> 440
Hi Dan,
The code is correct and explained in the comment above it - while() loop
above can be exited before emptying the queue only when tty != NULL.
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] usb: gadget: u_serial: process RX in workqueue instead of tasklet
2019-06-24 13:41 ` mirq-linux
@ 2019-06-24 14:16 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2019-06-24 14:16 UTC (permalink / raw)
To: mirq-linux; +Cc: linux-usb
On Mon, Jun 24, 2019 at 03:41:54PM +0200, mirq-linux@rere.qmqm.pl wrote:
> On Mon, Jun 24, 2019 at 03:32:58PM +0300, Dan Carpenter wrote:
> > Hello Michał Mirosław,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch 8b4c62aef6f6: "usb: gadget: u_serial: process RX in
> > workqueue instead of tasklet" from Dec 16, 2018, leads to the
> > following Smatch complaint:
> [...]
> > 431 /* We want our data queue to become empty ASAP, keeping data
> > 432 * in the tty and ldisc (not here). If we couldn't push any
> > 433 * this time around, RX may be starved, so wait until next jiffy.
> > 434 *
> > 435 * We may leave non-empty queue only when there is a tty, and
> > 436 * either it is throttled or there is no more room in flip buffer.
> > 437 */
> > 438 if (!list_empty(queue) && !tty_throttled(tty))
> > ^^^^^^^^^^^^^^^^^^^
> > in the original code there was check for NULL here but the patch removed
> > it.
> >
> > 439 schedule_delayed_work(&port->push, 1);
> > 440
>
> Hi Dan,
>
> The code is correct and explained in the comment above it - while() loop
> above can be exited before emptying the queue only when tty != NULL.
>
:( Sorry... Smatch isn't supposed to generate those warnings. I don't
know exactly what happened. I thought it was because of some recent
code changes I had made but now I think it it's because my build system
had a disk failure... Anyway, I can't reproduce the warning any more.
Thanks for looking at this.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-24 14:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 12:32 [bug report] usb: gadget: u_serial: process RX in workqueue instead of tasklet Dan Carpenter
2019-06-24 13:41 ` mirq-linux
2019-06-24 14:16 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).