Re: [TOMOYO 05/15](repost) Domain transition handler functions.

[James Morris <jmorris@namei.org>]

On Tue, 2 Oct 2007, Tetsuo Handa wrote:

> Hello.
> 
> Thank you for your comment.
> 
> James Morris wrote:
> > Why do you need to avoid spinlocks for these manipulations?
> 
> I don't need to use spinlocks here.
> What I need to do here is avoid read/write reordering,
> so mb() will be appropriate here.
> 
> struct something {
>   struct something *next;
>   void *data;
> };
> 
> struct something *prev_ptr = ...;
> struct something *new_ptr = kmalloc(sizeof(*new_ptr), GFP_KERNEL);
> new_ptr->next = NULL;
> new_ptr->data = some_value;
> mb();
> prev_ptr->next = new_ptr;

You're introducing a custom API, which is open-coded repeatedly throughout 
your module.

All linked lists (at least, new ones) must use the standard kernel list
API.

> Performance is not critical because this mb() is called
> only when appending new entry to the singly-linked list.

Most of these uses appear to be slow path or nested inside other locks, 
while overall, performance is likely to be dominated by your string 
matching and permission checking.  Use of mb(), which is typically 
considered less understandable, in this case does not appear to be 
justified vs. normal locking, and you also need to use the standard list 
API.  If performance really is an issue, then consider RCU.


- James
-- 
James Morris
<jmorris@namei.org>
-
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/