Skip to content

Commit 53aa85a

Browse files
WerkovWerkov
committed
cgroup: Allow empty assignments of Memory{Low,Min}=
Currently, an empty assignment of Memory{Low,Min}= directives would be interpretted as setting it to global default, i.e. zero. However, if we set a runtime protection value on a unit that inherits parent's DefaultMemory{Low,Min}=, it is not possible to revert it back to the state where the DefaultMemory{Low,Min}= is propagated from parent slice(s). This patch changes the semantics of the empty assignments to explicitly nullify any value set by the user previously. Since DBus API uses uint64_t where 0 is a valid configuration, the patch modifies DBus API by exploiting the variant type of property value to pass the NULL value.
1 parent db2b8d2 commit 53aa85a

File tree

3 files changed

+130
-60
lines changed

3 files changed

+130
-60
lines changed

‎src/core/dbus-cgroup.c‎

Lines changed: 115 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -707,14 +707,112 @@ static int bus_cgroup_set_boolean(
707707
return 1; \
708708
}
709709

710+
#define BUS_DEFINE_SET_CGROUP_PROTECTION(function, mask, scale) \
711+
static int bus_cgroup_set_##function( \
712+
Unit *u, \
713+
const char *name, \
714+
uint64_t *p, \
715+
bool *s, \
716+
sd_bus_message *message, \
717+
UnitWriteFlags flags, \
718+
sd_bus_error *error) { \
719+
\
720+
uint64_t v = CGROUP_LIMIT_MIN; \
721+
bool nonempty = true; \
722+
char type; \
723+
int r; \
724+
\
725+
assert(p); \
726+
assert(s); \
727+
\
728+
r = sd_bus_message_peek_type(message, &type, NULL); \
729+
if (r < 0) \
730+
return r; \
731+
if (type == SD_BUS_TYPE_BOOLEAN) { \
732+
r = sd_bus_message_read(message, "b", &nonempty); \
733+
if (r < 0) \
734+
return r; \
735+
/* Bool is used to denote empty value only */ \
736+
if (nonempty) \
737+
return -EINVAL; \
738+
} else if (type != SD_BUS_TYPE_UINT64) { \
739+
return -EINVAL; \
740+
} else { \
741+
r = sd_bus_message_read(message, "t", &v); \
742+
if (r < 0) \
743+
return r; \
744+
} \
745+
\
746+
if (!UNIT_WRITE_FLAGS_NOOP(flags)) { \
747+
*p = v; \
748+
unit_invalidate_cgroup(u, mask); \
749+
if (!nonempty) { \
750+
*s = false; \
751+
unit_write_settingf(u, flags, name, \
752+
"%s=", name); \
753+
} else if (v == CGROUP_LIMIT_MAX) { \
754+
*s = true; \
755+
unit_write_settingf(u, flags, name, \
756+
"%s=infinity", name); \
757+
} else { \
758+
*s = true; \
759+
unit_write_settingf(u, flags, name, \
760+
"%s=%" PRIu64, name, v); \
761+
} \
762+
} \
763+
\
764+
return 1; \
765+
} \
766+
static int bus_cgroup_set_##function##_scale( \
767+
Unit *u, \
768+
const char *name, \
769+
uint64_t *p, \
770+
bool *s, \
771+
sd_bus_message *message, \
772+
UnitWriteFlags flags, \
773+
sd_bus_error *error) { \
774+
\
775+
uint64_t v; \
776+
uint32_t raw; \
777+
int r; \
778+
\
779+
assert(p); \
780+
assert(s); \
781+
\
782+
r = sd_bus_message_read(message, "u", &raw); \
783+
if (r < 0) \
784+
return r; \
785+
\
786+
v = scale(raw, UINT32_MAX); \
787+
if (v >= UINT64_MAX) \
788+
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, \
789+
"Value specified in %s is out of range", name); \
790+
\
791+
if (!UNIT_WRITE_FLAGS_NOOP(flags)) { \
792+
*p = v; \
793+
unit_invalidate_cgroup(u, mask); \
794+
\
795+
/* Prepare to chop off suffix */ \
796+
assert_se(endswith(name, "Scale")); \
797+
\
798+
uint32_t scaled = DIV_ROUND_UP((uint64_t) raw * 1000, (uint64_t) UINT32_MAX); \
799+
unit_write_settingf(u, flags, name, "%.*s=%" PRIu32 ".%" PRIu32 "%%", \
800+
(int)(strlen(name) - strlen("Scale")), name, \
801+
scaled / 10, scaled % 10); \
802+
} \
803+
\
804+
*s = true; \
805+
return 1; \
806+
}
807+
710808
DISABLE_WARNING_TYPE_LIMITS;
711809
BUS_DEFINE_SET_CGROUP_WEIGHT(cpu_weight, CGROUP_MASK_CPU, CGROUP_WEIGHT_IS_OK, CGROUP_WEIGHT_INVALID);
712810
BUS_DEFINE_SET_CGROUP_WEIGHT(cpu_shares, CGROUP_MASK_CPU, CGROUP_CPU_SHARES_IS_OK, CGROUP_CPU_SHARES_INVALID);
713811
BUS_DEFINE_SET_CGROUP_WEIGHT(io_weight, CGROUP_MASK_IO, CGROUP_WEIGHT_IS_OK, CGROUP_WEIGHT_INVALID);
714812
BUS_DEFINE_SET_CGROUP_WEIGHT(blockio_weight, CGROUP_MASK_BLKIO, CGROUP_BLKIO_WEIGHT_IS_OK, CGROUP_BLKIO_WEIGHT_INVALID);
715813
BUS_DEFINE_SET_CGROUP_LIMIT(memory, CGROUP_MASK_MEMORY, physical_memory_scale, 1);
716-
BUS_DEFINE_SET_CGROUP_LIMIT(memory_protection, CGROUP_MASK_MEMORY, physical_memory_scale, 0);
717814
BUS_DEFINE_SET_CGROUP_LIMIT(swap, CGROUP_MASK_MEMORY, physical_memory_scale, 0);
815+
BUS_DEFINE_SET_CGROUP_PROTECTION(memory_protection, CGROUP_MASK_MEMORY, physical_memory_scale);
718816
REENABLE_WARNING;
719817

