On June 9, 2025 9:21:20 AM EDT, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>On Mon, 2025-06-09 at 09:12 -0400, Jan Harkes wrote:
>> There are definitely still users of Coda at CMU, I don't track who else uses it, but it cannot be too many for sure.
>>
>> At this point it mostly keeps you honest about little locking details in the vfs. There are some tricky details in how the inode mappings are accessed and such. I think that is helpful for overlay and user filesystems like fuse, overlayfs, etc. but Coda is quite small so it is easy to reason about how it uses these features.
>>
>>
>
>The reason I ask is that it's one of the places that we often have to
>do odd fixups like this when making changes to core VFS APIs. It's also
>not seen any non-collateral changes since 2021. I'm just wondering
>whether it's worth it to keep coda in-tree for so few users.
I have no problem maintaining an external module. I already do that to both make development easier and to support building a custom module in case some distro didn't ship with a prebuilt Coda kernel module.
>IIRC, it's also the only fs in the kernel that changes its
>inode->i_mapping pointer after inode instantiation. If not for coda, we
>could probably replace the i_mapping pointer with some macro wizardry
>and shrink struct inode by 8 bytes.
And that would be why an out-of-tree solution will not work in the long run. A small change like that to shave 8 bytes off the inode structure is a fairly easy call to make when there are no in-tree filesystems that use it anymore.
Ultimately it is up to you guys to decide if the extra burden on VFS maintenance is worth it.
Jan
>> On June 9, 2025 9:00:31 AM EDT, Jan Kara <jack@xxxxxxx> wrote:
>> > On Mon 09-06-25 08:17:15, Jeff Layton wrote:
>> > > On Mon, 2025-06-09 at 09:09 +1000, NeilBrown wrote:
>> > > > The code in coda_readdir() is nearly identical to iterate_dir().
>> > > > Differences are:
>> > > > - iterate_dir() is killable
>> > > > - iterate_dir() adds permission checking and accessing notifications
>> > > >
>> > > > I believe these are not harmful for coda so it is best to use
>> > > > iterate_dir() directly. This will allow locking changes without
>> > > > touching the code in coda.
>> > > >
>> > > > Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
>> > > > ---
>> > > > fs/coda/dir.c | 12 ++----------
>> > > > 1 file changed, 2 insertions(+), 10 deletions(-)
>> > > >
>> > > > diff --git a/fs/coda/dir.c b/fs/coda/dir.c
>> > > > index ab69d8f0cec2..ca9990017265 100644
>> > > > --- a/fs/coda/dir.c
>> > > > +++ b/fs/coda/dir.c
>> > > > @@ -429,17 +429,9 @@ static int coda_readdir(struct file *coda_file, struct dir_context *ctx)
>> > > > cfi = coda_ftoc(coda_file);
>> > > > host_file = cfi->cfi_container;
>> > > >
>> > > > - if (host_file->f_op->iterate_shared) {
>> > > > - struct inode *host_inode = file_inode(host_file);
>> > > > - ret = -ENOENT;
>> > > > - if (!IS_DEADDIR(host_inode)) {
>> > > > - inode_lock_shared(host_inode);
>> > > > - ret = host_file->f_op->iterate_shared(host_file, ctx);
>> > > > - file_accessed(host_file);
>> > > > - inode_unlock_shared(host_inode);
>> > > > - }
>> > > > + ret = iterate_dir(host_file, ctx);
>> > > > + if (ret != -ENOTDIR)
>> > > > return ret;
>> > > > - }
>> > > > /* Venus: we must read Venus dirents from a file */
>> > > > return coda_venus_readdir(coda_file, ctx);
>> > > > }
>> > >
>> > >
>> > > Is it already time for my annual ask of "Who the heck is using coda
>> > > these days?" Anyway, this patch looks fine to me.
>> > >
>> > > Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> >
>> > Send a patch proposing deprecating it and we might learn that :) Searching
>> > the web seems to suggest it is indeed pretty close to dead.
>> >
>> > Honza
>
