On Mon, May 12, 2014 at 02:56:17PM +0800, Lai Jiangshan wrote:
> worker_idr has the iteration(iterating for attached workers) and worker ID
^
Please put a space before an opening parenthses. It becomes a lot
easier on the eyes.
> duties. These two duties are not necessary tied together. We can separate
necessarily
> them and use a list for tracking attached workers and iteration
>
> After separation, we can add the rescuer workers to the list for iteration
> in future. worker_idr can't add rescuer workers due to rescuer workers
> can't allocate id from worker_idr.
Explaining how that'd be beneficial and justifies this change would be
nice.
...
> /**
> - * for_each_pool_worker - iterate through all workers of a worker_pool
> - * @worker: iteration cursor
> - * @wi: integer used for iteration
> - * @pool: worker_pool to iterate workers of
> - *
> - * This must be called with @pool->manager_mutex.
> - *
> - * The if/else clause exists only for the lockdep assertion and can be
> - * ignored.
> - */
> -#define for_each_pool_worker(worker, wi, pool)
> \
> - idr_for_each_entry(&(pool)->worker_idr, (worker), (wi)) \
> - if (({ lockdep_assert_held(&pool->manager_mutex); false; })) {
> } \
> - else
Hmmm.... don't we still want the lockdep protection?
> @@ -1772,6 +1759,8 @@ static struct worker *create_worker(struct worker_pool
> *pool)
>
> /* successful, commit the pointer to idr */
/* successful, commit it to idr and link on the workers list */
> idr_replace(&pool->worker_idr, worker, worker->id);
> + /* successful, attach the worker to the pool */
and lose the above line.
> diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h
> index 7e2204d..c44abc3 100644
> --- a/kernel/workqueue_internal.h
> +++ b/kernel/workqueue_internal.h
> @@ -37,6 +37,7 @@ struct worker {
> struct task_struct *task; /* I: worker task */
> struct worker_pool *pool; /* I: the associated pool */
> /* L: for rescuers */
> + struct list_head node; /* M: attached to the pool */
/* M: anchored at pool->workers
*/
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
