From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by befuddled.reisers.ca (Postfix, from userid 65534) id DF51D1EFAE7; Sun, 27 Aug 2017 01:03:13 -0400 (EDT) Received: from mail-oi0-x242.google.com (mail-oi0-x242.google.com [IPv6:2607:f8b0:4003:c06::242]) by befuddled.reisers.ca (Postfix) with ESMTPS id 224E71EF123 for ; Sun, 27 Aug 2017 01:03:11 -0400 (EDT) Received: by mail-oi0-x242.google.com with SMTP id 4so2666736oie.2 for ; Sat, 26 Aug 2017 22:03:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=dCdVuNt3HIscn1SBfuYLtDLOCH/SJoWYwuDvEhr+YEs=; b=L2UE6D+yzdm3qzE37YJvRfMgxiW2X6GG5kVmcXTDvqYIC4CbBuXHBXiY4TkXlCTb/E 7hNCXuYWawCMggrxPoZqQ5cL/6kWoNXtDh5v8b4JApXBO/VpASi0zqJQw5d/LJGrUHRC M4VRu3N8maiKnZTfI3jkcKzr9MjSFWjl7rtyybtgR7RVfrW7bX087h3wEpMQkcI2JTb/ NiZf+b1A/6iDKHiYjA7nkJc+24qzLDfVBrLiQs2WcjRqj7MGOmdMJeuKw/jvyGz3D0bT U6b+Il8xo/lhSdYpv/jjeBMoPINNGruyplp5Q5eB/D7567BCW8OJowkwAJLcuszJLQUG 0Xrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=dCdVuNt3HIscn1SBfuYLtDLOCH/SJoWYwuDvEhr+YEs=; b=Mr8FmaiT1ZYTKji3cuVDkmsld8ki21bWLxFsprEJwPf6y0LAcpnrveisDkjSUkFjey +/DW+7AI5SkSdZ9IXleLU++B2Z9VCAMPqnDa+gza6SPK0ADCE7Yxyst/uvUWVg793ixm +o5HNGTZBcVwg0Fd3R682RbaLsCbkw+x9rupTTgmrj3XqUb8IfmGQrqYq3VVu/9nb8Vs 9lG4v5maVprwLEjeXv/i9jqWQdHK6eNaoqWDN2wajPPWhvWUKdToruUuTGSYhGsUY9mI 53+zXII3m2bRRiAVpQ68HBWUr9FU5jW/MgfI+NsTLwQ/1HathllWq0Bao9z/c2xD1Q9m NijA== X-Gm-Message-State: AHYfb5jMa5x1rVUzKmup2czRj/pKbSpWF2oKxdmX7Wjr45gQf50jSGlg 6IEMdPP818zssnPrKW65uR7nmPZxoA== X-Received: by 10.202.171.10 with SMTP id u10mr4138212oie.83.1503810189312; Sat, 26 Aug 2017 22:03:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.42.85 with HTTP; Sat, 26 Aug 2017 22:03:08 -0700 (PDT) Received: by 10.157.42.85 with HTTP; Sat, 26 Aug 2017 22:03:08 -0700 (PDT) In-Reply-To: References: <20170826171513.GA644@sanghar> From: Okash Khawaja Date: Sun, 27 Aug 2017 06:03:08 +0100 Message-ID: Subject: Re: [patch] staging: speakup: fix speakup-r empty line lockup To: John Covici Cc: Christopher Brannon , Samuel Thibault , "Speakup is a screen review system for Linux." X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 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: Sun, 27 Aug 2017 05:03:14 -0000 Hi John, Yes it is the same. But it is something we need otherwise the code will have a path which causes deadlock. My suggestion is we include this patch in speakup code and deal with incorrect cursor position when control key is hit, as a separate bug. Previous email explains the deadlock. Thanks, Okash On 27 Aug 2017 5:46 am, "John Covici" wrote: > hmmm, new patch is the same as the old one and so will not work any > better. > > On Sat, 26 Aug 2017 13:15:13 -0400, > Okash Khawaja wrote: > > > > When cursor is at beginning of an empty or whitespace-only line and > > speakup-r typed, kernel locks up. This happens because deadlock of in > > input_event function over dev->event_lock, as demonstrated by lockdep > > logs. The reason for that is speakup simulates a down arrow - because > > cursor is at an empty line - while inside key press notifier handler > > which is ultimately triggered from input_event function. The simulated > > key press leads to input_event being called again, this time under its > > own context. So the spinlock is dev->event_lock is acquired while still > > being held. > > > > This patch ensures that key press is not simulated from inside key press > > notifier handler. Instead it delegates to cursor_timer. It starts the > > 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. > > > > When not inside key press notifier callback, the behaviour will remain > > the same as before this patch. > > > > Signed-off-by: Okash Khawaja > > > > --- > > drivers/staging/speakup/main.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > --- a/drivers/staging/speakup/main.c > > +++ b/drivers/staging/speakup/main.c > > @@ -1376,6 +1376,8 @@ static void reset_highlight_buffers(stru > > > > static int read_all_key; > > > > +static volatile int in_keyboard_notifier = 0; > > + > > static void start_read_all_timer(struct vc_data *vc, int command); > > > > enum { > > @@ -1408,7 +1410,10 @@ 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); > > + if (!in_keyboard_notifier) > > + speakup_fake_down_arrow(); > > + start_read_all_timer(vc, RA_DOWN_ARROW); > > } else { > > say_sentence_num(0, 0); > > synth_insert_next_index(0); > > @@ -2212,8 +2217,10 @@ static int keyboard_notifier_call(struct > > int ret = NOTIFY_OK; > > static int keycode; /* to hold the current keycode */ > > > > + in_keyboard_notifier = 1; > > + > > if (vc->vc_mode == KD_GRAPHICS) > > - return ret; > > + goto out; > > > > /* > > * First, determine whether we are handling a fake keypress on > > @@ -2225,7 +2232,7 @@ static int keyboard_notifier_call(struct > > */ > > > > if (speakup_fake_key_pressed()) > > - return ret; > > + goto out; > > > > switch (code) { > > case KBD_KEYCODE: > > @@ -2266,6 +2273,8 @@ static int keyboard_notifier_call(struct > > break; > > } > > } > > +out: > > + in_keyboard_notifier = 0; > > return ret; > > } > > > > -- > 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 >