[RFC,73/86] treewide: security: remove cond_resched()

Commit Message

Ankur Arora Nov. 7, 2023, 11:08 p.m. UTC

There are broadly three sets of uses of cond_resched():

1.  Calls to cond_resched() out of the goodness of our heart,
    otherwise known as avoiding lockup splats.

2.  Open coded variants of cond_resched_lock() which call
    cond_resched().

3.  Retry or error handling loops, where cond_resched() is used as a
    quick alternative to spinning in a tight-loop.

When running under a full preemption model, the cond_resched() reduces
to a NOP (not even a barrier) so removing it obviously cannot matter.

But considering only voluntary preemption models (for say code that
has been mostly tested under those), for set-1 and set-2 the
scheduler can now preempt kernel tasks running beyond their time
quanta anywhere they are preemptible() [1]. Which removes any need
for these explicitly placed scheduling points.

The cond_resched() calls in set-3 are a little more difficult.
To start with, given it's NOP character under full preemption, it
never actually saved us from a tight loop.
With voluntary preemption, it's not a NOP, but it might as well be --
for most workloads the scheduler does not have an interminable supply
of runnable tasks on the runqueue.

So, cond_resched() is useful to not get softlockup splats, but not
terribly good for error handling. Ideally, these should be replaced
with some kind of timed or event wait.
For now we use cond_resched_stall(), which tries to schedule if
possible, and executes a cpu_relax() if not.

All the cond_resched() calls are to avoid monopolizing the CPU while
executing in long loops (set-1 or set-2).

Remove them.

[1] https://lore.kernel.org/lkml/[e-mail]/

Cc: David Howells <[e-mail]>
Cc: Jarkko Sakkinen <[e-mail]>
Signed-off-by: Ankur Arora <[e-mail]>
---
 security/keys/gc.c             | 1 -
 security/landlock/fs.c         | 1 -
 security/selinux/ss/hashtab.h  | 2 --
 security/selinux/ss/policydb.c | 6 ------
 security/selinux/ss/services.c | 1 -
 security/selinux/ss/sidtab.c   | 1 -
 6 files changed, 12 deletions(-)

Patch

diff --git a/security/keys/gc.c b/security/keys/gc.c
index 3c90807476eb..edb886df2d82 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -265,7 +265,6 @@  static void key_garbage_collector(struct work_struct *work)
 
 maybe_resched:
 	if (cursor) {
-		cond_resched();
 		spin_lock(&key_serial_lock);
 		goto continue_scanning;
 	}
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 1c0c198f6fdb..e7ecd8cca418 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1013,7 +1013,6 @@  static void hook_sb_delete(struct super_block *const sb)
 			 * previous loop walk, which is not needed anymore.
 			 */
 			iput(prev_inode);
-			cond_resched();
 			spin_lock(&sb->s_inode_list_lock);
 		}
 		prev_inode = inode;
diff --git a/security/selinux/ss/hashtab.h b/security/selinux/ss/hashtab.h
index f9713b56d3d0..1e297dd83b3e 100644
--- a/security/selinux/ss/hashtab.h
+++ b/security/selinux/ss/hashtab.h
@@ -64,8 +64,6 @@  static inline int hashtab_insert(struct hashtab *h, void *key, void *datum,
 	u32 hvalue;
 	struct hashtab_node *prev, *cur;
 
-	cond_resched();
-
 	if (!h->size || h->nel == HASHTAB_MAX_NODES)
 		return -EINVAL;
 
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 2d528f699a22..2737b753d9da 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -336,7 +336,6 @@  static int filenametr_destroy(void *key, void *datum, void *p)
 		kfree(d);
 		d = next;
 	} while (unlikely(d));
-	cond_resched();
 	return 0;
 }
 
@@ -348,7 +347,6 @@  static int range_tr_destroy(void *key, void *datum, void *p)
 	ebitmap_destroy(&rt->level[0].cat);
 	ebitmap_destroy(&rt->level[1].cat);
 	kfree(datum);
-	cond_resched();
 	return 0;
 }
 
@@ -786,7 +784,6 @@  void policydb_destroy(struct policydb *p)
 	struct role_allow *ra, *lra = NULL;
 
 	for (i = 0; i < SYM_NUM; i++) {
-		cond_resched();
 		hashtab_map(&p->symtab[i].table, destroy_f[i], NULL);
 		hashtab_destroy(&p->symtab[i].table);
 	}
@@ -802,7 +799,6 @@  void policydb_destroy(struct policydb *p)
 	avtab_destroy(&p->te_avtab);
 
 	for (i = 0; i < OCON_NUM; i++) {
-		cond_resched();
 		c = p->ocontexts[i];
 		while (c) {
 			ctmp = c;
@@ -814,7 +810,6 @@  void policydb_destroy(struct policydb *p)
 
 	g = p->genfs;
 	while (g) {
-		cond_resched();
 		kfree(g->fstype);
 		c = g->head;
 		while (c) {
@@ -834,7 +829,6 @@  void policydb_destroy(struct policydb *p)
 	hashtab_destroy(&p->role_tr);
 
 	for (ra = p->role_allow; ra; ra = ra->next) {
-		cond_resched();
 		kfree(lra);
 		lra = ra;
 	}
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 1eeffc66ea7d..0cb652456256 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2790,7 +2790,6 @@  int security_get_user_sids(u32 fromsid,
 					  &dummy_avd);
 		if (!rc)
 			mysids2[j++] = mysids[i];
-		cond_resched();
 	}
 	kfree(mysids);
 	*sids = mysids2;
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index d8ead463b8df..c5537cecb755 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -415,7 +415,6 @@  static int sidtab_convert_tree(union sidtab_entry_inner *edst,
 			(*pos)++;
 			i++;
 		}
-		cond_resched();
 	}
 	return 0;
 }