Message-ID: <CAL+tcoAdbQEuS5ZA3CJuEm4PDMKBF4mvv6cSV-xBtqK-pr_k2g@mail.gmail.com>
Date: Mon, 12 May 2025 20:04:14 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: "Dr. David Alan Gilbert" <linux@...blig.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, andriy.shevchenko@...ux.intel.com, 
	corbet@....net, linux-doc@...r.kernel.org, 
	LKML <linux-kernel@...r.kernel.org>, viro@...iv.linux.org.uk, 
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH] relay: Remove unused relay_late_setup_files

On Mon, May 12, 2025 at 7:50 PM Dr. David Alan Gilbert
<linux@...blig.org> wrote:
>
> * Jason Xing (kerneljasonxing@...il.com) wrote:
> > On Mon, May 12, 2025 at 9:30 AM Dr. David Alan Gilbert
> > <linux@...blig.org> wrote:
> > >
> > > * Jason Xing (kerneljasonxing@...il.com) wrote:
> > > > Hi All,
> > >
> > > Hi Jason,
> > >
> > > > I noticed this patch "relay: Remove unused relay_late_setup_files"
> > > > appears in the mm branch already[1], which I totally missed. Sorry for
> > > > joining the party late.
> > > >
> > > > I have a different opinion on this. For me, I'm very cautious about
> > > > what those so-called legacy interfaces are and how they can work in
> > > > different cases and what the use case might be... There are still a
> > > > small number of out-of-tree users like me heavily relying on relayfs
> > > > mechanism. So my humble opinion is that if you want to remove
> > > > so-called dead code, probably clearly state why it cannot be used
> > > > anymore in the future.
> > >
> > > We've got lots of deadcode, why it's dead varies a lot; for example
> > > people forgetting to clean it up after other patches etc - so this
> > > _could_ be used but hasn't been for well over 7 years.
> > >
> > > > Dr. David, I appreciate your patch, but please do not simply do the
> > > > random cleanup work __here__. If you take a deep look at the relayfs,
> > > > you may find there are other interfaces/functions no one uses in the
> > > > kernel tree.
> > >
> > > Actually, that was the only interface in relay that I found unused.
> >
> > Not really. More than this single one, say, __relay_write() and
> > subbuf_start_reserve()...
>
> Ah, my tools only spot unused symbols, they're header inlines; I've
> not found a way to spot those yet.
>
> > > > I'm now checking this kind of patch in relayfs one by one to avoid
> > > > such a thing happening. I'm trying to maintain it as much as possible
> > > > since we internally use it in the networking area to output useful
> > > > information in the hot paths, a little bit like blktrace. BTW, relayfs
> > > > is really a wonderful one that helps kernel modules communicate with
> > > > userspace very efficiently. I'm trying to revive it if I can.
> > >
> > > If you've got a use for that function, then I'm more than happy to suggest
> > > just dropping my patch.
> > >
> > > However, it is a fairly chunky function that is built into distro
> > > kernels - so I think it should have a little thought put to it.
> > >
> > > As I say, if you are using it, it's fine by me just to drop this patch.
> >
> > For now, I'm not using it but still considering what the use case
> > might be in the future. As I mentioned earlier, I'm trying to make
> > relayfs more robust with more realistic functions.
> >
> > IMHO, it's not really a dead code to me unless you can clarify why
> > it's obsolete instead of claiming "no one is using it".
>
> i'm very gentle about this; I'm not pushing back hard if someone
> says actually they want to keep something.
>
> I'd say my 'claim' is fairly good as even you say
>   'I'm not using it but still considering..'
>
> You don't need to push back quite as hard on me!

Oh, sorry, It's absolutely not my intention. I was trying to share my
opinion. No hard feelings;)

>
> > If you insist
> > on the point, then most of relayfs would be removed, which is
> > apparently not what I'm wishing for.
>
> You could forgive me for thinking this is unused;
>   a) There are no callers in the tree - I can't possibly imagine all
>    other trees to check; especially those on someones local disk
>    or thoughts still bouncing around in your brain!
>
>   b) There's no listed maintainer, so I can't assume anyone is actively
>   working on it
>
>   c) The only changes in years in the tree are cleanups, like strcpy
>   variants.
>
> We do have other APIs that people care about and aren't in use;
> now if those are nicely thought out APIs etc I think that's fine.
> (I've had others say they want to keep some because they like them or
> they're part of a well thought out API)

I see what you mean here. Relayfs is a bit special.

>
> > Probably it will be finally removed, but not at the moment. Evidence
> > is still not clear to me :S
> >
> > For sure, the last call would be made by Andrew and Jens. Please help
> > review this patch one more time. Thanks!
>
> Why don't you add a MAINTAINERS section with you added just as a
> reviewer?   That at least gets you told if someone dares to clean it up
> in the future!

Right, thanks for advising. I'm now asking for permission from Andrew and Jens.

Thanks,
Jason

>
> Dave
>
> > Thanks,
> > Jason
> >
> > >
> > > Dave
> > >
> > > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-everything&id=46aa76118ee365c25911806e34d28fc2aa5ef997
> > > >
> > > > Thanks,
> > > > Jason
> > > --
> > >  -----Open up your eyes, open up your mind, open up your code -------
> > > / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> > > \        dave @ treblig.org |                               | In Hex /
> > >  \ _________________________|_____ http://www.treblig.org   |_______/
> --
>  -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/

Powered by blists - more mailing lists