Messages in this thread | | | Date | Thu, 11 Oct 2007 08:58:16 +0200 | From | Nadia Derbey <> | Subject | Re: [PATCH] IPC: fix error case when idr-cache is empty in ipcget() |
| |
Pierre Peiffer wrote: > With the use of idr to store the ipc, the case where the idr cache is > empty, when idr_get_new is called (this may happen even if we call > idr_pre_get() before), is not well handled: it lets semget()/shmget()/msgget() > return ENOSPC when this cache is empty, what 1. does not reflect the facts > and 2. does not conform to the man(s). > > This patch fixes this by retrying the whole process of allocation in this case. > > Note: we could directly return ENOMEM if idr_pre_get() fails, but it does not > mean that the cache is empty... >
Pierre,
I agree with everythying, but not on idr_pre_get() failure: I think we should give up in that case: the tests on idr_pre_get() return code should remain IMHO (see after).
><snip> > > @@ -309,17 +309,20 @@ int ipc_addid(struct ipc_ids* ids, struc > int ipcget_new(struct ipc_namespace *ns, struct ipc_ids *ids, > struct ipc_ops *ops, struct ipc_params *params) > { > - int err; > - > - err = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL); > - > - if (!err) > - return -ENOMEM; > + int err, alloc; > +retry: > + alloc = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL); > > mutex_lock(&ids->mutex); > err = ops->getnew(ns, params); > mutex_unlock(&ids->mutex); > > + if (err == -EAGAIN) { > + if (alloc) > + goto retry; > + else > + err = -ENOMEM; > + } > return err; > }
ops->getnew will call idr_get_new() that should not be called if idr_pre_get() has failed. So I think that we should give up if idr_pre_get() fails and the test on idr_pre_get() return code should not be removed.
> > @@ -372,9 +375,9 @@ int ipcget_public(struct ipc_namespace * > { > struct kern_ipc_perm *ipcp; > int flg = params->flg; > - int err; > - > - err = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL); > + int err, alloc; > +retry: > + alloc = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL); > > mutex_lock(&ids->mutex); > ipcp = ipc_findkey(ids, params->key); > @@ -382,8 +385,6 @@ int ipcget_public(struct ipc_namespace * > /* key not used */ > if (!(flg & IPC_CREAT)) > err = -ENOENT; > - else if (!err) > - err = -ENOMEM; > else > err = ops->getnew(ns, params);
Same remark as above: ops->getnew will call idr_get_new() that should not be called if idr_pre_get() has failed. So I think that we should give up if idr_pre_get() fails and this this test should not be removed.
Regards, Nadia
- 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/
| |