Okash Khawaja, le ven. 29 mars 2019 11:31:04 +0000, a ecrit:
> + sel = ssw->sel;
> + /* this ensures we copy sel before releasing the lock below */
> + mb();
Make it an rmb() for a small efficiency improvement, but also better
documentation of what we actually need here.
> +int speakup_set_selection(struct tty_struct *tty)
> +{
> + if (cmpxchg(&speakup_sel_work.tty, NULL, tty))
> + return -EBUSY;
> + /* now we have the 'lock' by setting tty member of
> + * speakup_selection_work
> + */
You should should add a wmb() here as well, to make sure that writes
below do not make it before the cmpxchg.
> + tty_kref_get(tty);
> +
> + 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);
> +void speakup_cancel_selection(void)
> +{
> + cancel_work_sync(&speakup_sel_work.work);
> + tty_kref_put(speakup_sel_work.tty);
I believe you also need to set speakup_sel_work.tty to NULL, otherwise
if the work didn't manage to run speakup_set_selection() will always
fail with EBUSY?
Samuel
_______________________________________________
Speakup mailing list
Speakup@xxxxxxxxxxxxxxxxx
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
