Okash Khawaja, le jeu. 04 avril 2019 10:04:09 +0100, a ecrit: > On Thu, 4 Apr 2019 08:00:24 +0100 > Okash Khawaja <okash.khawaja@xxxxxxxxx> wrote: > > > On Thu, 4 Apr 2019 00:21:34 +0200 > > Samuel Thibault <samuel.thibault@xxxxxxxxxxxx> wrote: > > > > > But can't just just get the kref in set_selection before using > > > cmpxchg? and if that fails, put it back. If it succeeded, the work > > > schedule will happen, and either it will manage to take the tty and > > > put its kref, or be canceled, and the kref will be put as well. > > > > Thinking about it, we can also make use of return value of > > cancel_work_sync(). It returns true if work was pending. So, if no > > work was pending then we don't need to do anything as the kref would > > be balanced and the tty would be set to NULL by the time > > cancel_work_sync() returns. This would make speakup_cancel_selection() > > simpler: > > > > void speakup_cancel_selection(void) > > { > > struct tty_struct *tty; > > bool was_pending; > > > > was_pending = cancel_work_sync(&speakup_sel_work.work); > > if (!was_pending) > > return; > > > > tty_kref_put(speakup_sel_work.tty); > > speakup_sel_work.tty = NULL; > > } > > > > We will still need to get kref before cmpxchg() as you suggested > > above. > > I think using xchg() in speakup_cancel_work() is better because it > covers a wider region (from cmpxchg() in speakup_set_selection() > onwards) as compared to checking return value of cancel_work_sync() > (which covers the region from schedule_work_on() in > speakup_set_selection()). Right. Samuel _______________________________________________ Speakup mailing list Speakup@xxxxxxxxxxxxxxxxx http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup