[PATCH 2/2] Subject: staging: speakup: refactor to use existing code in vt

[Samuel Thibault]

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