Joern Engel pointed this out. The exposed "if" scares me: noone uses
it in a dangerous way yet, but it's (1) damn ugly, and (2) waiting to
trap someone.
The task_for_first() usage I left with a FIXME: probably better would
be to introduce:
#define task_first(head) \
(list_empty(head) ? NULL \
: list_entry((head)->next, struct rpc_task, tk_list))
Then change the callsites to use the more straightforward:
if ((task = task_first(head)) != NULL) {
But that's a patch for another day (volunteers?).
Rusty.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.4.21-pre4/include/linux/sunrpc/sched.h working-2.4.21-pre4-rest-non_drivers/include/linux/sunrpc/sched.h
--- linux-2.4.21-pre4/include/linux/sunrpc/sched.h 2003-01-30 14:01:03.000000000 +1100
+++ working-2.4.21-pre4-rest-non_drivers/include/linux/sunrpc/sched.h 2003-02-11 16:05:10.000000000 +1100
@@ -85,18 +85,17 @@ struct rpc_task {
#define tk_xprt tk_client->cl_xprt
/* support walking a list of tasks on a wait queue */
-#define task_for_each(task, pos, head) \
- list_for_each(pos, head) \
- if ((task=list_entry(pos, struct rpc_task, tk_list)),1)
+#define task_for_each(task, head) \
+ list_for_each_entry(task, head, tk_list)
+/* FIXME: Exposed "if" in macro is dangerous. */
#define task_for_first(task, head) \
if (!list_empty(head) && \
((task=list_entry((head)->next, struct rpc_task, tk_list)),1))
/* .. and walking list of all tasks */
#define alltask_for_each(task, pos, head) \
- list_for_each(pos, head) \
- if ((task=list_entry(pos, struct rpc_task, tk_task)),1)
+ list_for_each_entry(task, head, tk_task)
typedef void (*rpc_action)(struct rpc_task *);
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.4.21-pre4/net/sunrpc/sched.c working-2.4.21-pre4-rest-non_drivers/net/sunrpc/sched.c
--- linux-2.4.21-pre4/net/sunrpc/sched.c 2002-12-02 17:33:51.000000000 +1100
+++ working-2.4.21-pre4-rest-non_drivers/net/sunrpc/sched.c 2003-02-11 16:06:05.000000000 +1100
@@ -886,10 +886,9 @@ static inline struct rpc_task *
rpc_find_parent(struct rpc_task *child)
{
struct rpc_task *task, *parent;
- struct list_head *le;
parent = (struct rpc_task *) child->tk_calldata;
- task_for_each(task, le, &childq.tasks)
+ task_for_each(task, &childq.tasks)
if (task == parent)
return parent;
@@ -947,7 +946,6 @@ void
rpc_killall_tasks(struct rpc_clnt *clnt)
{
struct rpc_task *rovr;
- struct list_head *le;
dprintk("RPC: killing all tasks for client %p\n", clnt);
@@ -955,7 +953,7 @@ rpc_killall_tasks(struct rpc_clnt *clnt)
* Spin lock all_tasks to prevent changes...
*/
spin_lock(&rpc_sched_lock);
- alltask_for_each(rovr, le, &all_tasks)
+ alltask_for_each(rovr, &all_tasks)
if (!clnt || rovr->tk_client == clnt) {
rovr->tk_flags |= RPC_TASK_KILLED;
rpc_exit(rovr, -EIO);
@@ -1138,7 +1136,6 @@ out:
#ifdef RPC_DEBUG
void rpc_show_tasks(void)
{
- struct list_head *le;
struct rpc_task *t;
spin_lock(&rpc_sched_lock);
@@ -1148,7 +1145,7 @@ void rpc_show_tasks(void)
}
printk("-pid- proc flgs status -client- -prog- --rqstp- -timeout "
"-rpcwait -action- --exit--\n");
- alltask_for_each(t, le, &all_tasks)
+ alltask_for_each(t, &all_tasks)
printk("%05d %04d %04x %06d %8p %6d %8p %08ld %8s %8p %8p\n",
t->tk_pid, t->tk_msg.rpc_proc, t->tk_flags, t->tk_status,
t->tk_client, t->tk_client->cl_prog,
-- Anyone who quotes me in their sig is an idiot. -- Rusty Russell. - 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/
This archive was generated by hypermail 2b29 : Sat Feb 15 2003 - 22:00:31 EST