Re: [PATCH] file: kill unnecessary timer in fdtable_defer

From: Dipankar Sarma
Date: Mon Aug 21 2006 - 03:52:59 EST


On Mon, Aug 21, 2006 at 02:18:16PM +0900, Tejun Heo wrote:
> On Mon, Aug 21, 2006 at 10:02:57AM +0530, Dipankar Sarma wrote:
> > > regardless of its return value. 0 return simply means that the work
> > > was already pending and thus no further action was required.
> >
> > Hmm.. Is this really true ? IIRC, schedule_work() checks pending
> > work based on bit ops on work->pending and clear_bit() is not
> > a memory barrier.
>
> Those bitops are not memory barriers but they can define order between
> them alright. Once the execution order is correct, the rest of

Huh ? If they are not memory barriers, they how can you guranttee
ordering on weakly ordered CPUs ?


> In workqueue, this is guaranteed by
>
> 1. If pending bit is set, the work is guaranteed to be executed in
> some future - it's on timer or queue.
>
> 2. The queuer sets the pending bit *after* producing a job to be
> done.
>
> 3. The worker clears the pending *before* executing the job.
>
> I sometimes find it easier to understand with a diagram which looks
> like the following. Time flows from top to bottom.
>
> Queuer Worker
>
> -------------
> | produce job |
> | | <--- clr pending --->
> ------------- |
> | v
> v --------------
> <--- set pending ---> | consume jobs |
> | |
> --------------
>
> Now move the queuer and worker up and down in your mind. If 'set
> pending' is higher than clr pending 'consume job' is guaranteed to see
> the job queuer has produced whether pending is set or clear
> beforehand. If 'set pending' is lower than 'clr pending', it is
> guaranteed to set pending and schedule worker. The workqueue is
> straight-forward expansion where there are N queuers and a repeating
> consumer.

Given that there is no smp_mb__after_clear_bit() after clearing
work->pending, what prevents the worker thread from seeing
the state of the deferred fd queue before setting the pending bit ?
IOW, the queuer sees pending = 1 and ignores waking the
worker thread, worker sees a stale state of the deferred fd queue
ignoring the newly queued work. That should be possible on
a cpu with weak memory ordering. Perhaps, we should fix
__queue_work() to add the smp_mb__after_clear_bit() and
make sure that we have a memory barrier after adding the
deferred fds.

Thanks
Dipankar


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/