[systemd-devel] RFC : PATCH: initial implementation of system wide rlimit

[Lennart Poettering]

On Mon, 26.03.12 14:25, Frederic Crozat (fcrozat at suse.com) wrote:

Heya,

> - currently, the limits are set on both rlim_cur and rlim_max (due to
> reuse of config_parse_limit code). I'd like to extend the
> implementation to support separate values for rlim_max and rlim_cur.
> But I'm not 100% sure how we should express that in configuration
> file. Should we use a additional keyword (LimitCPU / LimitSoftCPU ) or
> a dual value (LimitCPU=100;10) ?

I decided not to expose separate values for soft/non-soft on purpose. I
have the suspicion that they only make sense for user sessions, not so
much for system services,

I am mostly waiting for a usecase here. If somebody makes a good case
for hwo they should be useful in system services we can add support for
them, but otherwise I am not convinced.

We generally only expose kernel features we think are useful, but if
they aren't they are better hidden away.


> +        for (i = 0; i < RLIMIT_NLIMITS; i++) {
> +                if (!default_rlimit[i])
> +                        if (!(default_rlimit[i]= new(struct rlimit, 1)))
> +                                break;
> +                getrlimit(i,default_rlimit[i]);
> +        }

For new code we try to write

x = new(foobar, 1);
if (!x) {
        ...
}

rather than:

if (!(x = new(foobar, 1))) {
        ...
}

Also, you need to check for OOM here. And then, if it is clear that
getrlimit() will always return 0 here, then we should make that clear by
enclosing this in "assert_se(getrlimit(...) == 0)" or so.

But in general: why this code anyway? We should just let the pointer
point to NULL if there is not rlimit set. I.e. only those array entries
with non-default values should actually point to something.

>  
>          fn = arg_running_as == MANAGER_SYSTEM ? SYSTEM_CONFIG_FILE : USER_CONFIG_FILE;
>          f = fopen(fn, "re");
> @@ -1400,6 +1424,15 @@ int main(int argc, char *argv[]) {
>          m->swap_auto = arg_swap_auto;
>          m->default_std_output = arg_default_std_output;
>          m->default_std_error = arg_default_std_error;
> +        for (j = 0; j < RLIMIT_NLIMITS; j++) {
> +                if (!m->rlimit[j])
> +                        if (!(m->rlimit[j]= new(struct rlimit, 1)))
> +                                break;
> +
> +                (m->rlimit[j])->rlim_cur = default_rlimit[j]->rlim_cur;
> +                (m->rlimit[j])->rlim_max = default_rlimit[j]->rlim_max;
> +        }

Need to handle OOM. memdup() might be nicer to use here. (In fact, might
be worth introducing newdup() here as a type-safe macro, i.e. a
combination of new() and memdup()).

Lennart

-- 
Lennart Poettering - Red Hat, Inc.