Okash Khawaja, le jeu. 04 avril 2019 10:04:42 +0100, a ecrit:
> This patch replaces speakup's implementations with calls to functions
> in tty/vt/selection.c. Those functions are:
>
> cancel_selection()
> do_set_selection()
> paste_selection()
>
> Currently setting selection is done in interrupt context. However,
> do_set_selection() can sleep - for instance, it requires console_lock
> which can sleep. Therefore we offload that work to a work_struct thread,
> following the same pattern which was already set for paste_selection().
> When setting selection, we also get a reference to tty and make sure to
> release the reference before returning.
>
> struct speakup_paste_work has been renamed to the more generic
> speakup_selection_work because it is now used for both pasting as well
> as setting selection. When paste work is cancelled, the code wasn't
> setting tty to NULL. This patch also fixes that by setting tty to NULL
> so that in case of failure we don't get EBUSY forever.
>
> Signed-off-by: Okash Khawaja <okash.khawaja@xxxxxxxxx>
Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> ---
> drivers/staging/speakup/main.c | 1 +
> drivers/staging/speakup/selection.c | 212 +++++++++++-----------------
> drivers/staging/speakup/speakup.h | 1 +
> 3 files changed, 88 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index b6a65b0c1896..488f2539aa9a 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2319,6 +2319,7 @@ static void __exit speakup_exit(void)
> unregister_keyboard_notifier(&keyboard_notifier_block);
> unregister_vt_notifier(&vt_notifier_block);
> speakup_unregister_devsynth();
> + speakup_cancel_selection();
> speakup_cancel_paste();
> del_timer_sync(&cursor_timer);
> kthread_stop(speakup_task);
> diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
> index 0ed1fefee0e9..6c6d77f8bc24 100644
> --- a/drivers/staging/speakup/selection.c
> +++ b/drivers/staging/speakup/selection.c
> @@ -9,178 +9,138 @@
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> #include <linux/atomic.h>
> +#include <linux/console.h>
>
> #include "speakup.h"
>
> -/* ------ cut and paste ----- */
> -/* Don't take this from <ctype.h>: 011-015 on the screen aren't spaces */
> -#define ishardspace(c) ((c) == ' ')
> -
> unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points */
> -
> -/* Variables for selection control. */
> -/* must not be deallocated */
> struct vc_data *spk_sel_cons;
> -/* cleared by clear_selection */
> -static int sel_start = -1;
> -static int sel_end;
> -static int sel_buffer_lth;
> -static char *sel_buffer;
>
> -static unsigned char sel_pos(int n)
> -{
> - return inverse_translate(spk_sel_cons,
> - screen_glyph(spk_sel_cons, n), 0);
> -}
> +struct speakup_selection_work {
> + struct work_struct work;
> + struct tiocl_selection sel;
> + struct tty_struct *tty;
> +};
>
> void speakup_clear_selection(void)
> {
> - sel_start = -1;
> + console_lock();
> + clear_selection();
> + console_unlock();
> }
>
> -/* does screen address p correspond to character at LH/RH edge of screen? */
> -static int atedge(const int p, int size_row)
> +static void __speakup_set_selection(struct work_struct *work)
> {
> - return !(p % size_row) || !((p + 2) % size_row);
> -}
> + struct speakup_selection_work *ssw =
> + container_of(work, struct speakup_selection_work, work);
>
> -/* constrain v such that v <= u */
> -static unsigned short limit(const unsigned short v, const unsigned short u)
> -{
> - return (v > u) ? u : v;
> -}
> + struct tty_struct *tty;
> + struct tiocl_selection sel;
>
> -int speakup_set_selection(struct tty_struct *tty)
> -{
> - int new_sel_start, new_sel_end;
> - char *bp, *obp;
> - int i, ps, pe;
> - struct vc_data *vc = vc_cons[fg_console].d;
> + sel = ssw->sel;
>
> - spk_xs = limit(spk_xs, vc->vc_cols - 1);
> - spk_ys = limit(spk_ys, vc->vc_rows - 1);
> - spk_xe = limit(spk_xe, vc->vc_cols - 1);
> - spk_ye = limit(spk_ye, vc->vc_rows - 1);
> - ps = spk_ys * vc->vc_size_row + (spk_xs << 1);
> - pe = spk_ye * vc->vc_size_row + (spk_xe << 1);
> + /* this ensures we copy sel before releasing the lock below */
> + rmb();
>
> - if (ps > pe) /* make sel_start <= sel_end */
> - swap(ps, pe);
> + /* release the lock by setting tty of the struct to NULL */
> + tty = xchg(&ssw->tty, NULL);
>
> if (spk_sel_cons != vc_cons[fg_console].d) {
> - speakup_clear_selection();
> spk_sel_cons = vc_cons[fg_console].d;
> - dev_warn(tty->dev,
> - "Selection: mark console not the same as cut\n");
> - return -EINVAL;
> + pr_warn("Selection: mark console not the same as cut\n");
> + goto unref;
> }
>
> - new_sel_start = ps;
> - new_sel_end = pe;
> -
> - /* select to end of line if on trailing space */
> - if (new_sel_end > new_sel_start &&
> - !atedge(new_sel_end, vc->vc_size_row) &&
> - ishardspace(sel_pos(new_sel_end))) {
> - for (pe = new_sel_end + 2; ; pe += 2)
> - if (!ishardspace(sel_pos(pe)) ||
> - atedge(pe, vc->vc_size_row))
> - break;
> - if (ishardspace(sel_pos(pe)))
> - new_sel_end = pe;
> - }
> - if ((new_sel_start == sel_start) && (new_sel_end == sel_end))
> - return 0; /* no action required */
> -
> - sel_start = new_sel_start;
> - sel_end = new_sel_end;
> - /* Allocate a new buffer before freeing the old one ... */
> - bp = kmalloc((sel_end - sel_start) / 2 + 1, GFP_ATOMIC);
> - if (!bp) {
> - speakup_clear_selection();
> - return -ENOMEM;
> - }
> - kfree(sel_buffer);
> - sel_buffer = bp;
> -
> - obp = bp;
> - for (i = sel_start; i <= sel_end; i += 2) {
> - *bp = sel_pos(i);
> - if (!ishardspace(*bp++))
> - obp = bp;
> - if (!((i + 2) % vc->vc_size_row)) {
> - /* strip trailing blanks from line and add newline,
> - * unless non-space at end of line.
> - */
> - if (obp != bp) {
> - bp = obp;
> - *bp++ = '\r';
> - }
> - obp = bp;
> - }
> + console_lock();
> + do_set_selection(&sel, tty);
> + console_unlock();
> +
> +unref:
> + tty_kref_put(tty);
> +}
> +
> +static struct speakup_selection_work speakup_sel_work = {
> + .work = __WORK_INITIALIZER(speakup_sel_work.work,
> + __speakup_set_selection)
> +};
> +
> +int speakup_set_selection(struct tty_struct *tty)
> +{
> + /* we get kref here first in order to avoid a subtle race when
> + * cancelling selection work. getting kref first establishes the
> + * invariant that if speakup_sel_work.tty is not NULL when
> + * speakup_cancel_selection() is called, it must be the case that a put
> + * kref is pending.
> + */
> + tty_kref_get(tty);
> + if (cmpxchg(&speakup_sel_work.tty, NULL, tty)) {
> + tty_kref_put(tty);
> + return -EBUSY;
> }
> - sel_buffer_lth = bp - sel_buffer;
> + /* now we have the 'lock' by setting tty member of
> + * speakup_selection_work. wmb() ensures that writes to
> + * speakup_sel_work don't happen before cmpxchg() above.
> + */
> + wmb();
> +
> + speakup_sel_work.sel.xs = spk_xs + 1;
> + speakup_sel_work.sel.ys = spk_ys + 1;
> + speakup_sel_work.sel.xe = spk_xe + 1;
> + speakup_sel_work.sel.ye = spk_ye + 1;
> + speakup_sel_work.sel.sel_mode = TIOCL_SELCHAR;
> +
> + schedule_work_on(WORK_CPU_UNBOUND, &speakup_sel_work.work);
> +
> return 0;
> }
>
> -struct speakup_paste_work {
> - struct work_struct work;
> +void speakup_cancel_selection(void)
> +{
> struct tty_struct *tty;
> -};
> +
> + cancel_work_sync(&speakup_sel_work.work);
> + /* setting to null so that if work fails to run and we cancel it,
> + * we can run it again without getting EBUSY forever from there on.
> + * we need to use xchg here to avoid race with speakup_set_selection()
> + */
> + tty = xchg(&speakup_sel_work.tty, NULL);
> + if (tty)
> + tty_kref_put(tty);
> +}
>
> static void __speakup_paste_selection(struct work_struct *work)
> {
> - struct speakup_paste_work *spw =
> - container_of(work, struct speakup_paste_work, work);
> - struct tty_struct *tty = xchg(&spw->tty, NULL);
> - struct vc_data *vc = (struct vc_data *)tty->driver_data;
> - int pasted = 0, count;
> - struct tty_ldisc *ld;
> - DECLARE_WAITQUEUE(wait, current);
> -
> - ld = tty_ldisc_ref(tty);
> - if (!ld)
> - goto tty_unref;
> - tty_buffer_lock_exclusive(&vc->port);
> -
> - add_wait_queue(&vc->paste_wait, &wait);
> - while (sel_buffer && sel_buffer_lth > pasted) {
> - set_current_state(TASK_INTERRUPTIBLE);
> - if (tty_throttled(tty)) {
> - schedule();
> - continue;
> - }
> - count = sel_buffer_lth - pasted;
> - count = tty_ldisc_receive_buf(ld, sel_buffer + pasted, NULL,
> - count);
> - pasted += count;
> - }
> - remove_wait_queue(&vc->paste_wait, &wait);
> - __set_current_state(TASK_RUNNING);
> + struct speakup_selection_work *ssw =
> + container_of(work, struct speakup_selection_work, work);
> + struct tty_struct *tty = xchg(&ssw->tty, NULL);
>
> - tty_buffer_unlock_exclusive(&vc->port);
> - tty_ldisc_deref(ld);
> -tty_unref:
> + paste_selection(tty);
> tty_kref_put(tty);
> }
>
> -static struct speakup_paste_work speakup_paste_work = {
> +static struct speakup_selection_work speakup_paste_work = {
> .work = __WORK_INITIALIZER(speakup_paste_work.work,
> __speakup_paste_selection)
> };
>
> int speakup_paste_selection(struct tty_struct *tty)
> {
> - if (cmpxchg(&speakup_paste_work.tty, NULL, tty))
> + tty_kref_get(tty);
> + if (cmpxchg(&speakup_paste_work.tty, NULL, tty)) {
> + tty_kref_put(tty);
> return -EBUSY;
> + }
>
> - tty_kref_get(tty);
> schedule_work_on(WORK_CPU_UNBOUND, &speakup_paste_work.work);
> return 0;
> }
>
> void speakup_cancel_paste(void)
> {
> + struct tty_struct *tty;
> +
> cancel_work_sync(&speakup_paste_work.work);
> - tty_kref_put(speakup_paste_work.tty);
> + tty = xchg(&speakup_paste_work.tty, NULL);
> + if (tty)
> + tty_kref_put(tty);
> }
> diff --git a/drivers/staging/speakup/speakup.h b/drivers/staging/speakup/speakup.h
> index e4f4f00be2dc..74fe49c2c511 100644
> --- a/drivers/staging/speakup/speakup.h
> +++ b/drivers/staging/speakup/speakup.h
> @@ -72,6 +72,7 @@ void synth_buffer_add(u16 ch);
> void synth_buffer_clear(void);
> void speakup_clear_selection(void);
> int speakup_set_selection(struct tty_struct *tty);
> +void speakup_cancel_selection(void);
> int speakup_paste_selection(struct tty_struct *tty);
> void speakup_cancel_paste(void);
> void speakup_register_devsynth(void);
> --
> 2.21.0
>
--
Samuel
tohi.cybercable.fr (212.198.0.3) si une personne se reconnait derriere
cette adresse que ce soit un pirate ou une victime qu'il se manifeste,
cette personne pourrait bien etre un petit malin
-+- Fred in NPC : Mamaaaaan, y a le routeur qui veut me hacker -+-
_______________________________________________
Speakup mailing list
Speakup@xxxxxxxxxxxxxxxxx
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
