| Message ID | 20231124060644.576611-15-viro@zeniv.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] selinux: saner handling of policy reloads | expand |
On Fri, 24 Nov 2023 at 07:08, Al Viro <viro@zeniv.linux.org.uk> wrote: > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/internal.h | 1 + > include/linux/dcache.h | 3 --- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/internal.h b/fs/internal.h > index 9e9fc629f935..d9a920e2636e 100644 > --- a/fs/internal.h > +++ b/fs/internal.h > @@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *); > extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *); > extern struct dentry *__d_lookup_rcu(const struct dentry *parent, > const struct qstr *name, unsigned *seq); > +extern void d_genocide(struct dentry *); Seriously, who came up with THAT name? "Genocide" is not a nice term, not even if you ignore political correctness. Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()? Ced > > /* > * pipe.c > diff --git a/include/linux/dcache.h b/include/linux/dcache.h > index 8c5e3bdf1147..b4324d47f249 100644 > --- a/include/linux/dcache.h > +++ b/include/linux/dcache.h > @@ -243,9 +243,6 @@ extern void d_invalidate(struct dentry *); > /* only used at mount-time */ > extern struct dentry * d_make_root(struct inode *); > > -/* <clickety>-<click> the ramfs-type tree */ > -extern void d_genocide(struct dentry *); > - > extern void d_mark_tmpfile(struct file *, struct inode *); > extern void d_tmpfile(struct file *, struct inode *); > > -- > 2.39.2 > > -- Cedric Blancher <cedric.blancher@gmail.com> [https://plus.google.com/u/0/+CedricBlancher/] Institute Pasteur
On Fri, Nov 24, 2023 at 07:35:34AM +0100, Cedric Blancher wrote: > On Fri, 24 Nov 2023 at 07:08, Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > --- > > fs/internal.h | 1 + > > include/linux/dcache.h | 3 --- > > 2 files changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/fs/internal.h b/fs/internal.h > > index 9e9fc629f935..d9a920e2636e 100644 > > --- a/fs/internal.h > > +++ b/fs/internal.h > > @@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *); > > extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *); > > extern struct dentry *__d_lookup_rcu(const struct dentry *parent, > > const struct qstr *name, unsigned *seq); > > +extern void d_genocide(struct dentry *); > > Seriously, who came up with THAT name? "Genocide" is not a nice term, > not even if you ignore political correctness. > Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()? kill_them_all(), on the account of that being what it's doing? I can explain the problems with each of your suggested identifiers, if you really wish that, but I would stronly suggest taking that off-list. As for the bad words... google "jesux" someday. Yes, we do have identifiers like "kill", "abort", etc. and those are really not going anywhere; live with it.
On Fri, Nov 24, 2023 at 06:57:59AM +0000, Al Viro wrote: > > > +extern void d_genocide(struct dentry *); > > > > Seriously, who came up with THAT name? "Genocide" is not a nice term, > > not even if you ignore political correctness. > > Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()? > > kill_them_all(), on the account of that being what it's doing? To elaborate a bit: what that function does (well, tries to do - it has serious limitations, which is why there is only one caller remaining and that one is used only when nothing else can access the filesystem anymore) is "kill given dentry, along with all its children, all their children, etc." I sincerely doubt that you will be able to come up with _any_ word describing such action in any real-world context that would not come with very nasty associations. Context: a bunch of filesystems have directory tree entirely in dcache; creating a file or directory bumps the reference count of dentry in question, so instead of going back to 0 after e.g. mkdir(2) returns it is left with refcount 1, which prevents its eviction. In effect, all positive dentries in there are artificially kept busy. On rmdir(2) or unlink(2) that extra reference is dropped and they get evicted. For filesystems like e.g. tmpfs that's a fairly obvious approach - they don't *have* any backing store, so dcache is not just caching the underlying objects - it's all there is. For such filesystems there is a quick way to do an equivalent of rm -rf - simply go over the subtree you want to remove and decrement refcounts of everything positive. That's fine on filesystem shutdown, but for anything in use it is *too* quick - you'd better not do that if there are mountpoints in the subtree you are removing, etc. At the moment we have 3 callers in the kernel; one in selinuxfs, removing stale directories on policy reload (not quite safe, but if attacker can do selinux policy reload, you are beyond lost), another in simple_fill_super() failure handling (safe, since filesystem is not mounted at the time, but actually pointless - normal cleanup after failure will take them out just fine) and the last one in kill_litter_super(). That one is actually fine - we are shutting the filesystem down and nobody can access it at that point unless the kernel is deeply broken. By the end of this series only that one caller remains, which is reason for taking the declaration from include/linux/dcache.h to fs/internal.h - making sure no new callers get added. Not because of the identifier having nasty connotations, but because it's pretty hard to use correctly.
Al Viro - 24.11.23, 08:48:57 CET: > On Fri, Nov 24, 2023 at 06:57:59AM +0000, Al Viro wrote: > > > > +extern void d_genocide(struct dentry *); > > > > > > Seriously, who came up with THAT name? "Genocide" is not a nice > > > term, > > > not even if you ignore political correctness. > > > Or what will be next? d_holodomor()? d_massmurder()? > > > d_execute_warcrimes()?> > > kill_them_all(), on the account of that being what it's doing? > > To elaborate a bit: what that function does (well, tries to do - it has > serious limitations, which is why there is only one caller remaining and > that one is used only when nothing else can access the filesystem > anymore) is "kill given dentry, along with all its children, all their > children, etc." I never got why in the context of computers anything is ever being killed. It does not live to begin with. You can stop something, remove it, delete it, destroy it, pause it, resume it, overwrite it and you can do it really quickly or (almost) instantly or slowly or recursively or some combination of those, but kill? You cannot kill what does not live. d_delete/destroy/remove_recursively() could be a suitable function name. Pick one. Similar it is with the term children or parent. There are no children in computer software. Period. But here it may be more difficult to find alternative wording. Would still be good to find something, cause I was quite taken aback by the wording of the OOM killer. (Actually I was taken aback that an operating system could even have something that forcefully quits a process without saving data. It never matched my expectations of reliability and stability.) So how about stopping to put meaning into computer software source code that simply is not there to begin with? How about starting to use terms that describe what is actually being done and what is actually there?
[search bait removed from subject] On Fri, Nov 24, 2023 at 10:36:05AM +0100, Martin Steigerwald wrote: > Al Viro - 24.11.23, 08:48:57 CET: > > To elaborate a bit: what that function does (well, tries to do - it has > > serious limitations, which is why there is only one caller remaining and > > that one is used only when nothing else can access the filesystem > > anymore) is "kill given dentry, along with all its children, all their > > children, etc." > > I never got why in the context of computers anything is ever being killed. > It does not live to begin with. Simple - one deals with objects that have complex lifecycle, with very different possible behaviour at various stages. And about the only example of such that would be well-covered in natural languages is just that - both in adjectives for states and verbs for transitions between those. Note that the word "lifecycle" itself is rather commonly used outside of biological context. > You can stop something, remove it, delete it, destroy it, pause it, resume > it, overwrite it and you can do it really quickly or (almost) instantly or > slowly or recursively or some combination of those, but kill? You cannot > kill what does not live. Why? "Do something that changes the state of target into one in which the target gradually becomes incapable of normal activity until it goes completely inert and eventually disappears, with its parts recycled for unrelated objects" vs. "kill the target", with associated transitional state being refered to as "dying"? Your suggestions all refer to operation rather than state transition. > d_delete/destroy/remove_recursively() could be a suitable function name. > Pick one. Thanks, but no thanks. d_delete() already exists and refers to rather different operation; "destroy" in such contexts would be loaded with an existing technical meaning, and that would be actively confusing; "remove_recursively"? Guess what the better-behaving replacement (far too heavy-weight for the only remaining use) is called? "simple_recursive_removal". It does more than this one, though. > Similar it is with the term children or parent. There are no children in > computer software. Period. Well-asserted. Unfortunately, the statement is wrong - "parents" and "children" have specific meanings when applied to nodes of directed graphs. And there's a plenty of those dealt with by software. Directory tree, in particular.
diff --git a/fs/internal.h b/fs/internal.h index 9e9fc629f935..d9a920e2636e 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *); extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *); extern struct dentry *__d_lookup_rcu(const struct dentry *parent, const struct qstr *name, unsigned *seq); +extern void d_genocide(struct dentry *); /* * pipe.c diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 8c5e3bdf1147..b4324d47f249 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -243,9 +243,6 @@ extern void d_invalidate(struct dentry *); /* only used at mount-time */ extern struct dentry * d_make_root(struct inode *); -/* <clickety>-<click> the ramfs-type tree */ -extern void d_genocide(struct dentry *); - extern void d_mark_tmpfile(struct file *, struct inode *); extern void d_tmpfile(struct file *, struct inode *);
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/internal.h | 1 + include/linux/dcache.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)