lkml.org 
[lkml]   [2007]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2007-10-11 08:55    [W:0.033 / U:7.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site