Re: speakup-r empty line lockup

[Okash Khawaja <okash.khawaja@gmail.com>]

Thanks, I'm looking into it.

On Wed, Jul 05, 2017 at 06:54:13AM -0400, John Covici wrote:
> Mope, its on regular serial, somehow your fix broke it.
> 
> On Mon, 03 Jul 2017 07:14:53 -0400,
> Okash Khawaja wrote:
> > 
> > Hi,
> > 
> > This looks like the problem you raised previously, the one which was
> > resolved by calling flush_buffers. Is this happening on ttyUSB?
> > usb-serial driver doesn't implement flush_buffers method which means
> > that speakup-r will have this issue until there is flush_buffers in
> > usb-serial.
> > 
> > Thanks,
> > Okash
> > 
> > On Mon, Jul 03, 2017 at 05:52:19AM -0400, John Covici wrote:
> > > I just tested the empty line lockup is  gone, but now speakup-r does
> > > not work, actually in the file I had, after reading a  number of lines
> > > when I hit control, the cursor was actually at the end of the file.
> > > 
> > > Thanks for working on this one.
> > > 
> > > On Sat, 24 Jun 2017 05:06:45 -0400,
> > > Okash Khawaja wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > The lockup when running speakup-r at start of an empty line occurs
> > > > because simulated key press is generated (using speakup_fake_down_arrow)
> > > > from context of keyboard_notifier_call callback which is called in
> > > > interrupt context. The simulated keypress leads to
> > > > keybaord_notifier_call to be trigerred again from the same context
> > > > leading to the lockup. The exact cause could be priority inversion where
> > > > simulated keypress cannot be processed because there is a real keyboard
> > > > interrupt already being processed (the one from which simulated keypress
> > > > was triggered), hence causing a deadlock. Please share your thoughts on
> > > > this. Here is the call chain.
> > > > 
> > > > (speakup-r) --> keyboard_notifier_call --> speakup_key --> do_spkup -->
> > > > read_all_doc --> get_sentence_buf [which returns -1 because of empty
> > > > line] --> kbd_fakekey2(RA_DOWN_ARROW) --> speakup_fake_down_arrow
> > > > 
> > > > The following patch resolves this by not simulating the keypress inside
> > > > keyboard notifier callback but instead delegating it to cursor_timer. In
> > > > the above chain, when get_sentence_buf returns -1, this patch starts
> > > > timer and passes RA_DOWN_ARROW as argument. When timer handler runs and
> > > > sees RA_DOWN_ARROW, it will then call kbd_fakekey2(RA_DOWN_ARROW) which
> > > > will correctly simulate the keypress inside timer context. I've tested
> > > > this succesfully.
> > > > 
> > > > It's the first time I've worked on this side of the code so please
> > > > review carefully :)
> > > > 
> > > > Finally, I have updated the test repo on github so you can test from
> > > > there.
> > > > 
> > > > Thanks,
> > > > Okash
> > > > 
> > > > ---
> > > >  drivers/staging/speakup/main.c |    3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > --- a/drivers/staging/speakup/main.c
> > > > +++ b/drivers/staging/speakup/main.c
> > > > @@ -1408,7 +1408,8 @@ static void read_all_doc(struct vc_data
> > > >  	cursor_track = read_all_mode;
> > > >  	spk_reset_index_count(0);
> > > >  	if (get_sentence_buf(vc, 0) == -1) {
> > > > -		kbd_fakekey2(vc, RA_DOWN_ARROW);
> > > > +		del_timer(&cursor_timer);
> > > > +		start_read_all_timer(vc, RA_DOWN_ARROW);
> > > >  	} else {
> > > >  		say_sentence_num(0, 0);
> > > >  		synth_insert_next_index(0);
> > > 
> > > -- 
> > > Your life is like a penny.  You're going to lose it.  The question is:
> > > How do
> > > you spend it?
> > > 
> > >          John Covici
> > >          covici@ccs.covici.com
> 
> -- 
> Your life is like a penny.  You're going to lose it.  The question is:
> How do
> you spend it?
> 
>          John Covici
>          covici@ccs.covici.com