On May 21, 2018, at 9:47 PM, Ming Lei <ming.lei@xxxxxxxxxx> wrote: > >> On Mon, May 21, 2018 at 09:16:33PM -0600, Jens Axboe wrote: >>> On 5/21/18 8:49 PM, Ming Lei wrote: >>>> On Mon, May 21, 2018 at 05:11:31PM -0600, Keith Busch wrote: >>>> This patch simplifies the timeout handling by relying on the request >>>> reference counting to ensure the iterator is operating on an inflight >>> >>> The reference counting isn't free, what is the real benefit in this way? >> >> Neither is the current scheme and locking, and this is a hell of a lot >> simpler. I'd get rid of the kref stuff and just do a simple >> atomic_dec_and_test(). Most of the time we should be uncontended on >> that, which would make it pretty darn cheap. I'd be surprised if it >> wasn't better than the current alternatives. > > The explicit memory barriers by atomic_dec_and_test() isn't free. I’m not saying it’s free. Neither is our current synchronization. > Also the double completion issue need to be fixed before discussing > this approach further. Certainly. Also not saying that the current patch is perfect. But it’s a lot more palatable than the alternatives, hence my interest. And I’d like for this issue to get solved, we seem to be a bit stuck atm. I’ll take a closer look tomorrow.