mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: David VomLehn <dvomlehn@cisco.com>
Cc: Oliver Neukum <oliver@neukum.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, greg@kroah.com
Subject: Re: [PATCH 1/1, v2] usb-use-kfifo-to-buffer-usb-generic-serial-writes.patch
Date: Fri, 28 Aug 2009 21:01:06 +0100	[thread overview]
Message-ID: <20090828210106.51b1182a@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <20090828194810.GA10728@cuplxvomd02.corp.sa.net>

> > You may be writing a command. If you clip out a sequence in the middle you
> > might be sending an altered command without telling user space.

Serial has no framing so you can't assume alignments

> Serial output does not provide a reliable delivery mechanism. If you flip a bit
> in the sending UART, you may also send an altered command without telling user
> space. Still, having had some time to think about it, I think discarding the
> rest of the data is at least as reasonable as anything else.

Agreed

> It seems propagating errors back to user space appears somewhat problematic,
> but this is true independent of whether a FIFO is used.

The best I can think of would be to implement socket like behaviour from
networking. That is

- at the start of a write check for an asynchronous error event
- if there is one, clear the event and report it
- if not do the normal write

and keep a single 's**t happened' variable to record such matters.

> > How can this duplicate data? If you know that you need to call
> > usb_serial_generic_write_start, you also know that no further
> > transfer will occur. If you don't test and restart on resume, you
> > get this scenario:
> > 
> > user space writes to device
> > user space waits for answer
> > 
> > kernel transfers 1. buffer
> > system suspension - 2. buffer cannot be transfered,
> > 	as usb_serial_generic_write_start gets -EPERM
> > system resumption - 2. buffer is never transmitted,
> > 	user space times out and reports an error

You would need to handle this in the shutdown - and beyond that you need
to wait for any FIFO in the device before you power it down.

Its not really fixable as it isn't transactional and you have no idea if
the state of the other device also changed. Consider 

ATD foo
oops flight time
suspend

Arrive in Australia
resume
ATD foo

Ouch that was an expensive data call...

So I am not sure that trying to be clever here is actually productive.

  reply	other threads:[~2009-08-28 20:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-27  1:34 David VomLehn
2009-08-27  8:14 ` Oliver Neukum
2009-08-27 17:22   ` David VomLehn
2009-08-28 13:07     ` Oliver Neukum
2009-08-28 19:48       ` David VomLehn
2009-08-28 20:01         ` Alan Cox [this message]
2009-08-29 12:56           ` Oliver Neukum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090828210106.51b1182a@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=dvomlehn@cisco.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=oliver@neukum.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®