Le lundi 26 mars 2012 à 18:51 +0200, Lennart Poettering a écrit :
> On Mon, 26.03.12 14:25, Frederic Crozat (fcro...@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.
Looking at the defaults shipped with openSUSE, some limits are set to
"sane" soft value (80% for soft virtual limit, 85% for soft resident
limit, etc..) and some default have different soft / hard value (fdlimit
= 8192 (hard), 1024 (soft) ; locklimit = 256KB (hard), 64KB (soft) ).
With the current implementation, we couldn't ensure such duality.
I'll ask around if people have other use cases.
Another interesting thing I didn't port from SUSE is being able to
express some of those values as percentage of system memory and not as
absolute value.
> > + 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))) {
> ...
> }
Sorry, I'm not yet used to it ;)
> 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.
I'll fix that.
> 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.
I wanted to show the correct system values (default, when not set in
systemd) when using systemctl show. Otherwise, they might look as
infinite value (I was getting that, but I might had a bug there).
> >
> > 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()).
I'll see what I can do.
--
Frederic Crozat <fcro...@suse.com>
SUSE
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