720818
static int bus_cgroup_set_tasks_max(
@@ -840,33 +938,17 @@ int bus_cgroup_set_property(
840938
if (streq(name, "MemoryAccounting"))
841939
return bus_cgroup_set_boolean(u, name, &c->memory_accounting, CGROUP_MASK_MEMORY, message, flags, error);
842940

843-
if (streq(name, "MemoryMin")) {
844-
r = bus_cgroup_set_memory_protection(u, name, &c->memory_min, message, flags, error);
845-
if (r > 0)
846-
c->memory_min_set = true;
847-
return r;
848-
}
941+
if (streq(name, "MemoryMin"))
942+
return bus_cgroup_set_memory_protection(u, name, &c->memory_min, &c->memory_min_set, message, flags, error);
849943

850-
if (streq(name, "MemoryLow")) {
851-
r = bus_cgroup_set_memory_protection(u, name, &c->memory_low, message, flags, error);
852-
if (r > 0)
853-
c->memory_low_set = true;
854-
return r;
855-
}
944+
if (streq(name, "MemoryLow"))
945+
return bus_cgroup_set_memory_protection(u, name, &c->memory_low, &c->memory_low_set, message, flags, error);
856946

857-
if (streq(name, "DefaultMemoryMin")) {
858-
r = bus_cgroup_set_memory_protection(u, name, &c->default_memory_min, message, flags, error);
859-
if (r > 0)
860-
c->default_memory_min_set = true;
861-
return r;
862-
}
947+
if (streq(name, "DefaultMemoryMin"))
948+
return bus_cgroup_set_memory_protection(u, name, &c->default_memory_min, &c->default_memory_min_set, message, flags, error);
863949

864-
if (streq(name, "DefaultMemoryLow")) {
865-
r = bus_cgroup_set_memory_protection(u, name, &c->default_memory_low, message, flags, error);
866-
if (r > 0)
867-
c->default_memory_low_set = true;
868-
return r;
869-
}
950+
if (streq(name, "DefaultMemoryLow"))
951+
return bus_cgroup_set_memory_protection(u, name, &c->default_memory_low, &c->default_memory_low_set, message, flags, error);
870952

871953
if (streq(name, "MemoryHigh"))
872954
return bus_cgroup_set_memory(u, name, &c->memory_high, message, flags, error);
@@ -880,33 +962,17 @@ int bus_cgroup_set_property(
880962
if (streq(name, "MemoryLimit"))
881963
return bus_cgroup_set_memory(u, name, &c->memory_limit, message, flags, error);
882964

883-
if (streq(name, "MemoryMinScale")) {
884-
r = bus_cgroup_set_memory_protection_scale(u, name, &c->memory_min, message, flags, error);
885-
if (r > 0)
886-
c->memory_min_set = true;
887-
return r;
888-
}
965+
if (streq(name, "MemoryMinScale"))
966+
return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_min, &c->memory_min_set, message, flags, error);
889967

890-
if (streq(name, "MemoryLowScale")) {
891-
r = bus_cgroup_set_memory_protection_scale(u, name, &c->memory_low, message, flags, error);
892-
if (r > 0)
893-
c->memory_low_set = true;
894-
return r;
895-
}
968+
if (streq(name, "MemoryLowScale"))
969+
return bus_cgroup_set_memory_protection_scale(u, name, &c->memory_low, &c->memory_low_set, message, flags, error);
896970

897-
if (streq(name, "DefaultMemoryMinScale")) {
898-
r = bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_min, message, flags, error);
899-
if (r > 0)
900-
c->default_memory_min_set = true;
901-
return r;
902-
}
971+
if (streq(name, "DefaultMemoryMinScale"))
972+
return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_min, &c->default_memory_min_set, message, flags, error);
903973

