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

[Samuel Thibault]

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 at gmail.com> wrote:
> 
> > On Thu, 4 Apr 2019 00:21:34 +0200
> > Samuel Thibault <samuel.thibault at ens-lyon.org> 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