On Wed, 04 Nov 2020, Johan Hovold wrote:
Hmm. I took at closer look at the parport code and it seems the current implementation is already racy but that removing the tasklet is going to widen that that window. Those register writes in restore() should be submitted before any later requests. Perhaps setting a flag and flushing the work in parport_prologue() could work?
Ah, I see and agree. Considering work is only deferred from restore_state() I don't even think we need a flag, no? We can let parport_prologue() just flush_work() unconditionally (right before taking the disc_mutex) which for the most part will be idle anyway. The flush_work() also becomes saner now that we'll stop rescheduling work in send_deferred_urbs(). Also, but not strictly related to this. What do you think of deferring all work in write_parport_reg_nonblock() unconditionally? I'd like to avoid that mutex_trylock() because eventually I'll be re-adding a warn in the locking code, but that would also simplify the code done here in the nonblocking irq write. I'm not at all familiar with parport, but I would think that restore_state context would not care.
On the other hand, the restore() implementation looks broken in that it doesn't actually restore the provided state. I'll go fix that up.
How did this thing ever work? Thanks, Davidlohr
