Messages in this thread | | | Date | Wed, 16 Mar 2005 15:09:29 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/2] del_timer_sync: proof of concept |
| |
Ingo Molnar wrote: > > * Oleg Nesterov <oleg@tv-sign.ru> wrote: > > > New rules: > > ->_base & 1 : is timer pending > > ->_base & ~1 : timer's base > > how would it look like if we had a separate timer->pending field after > all? Would it be faster/cleaner?
The only change visible outside kernel/timer.c is:
static inline int timer_pending(const struct timer_list * timer) { - return timer->base != NULL; + return timer->base & 1; }
Currently __get_base() usage in the kernel/time.c suboptimal and should be cleanuped, I see no other problems with performance.
> (we dont need to keep them small _that_ bad - if there's a good reason > we should rather add a clean new field than to encode two fields into > one field and complicate the code.)
I think that separate timer->pending field will require more changes, because we can't read/write base+pending atomically.
int del_timer() { again: if (!timer->pending) // not strictly necessary, but it is return 0; // nice to avoid locking base = timer->base; if (!base) return 0;
spin_lock(base->lock);
if (!timer->pending) { spin_unlock(); goto again; } if (timer->base != base) { spin_unlock(); goto again; } .... }
Note also, that we have to audit every timer->base usage anyway, because currently it mix base and pending.
But may be you are right, the encoding of a bit in a pointer is indeed weird.
Oleg. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |