On Wed, Mar 23, 2011 at 2:08 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Wed, Mar 23, 2011 at 02:04:28AM -0700, Dan Williams wrote: >> Not much beyond the defined semantics of cancel_delayed_work and >> delayed_work_pending. I was looking to eliminate the current open >> coded equivalent functionality. > > What's the problem with del_timer_sync and timer_pending? Actually none. I mistakenly thought delayed_work_pending would return true for executing work as well... so the only remaining benefit of delayed_work over the timer api is flush_workqueue(). One of the fixes I made in that interim timer api cleanup (8d32e5b2) was to use del_timer_sync on all pre-allocated timers to make sure nothing fired or was in the process of firing as the driver was brought down (unlikely). The core runs under spin_lock_irqsave(ihost->scic_lock) context, and the expectation is that all these timeout callbacks are done in that context. As a result we can't call del_timer_sync from the core to cancel a timer because that might deadlock on a timer event that is spinning on the lock. Instead we need to set a "cancelled" flag that the timer callback will evaluate while holding the lock to check if it was cancelled while wait to acquire. I was thinking "!delayed_work_pending()" could stand-in for "cancelled", but it can't. So we need a different "flush all pending events" mechanism once the timers are no longer globally allocated, and that's why I suggested delayed_work. The core will have called cancel_delayed_work on all pending items. >> I also want to get rid of the pre-allocation, which I assume was the >> reason for the original comment? > > The preallocation is even more pointless. Just embedd the struct > timer_list where the code currently has a pointer to the timer object. Exactly. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
