Commit d591fb56618f4f93160b477dfa25bbb1e31b0e85
1 parent
75501a6d59
Exists in
smarc-imx_3.14.28_1.0.0_ga
and in
1 other branch
device_cgroup: simplify cgroup tree walk in propagate_exception()
During a config change, propagate_exception() needs to traverse the subtree to update config on the subtree. Because such config updates need to allocate memory, it couldn't directly use cgroup_for_each_descendant_pre() which required the whole iteration to be contained in a single RCU read critical section. To work around the limitation, propagate_exception() built a linked list of descendant cgroups while read-locking RCU and then walked the list afterwards, which is safe as the whole iteration is protected by devcgroup_mutex. This works but is cumbersome. With the recent updates, cgroup iterators now allow dropping RCU read lock while iteration is in progress making this workaround no longer necessary. This patch replaces dev_cgroup->propagate_pending list and get_online_devcg() with direct cgroup_for_each_descendant_pre() walk. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Aristeu Rozanski <aris@redhat.com> Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com> Reviewed-by: Michal Hocko <mhocko@suse.cz>
Showing 1 changed file with 18 additions and 38 deletions Side-by-side Diff
security/device_cgroup.c
| ... | ... | @@ -49,8 +49,6 @@ |
| 49 | 49 | struct cgroup_subsys_state css; |
| 50 | 50 | struct list_head exceptions; |
| 51 | 51 | enum devcg_behavior behavior; |
| 52 | - /* temporary list for pending propagation operations */ | |
| 53 | - struct list_head propagate_pending; | |
| 54 | 52 | }; |
| 55 | 53 | |
| 56 | 54 | static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) |
| ... | ... | @@ -241,7 +239,6 @@ |
| 241 | 239 | if (!dev_cgroup) |
| 242 | 240 | return ERR_PTR(-ENOMEM); |
| 243 | 241 | INIT_LIST_HEAD(&dev_cgroup->exceptions); |
| 244 | - INIT_LIST_HEAD(&dev_cgroup->propagate_pending); | |
| 245 | 242 | dev_cgroup->behavior = DEVCG_DEFAULT_NONE; |
| 246 | 243 | |
| 247 | 244 | return &dev_cgroup->css; |
| ... | ... | @@ -445,34 +442,6 @@ |
| 445 | 442 | } |
| 446 | 443 | |
| 447 | 444 | /** |
| 448 | - * get_online_devcg - walks the cgroup tree and fills a list with the online | |
| 449 | - * groups | |
| 450 | - * @root: cgroup used as starting point | |
| 451 | - * @online: list that will be filled with online groups | |
| 452 | - * | |
| 453 | - * Must be called with devcgroup_mutex held. Grabs RCU lock. | |
| 454 | - * Because devcgroup_mutex is held, no devcg will become online or offline | |
| 455 | - * during the tree walk (see devcgroup_online, devcgroup_offline) | |
| 456 | - * A separated list is needed because propagate_behavior() and | |
| 457 | - * propagate_exception() need to allocate memory and can block. | |
| 458 | - */ | |
| 459 | -static void get_online_devcg(struct cgroup *root, struct list_head *online) | |
| 460 | -{ | |
| 461 | - struct cgroup *pos; | |
| 462 | - struct dev_cgroup *devcg; | |
| 463 | - | |
| 464 | - lockdep_assert_held(&devcgroup_mutex); | |
| 465 | - | |
| 466 | - rcu_read_lock(); | |
| 467 | - cgroup_for_each_descendant_pre(pos, root) { | |
| 468 | - devcg = cgroup_to_devcgroup(pos); | |
| 469 | - if (is_devcg_online(devcg)) | |
| 470 | - list_add_tail(&devcg->propagate_pending, online); | |
| 471 | - } | |
| 472 | - rcu_read_unlock(); | |
| 473 | -} | |
| 474 | - | |
| 475 | -/** | |
| 476 | 445 | * propagate_exception - propagates a new exception to the children |
| 477 | 446 | * @devcg_root: device cgroup that added a new exception |
| 478 | 447 | * @ex: new exception to be propagated |
| 479 | 448 | |
| 480 | 449 | |
| 481 | 450 | |
| 482 | 451 | |
| ... | ... | @@ -482,17 +451,26 @@ |
| 482 | 451 | static int propagate_exception(struct dev_cgroup *devcg_root, |
| 483 | 452 | struct dev_exception_item *ex) |
| 484 | 453 | { |
| 485 | - struct cgroup *root = devcg_root->css.cgroup; | |
| 486 | - struct dev_cgroup *devcg, *parent, *tmp; | |
| 454 | + struct cgroup *root = devcg_root->css.cgroup, *pos; | |
| 487 | 455 | int rc = 0; |
| 488 | - LIST_HEAD(pending); | |
| 489 | 456 | |
| 490 | - get_online_devcg(root, &pending); | |
| 457 | + rcu_read_lock(); | |
| 491 | 458 | |
| 492 | - list_for_each_entry_safe(devcg, tmp, &pending, propagate_pending) { | |
| 493 | - parent = cgroup_to_devcgroup(devcg->css.cgroup->parent); | |
| 459 | + cgroup_for_each_descendant_pre(pos, root) { | |
| 460 | + struct dev_cgroup *devcg = cgroup_to_devcgroup(pos); | |
| 494 | 461 | |
| 495 | 462 | /* |
| 463 | + * Because devcgroup_mutex is held, no devcg will become | |
| 464 | + * online or offline during the tree walk (see on/offline | |
| 465 | + * methods), and online ones are safe to access outside RCU | |
| 466 | + * read lock without bumping refcnt. | |
| 467 | + */ | |
| 468 | + if (!is_devcg_online(devcg)) | |
| 469 | + continue; | |
| 470 | + | |
| 471 | + rcu_read_unlock(); | |
| 472 | + | |
| 473 | + /* | |
| 496 | 474 | * in case both root's behavior and devcg is allow, a new |
| 497 | 475 | * restriction means adding to the exception list |
| 498 | 476 | */ |
| 499 | 477 | |
| ... | ... | @@ -512,8 +490,10 @@ |
| 512 | 490 | } |
| 513 | 491 | revalidate_active_exceptions(devcg); |
| 514 | 492 | |
| 515 | - list_del_init(&devcg->propagate_pending); | |
| 493 | + rcu_read_lock(); | |
| 516 | 494 | } |
| 495 | + | |
| 496 | + rcu_read_unlock(); | |
| 517 | 497 | return rc; |
| 518 | 498 | } |
| 519 | 499 |