904-
if (streq(name, "DefaultMemoryLowScale")) {
905-
r = bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_low, message, flags, error);
906-
if (r > 0)
907-
c->default_memory_low_set = true;
908-
return r;
909-
}
974+
if (streq(name, "DefaultMemoryLowScale"))
975+
return bus_cgroup_set_memory_protection_scale(u, name, &c->default_memory_low, &c->default_memory_low_set, message, flags, error);
910976

911977
if (streq(name, "MemoryHighScale"))
912978
return bus_cgroup_set_memory_scale(u, name, &c->memory_high, message, flags, error);

‎src/core/load-fragment.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3396,6 +3396,8 @@ int config_parse_memory_limit(
33963396
}
33973397
}
33983398

3399+
/* Keep Memory{Low,Min} unset with empty assignment so that we fall back to DefaultMemory* which in
3400+
* contrast means zeroing the property. */
33993401
if (streq(lvalue, "DefaultMemoryLow")) {
34003402
c->default_memory_low = bytes;
34013403
c->default_memory_low_set = true;
@@ -3404,10 +3406,10 @@ int config_parse_memory_limit(
34043406
c->default_memory_min_set = true;
34053407
} else if (streq(lvalue, "MemoryMin")) {
34063408
c->memory_min = bytes;
3407-
c->memory_min_set = true;
3409+
c->memory_min_set = !isempty(rvalue);
34083410
} else if (streq(lvalue, "MemoryLow")) {
34093411
c->memory_low = bytes;
3410-
c->memory_low_set = true;
3412+
c->memory_low_set = !isempty(rvalue);
34113413
} else if (streq(lvalue, "MemoryHigh"))
34123414
c->memory_high = bytes;
34133415
else if (streq(lvalue, "MemoryMax"))

‎src/shared/bus-unit-util.c‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -494,16 +494,18 @@ static int bus_append_cgroup_property(sd_bus_message *m, const char *field, cons
494494
if (r < 0)
495495
return bus_log_create_error(r);
496496
return 1;
497+
} else if (isempty(eq) && STR_IN_SET(field, "DefaultMemoryLow",
498+
"DefaultMemoryMin",
499+
"MemoryLow",
500+
"MemoryMin")) {
501+
/* We can't use CGROUP_LIMIT_MIN nor CGROUP_LIMIT_MAX to convey the empty assignment
502+
* so marshall specially as a boolean. */
503+
r = sd_bus_message_append(m, "(sv)", field, "b", 0);
504+
if (r < 0)
505+
return bus_log_create_error(r);
506+
return 1;
497507
} else if (isempty(eq)) {
498-
uint64_t empty_value = STR_IN_SET(field,
499-
"DefaultMemoryLow",
500-
"DefaultMemoryMin",
501-
"MemoryLow",
502-
"MemoryMin") ?
503-
CGROUP_LIMIT_MIN :
504-
CGROUP_LIMIT_MAX;
505-
506-
r = sd_bus_message_append(m, "(sv)", field, "t", empty_value);
508+
r = sd_bus_message_append(m, "(sv)", field, "t", CGROUP_LIMIT_MAX);
507509
if (r < 0)
508510
return bus_log_create_error(r);
509511
return 1;

0 commit comments

Comments
 (0)