Re: [PATCH] cdc-acm: some enhancement on acm delayed write

[Johan Hovold <jhovold@gmail.com>]

[ +CC: Alan ]

On Tue, Apr 08, 2014 at 12:33:31PM +0200, Oliver Neukum wrote:
> On Tue, 2014-04-08 at 09:33 +0200, Johan Hovold wrote:
> > On Tue, Apr 08, 2014 at 11:05:20AM +0800, Xiao Jin wrote:
> 
> > > (2) acm tty port ASYNCB_INITIALIZED flag will be cleared when
> > > close. If acm resume callback run after ASYNCB_INITIALIZED flag
> > > cleared, there will have no chance for delayed write to start.
> > > That lead to acm_wb.use can't be cleared. If user space open
> > > acm tty again and try to setd, tty will be blocked in
> > > tty_wait_until_sent for ever.
> > >
> > > This patch have two modification.
> > > (1) use list_head to save the write acm_wb during acm suspend.
> > > It can ensure no acm write abandoned.
> > > (2) enable flush buffer callback when acm tty close. acm delayed
> > > wb will start before acm port shutdown callback, it make sure
> > > the delayed acm_wb.use to be cleared. The patch also clear
> > > acm_wb.use and acm.transmitting when port shutdown.
> > 
> > This is not the right way to do this. See below.
> 
> If I see this correctly, then ASYNCB_INITIALIZED is cleared in
> tty_port_close() we is called from acm_tty_close(). Thus it should
> be enough to make sure that the device is resumed at the beginning
> of acm_tty_close() and acm_resume() will do the job automatically.
> What do you think?

But the device should already be about to be resumed, right? If the port
is closed fast enough that resume hasn't had time to run before
shutdown is called I think the right thing to do is simply to discard
the delayed bytes (in shutdown).

Johan