Messages in this thread | | | Date | Thu, 24 Oct 2002 17:07:11 -0700 | From | mingming cao <> | Subject | Re: [PATCH]updated ipc lock patch |
| |
Andrew Morton wrote: > > What about this code? > > void ipc_rcu_free(void* ptr, int size) > { > struct rcu_ipc_free* arg; > > arg = (struct rcu_ipc_free *) kmalloc(sizeof(*arg), GFP_KERNEL); > if (arg == NULL) > return; > arg->ptr = ptr; > arg->size = size; > call_rcu(&arg->rcu_head, ipc_free_callback, arg); > } > > Are we sure that it's never called under locks? Did you see any place where this is called with lock(s) hold? Maybe there is, but I could not see here. They are called from the functions which are used by IPC code only. Inside IPC there is only spin_lock per ID and sem_undo lock. Both of them are not hold when ipc_rcu_free is called.
> > And it seems that if the kmalloc fails, we decide to leak some > memory, yes? >
yes.
> If so it would be better to use GFP_ATOMIC there. Avoids any > locking problems and also increases the chance of the allocation > succeeding. (With an explanatory comment, naturally :)). >
Good point. I agree GFP_ATOMIC fits better here.
> Even better: is it possible to embed the rcu_ipc_free inside the > object-to-be-freed? Perhaps not?
Are you saying that have a static RCU header structure in the object-to-be-freed? I think it's possible. It fits well in the rmid case, where the object to be freed is an kern_ipc_perm structure. But for the grow_ary() case, the object to be freed is a array of struct ipc_id, so it need a little bit more changes there. Maybe add a new structure ipc_entries, which include the RCU header structure and the pointer to the entries array. Then have the ipc_ids->entries point to ipc_entries. Just a little concern that this way we added a reference when looking up the IPC ID from the array. - 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/
| |