From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by befuddled.reisers.ca (Postfix, from userid 65534) id 691871EFD6B; Wed, 5 Jul 2017 15:52:09 -0400 (EDT) Received: from mail-wr0-x234.google.com (mail-wr0-x234.google.com [IPv6:2a00:1450:400c:c0c::234]) by befuddled.reisers.ca (Postfix) with ESMTPS id 1E7641EF10A for ; Wed, 5 Jul 2017 15:52:07 -0400 (EDT) Received: by mail-wr0-x234.google.com with SMTP id c11so269641813wrc.3 for ; Wed, 05 Jul 2017 12:52:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=tf22D7YqDg20LB4Qt4t1P9QNKXketHzAO/qKKaJuuGU=; b=uiWtvamlL+h/w84HbhCSjh9YyTx/xMgWNHg/hU7ucX/76tKbnZpV5CgT0/zxc/RUhK atqDCte9oo2Ck3e7vmPM8Ye3WnQect6b+rLmokQRcPJq5X8SqsaSuKDV1lsF35xKXhSA bt+s5Dz+vNoWRu25lwqHmWDJkYhCOgYAJpLW8QINFCXE1VSfTDi/FmBogEcpVzjf7wui 67e8yFFH0y0YCi+JblMuWex1lrr0/fTbba3EqUjr7ye/480N7WhB0QPzMPIKaS1D6Eni R/RGqHkao1ELUP97dQK5HNlVieM8xVf7lcJALFarkBiWfX6Ep/na4yyF4heylN80YmhC 4BLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=tf22D7YqDg20LB4Qt4t1P9QNKXketHzAO/qKKaJuuGU=; b=c/k8fEWX5oU8FcqO2/w/QAM9oGpgcUy8w190Nta5TNOIhXQup92SN0mEz0uYyJO/OA w6aosnTS38oFlq3o2J+MkHOi59VbS3RKr9IPdB53+rM6Ko4qDz8nWtwkUSHyZkV4hwFy U33I2JzQ4NDXD/LEmIImvz5+Xdqr1Hs8q6HUf9ycDISep7gWxAiw15oaM8nE6MNs/g/v eKMeZEEIDOt0H5kbhByVJ7ATx8tOkXLY2/5TB75yHfdGiy2ie3eRV7dVVi+8t5Yfwmi1 MznLaxWvAzc9If5L3i98P1/iDaE4KUf9iQjcSUWdP/BdvGEl5c2hHCHWz8PM6V+iUQyY cqpA== X-Gm-Message-State: AKS2vOxWBWBnl3zoio/hUroT3kdRmBOZ5Jh7M9e673y1A3PxIF8XDL4F +6m3xQvSsOwKkQ== X-Received: by 10.223.166.8 with SMTP id k8mr41841532wrc.172.1499284323851; Wed, 05 Jul 2017 12:52:03 -0700 (PDT) Received: from sanghar ([2a00:23c4:7320:5900:224:d6ff:fe76:7136]) by smtp.gmail.com with ESMTPSA id y5sm18574780wrd.52.2017.07.05.12.52.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 05 Jul 2017 12:52:03 -0700 (PDT) Date: Wed, 5 Jul 2017 20:52:01 +0100 From: Okash Khawaja To: John Covici Cc: speakup@linux-speakup.org, Samuel Thibault Subject: Re: speakup-r empty line lockup Message-ID: <20170705195201.GA1573@sanghar> References: <20170624090645.GA8813@sanghar> <20170703111453.GA2857@sanghar> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.2 (2017-04-18) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 X-BeenThere: speakup@linux-speakup.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Speakup is a screen review system for Linux." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Jul 2017 19:52:09 -0000 Hi, I used printk to output result of in_interrupt() when the affected code is called. It looks like the affected code is only called when inside interrupt context, i.e. when keyboard_notifier_callback is called and that's when we can't call speakup_fake_down_arrow as that leads to lockup, as explained earlier in this thread. Given limited scope of the change - it is only called when running speakup-r from empty line - I can't work out how this results in cursor going to the end of file when control is hit. However, I say this based on speakup_soft + espeakup test in which speakup-r stops automatically after reading two lines (both before and after this patch). So until I am able to replicate the problem, it will be hard for me to test. The triple talk I bricked, I haven't got round to fixing it yet - if it can be fixed. If there is any additional information that might be relevant, that will be helpful. Until then, I will try to work out what might be happening. Since the github repo contains this and other changes, it will be interesting to check if the problem is down to this change or is a result of other changes that also went in. Thanks, Okash 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