| Message ID | 20250401050847.1071675-1-mjguzik@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs: remove stale log entries from fs/namei.c | expand |
On Tue, 01 Apr 2025 07:08:46 +0200, Mateusz Guzik wrote:
>
I have zero attachment to these comments so I'm inclined to agree and
remove them. Please anyone who really really thinks we need them speak
up!
---
Applied to the vfs-6.16.misc branch of the vfs/vfs.git tree.
Patches in the vfs-6.16.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.16.misc
[1/1] fs: remove stale log entries from fs/namei.c
https://git.kernel.org/vfs/vfs/c/3dddecbd2b47
On Tue, Apr 1, 2025 at 12:49 PM Christian Brauner <brauner@kernel.org> wrote: > > On Tue, 01 Apr 2025 07:08:46 +0200, Mateusz Guzik wrote: > > > > I have zero attachment to these comments so I'm inclined to agree and > remove them. Please anyone who really really thinks we need them speak > up! ouch man this submission was a joke, which is why I only sent it to the list and skipped the maintainers as direct recipients it *adds* the following: > /*[Apr 1 2024 Mateusz Guzik] Removed stale log entries. I can't tell if this actually landed because the url: > [1/1] fs: remove stale log entries from fs/namei.c > https://git.kernel.org/vfs/vfs/c/3dddecbd2b47 says "bad object id" at the moment. I very much support removal of this kind of commentary, but this could be very flamewar inducing and I did not want to spend time on a non-tech discussion about it. However, if actually doing this, there is more to whack and I'll be happy to do a real submission with more files. Even in this file alone: > /* In order to reduce some races, while at the same time doing additional > * checking and hopefully speeding things up, we copy filenames to the > * kernel data space before using them.. I think this comment also needs to get whacked. Copying the path is not optional.
On Tue, Apr 01, 2025 at 02:28:08PM +0200, Mateusz Guzik wrote: > On Tue, Apr 1, 2025 at 12:49 PM Christian Brauner <brauner@kernel.org> wrote: > > > > On Tue, 01 Apr 2025 07:08:46 +0200, Mateusz Guzik wrote: > > > > > > > I have zero attachment to these comments so I'm inclined to agree and > > remove them. Please anyone who really really thinks we need them speak > > up! > > ouch man > > this submission was a joke, which is why I only sent it to the list > and skipped the maintainers as direct recipients > > it *adds* the following: > > /*[Apr 1 2024 Mateusz Guzik] Removed stale log entries. > > I can't tell if this actually landed because the url: > > [1/1] fs: remove stale log entries from fs/namei.c > > https://git.kernel.org/vfs/vfs/c/3dddecbd2b47 > > says "bad object id" at the moment. > > I very much support removal of this kind of commentary, but this could > be very flamewar inducing and I did not want to spend time on a > non-tech discussion about it. > > However, if actually doing this, there is more to whack and I'll be > happy to do a real submission with more files. > > Even in this file alone: > > /* In order to reduce some races, while at the same time doing additional > > * checking and hopefully speeding things up, we copy filenames to the > > * kernel data space before using them.. > > I think this comment also needs to get whacked. Copying the path is > not optional. I'm thoroughly confused how this would be a meaningful April fools joke? The comments in that file are literally 20+ years old and no one has ever bothered to add new updates there even though Al, Neil, Jeff, myself and a lot of others probably rewrote that file a gazillion number of times together or significantly or at least subtly changed the rules. So should we have added comments to the top of the file each time? And since we didn't does it really serve as an interesting historical log? The proper place for that has been Documentation/filesystems/{locking.rst,porting.rst,path_lookup.rst} for a long time now. The comments are historical artifacts. At best they serve as a humble-brag about who massaged what. I venture a guess and think that no one needs that comment to figure out who has made a significant impact here.
On Thu, Apr 3, 2025 at 10:39 AM Christian Brauner <brauner@kernel.org> wrote: > I'm thoroughly confused how this would be a meaningful April fools joke? > > The comments in that file are literally 20+ years old and no one has > ever bothered to add new updates there even though Al, Neil, Jeff, > myself and a lot of others probably rewrote that file a gazillion number > of times together or significantly or at least subtly changed the rules. > I agree they need to go. The joke part was not the removal, but the addition my own log entry stating the removal has happened. A genuine removal might have ran into opposition and I'm happy it did not. That said, my "submission" does not even have a commit message. Perhaps it would be most expedient if you committed the removal and repurposed part of your response. No need to mention in any capacity in such a change.
On Thu, Apr 3, 2025 at 12:39 PM Mateusz Guzik <mjguzik@gmail.com> wrote: > > On Thu, Apr 3, 2025 at 10:39 AM Christian Brauner <brauner@kernel.org> wrote: > > I'm thoroughly confused how this would be a meaningful April fools joke? > > > > The comments in that file are literally 20+ years old and no one has > > ever bothered to add new updates there even though Al, Neil, Jeff, > > myself and a lot of others probably rewrote that file a gazillion number > > of times together or significantly or at least subtly changed the rules. > > > > I agree they need to go. The joke part was not the removal, but the > addition my own log entry stating the removal has happened. > > A genuine removal might have ran into opposition and I'm happy it did not. > > That said, my "submission" does not even have a commit message. > > Perhaps it would be most expedient if you committed the removal and > repurposed part of your response. No need to mention in any capacity > in such a change. > -- I see I dropped a word: no need to mention me in the commit, just in case :)
diff --git a/fs/namei.c b/fs/namei.c index 360a86ca1f02..b125dadd7100 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -5,14 +5,7 @@ * Copyright (C) 1991, 1992 Linus Torvalds */ -/* - * Some corrections by tytso. - */ - -/* [Feb 1997 T. Schoebel-Theuer] Complete rewrite of the pathname - * lookup logic. - */ -/* [Feb-Apr 2000, AV] Rewrite to the new namespace architecture. +/*[Apr 1 2024 Mateusz Guzik] Removed stale log entries. */ #include <linux/init.h> @@ -44,77 +37,6 @@ #include "internal.h" #include "mount.h" -/* [Feb-1997 T. Schoebel-Theuer] - * Fundamental changes in the pathname lookup mechanisms (namei) - * were necessary because of omirr. The reason is that omirr needs - * to know the _real_ pathname, not the user-supplied one, in case - * of symlinks (and also when transname replacements occur). - * - * The new code replaces the old recursive symlink resolution with - * an iterative one (in case of non-nested symlink chains). It does - * this with calls to <fs>_follow_link(). - * As a side effect, dir_namei(), _namei() and follow_link() are now - * replaced with a single function lookup_dentry() that can handle all - * the special cases of the former code. - * - * With the new dcache, the pathname is stored at each inode, at least as - * long as the refcount of the inode is positive. As a side effect, the - * size of the dcache depends on the inode cache and thus is dynamic. - * - * [29-Apr-1998 C. Scott Ananian] Updated above description of symlink - * resolution to correspond with current state of the code. - * - * Note that the symlink resolution is not *completely* iterative. - * There is still a significant amount of tail- and mid- recursion in - * the algorithm. Also, note that <fs>_readlink() is not used in - * lookup_dentry(): lookup_dentry() on the result of <fs>_readlink() - * may return different results than <fs>_follow_link(). Many virtual - * filesystems (including /proc) exhibit this behavior. - */ - -/* [24-Feb-97 T. Schoebel-Theuer] Side effects caused by new implementation: - * New symlink semantics: when open() is called with flags O_CREAT | O_EXCL - * and the name already exists in form of a symlink, try to create the new - * name indicated by the symlink. The old code always complained that the - * name already exists, due to not following the symlink even if its target - * is nonexistent. The new semantics affects also mknod() and link() when - * the name is a symlink pointing to a non-existent name. - * - * I don't know which semantics is the right one, since I have no access - * to standards. But I found by trial that HP-UX 9.0 has the full "new" - * semantics implemented, while SunOS 4.1.1 and Solaris (SunOS 5.4) have the - * "old" one. Personally, I think the new semantics is much more logical. - * Note that "ln old new" where "new" is a symlink pointing to a non-existing - * file does succeed in both HP-UX and SunOs, but not in Solaris - * and in the old Linux semantics. - */ - -/* [16-Dec-97 Kevin Buhr] For security reasons, we change some symlink - * semantics. See the comments in "open_namei" and "do_link" below. - * - * [10-Sep-98 Alan Modra] Another symlink change. - */ - -/* [Feb-Apr 2000 AV] Complete rewrite. Rules for symlinks: - * inside the path - always follow. - * in the last component in creation/removal/renaming - never follow. - * if LOOKUP_FOLLOW passed - follow. - * if the pathname has trailing slashes - follow. - * otherwise - don't follow. - * (applied in that order). - * - * [Jun 2000 AV] Inconsistent behaviour of open() in case if flags==O_CREAT - * restored for 2.4. This is the last surviving part of old 4.2BSD bug. - * During the 2.4 we need to fix the userland stuff depending on it - - * hopefully we will be able to get rid of that wart in 2.5. So far only - * XEmacs seems to be relying on it... - */ -/* - * [Sep 2001 AV] Single-semaphore locking scheme (kudos to David Holland) - * implemented. Let's see if raised priority of ->s_vfs_rename_mutex gives - * any extra contention... - */ - /* In order to reduce some races, while at the same time doing additional * checking and hopefully speeding things up, we copy filenames to the * kernel data space before using them..
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com> --- fs/namei.c | 80 +----------------------------------------------------- 1 file changed, 1 insertion(+), 79 deletions(-)