Re: [PATCH][RFC] Linux VM hooks for advanced RDMA NICs

From: David Addison
Date: Thu Apr 28 2005 - 04:25:23 EST

Brice Goglin wrote:
@@ -267,6 +270,11 @@
unsigned long hiwater_rss; /* High-water RSS usage */
unsigned long hiwater_vm; /* High-water virtual memory usage */
+ /* hooks for io devices with advanced RDMA capabilities */
+ struct ioproc_ops *ioproc_ops;

+ioproc_register_ops(struct mm_struct *mm, struct ioproc_ops *ip)
+ ip->next = mm->ioproc_ops;
+ mm->ioproc_ops = ip;
+ return 0;
+ioproc_unregister_ops(struct mm_struct *mm, struct ioproc_ops *ip)
+ struct ioproc_ops **tmp;
+ for (tmp = &mm->ioproc_ops; *tmp && *tmp != ip; tmp= &(*tmp)->next)
+ ;
+ if (*tmp) {
+ *tmp = ip->next;
+ return 0;
+ }
+ return -EINVAL;

You don't seem to use any synchronization mechanism to protect the
ioproc list from concurrent modifications, right ?
I understand that it might be useless as long as QsNet is the only user
of ioprocs and takes care of locking the address space somewhere in the
driver before adding/removing hooks.
But, if this patch is to be merged to the mainline, you probably need
to do something here. It's not clear how other in-kernel users
(IB, Myri, Ammasso, ...) might use ioprocs.
And actually, I think all ioproc list traversal need to be protected
as well.

All ioproc list traversal is protected by the mm->page_table_lock which is
held at all points where the callbacks are invoked.
[Actually there is one case where this isn't true, which I'll fix
when we refresh this patch later today]

The registration/unregister functions also need to be called holding this
spinlock, our device driver does this, but perhaps we need to document
that requirement more clearly.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at