Message-ID: <CAK8P3a0hPG1cTxksTBCJHkAV_=TLZLCi2pZYMk2Dc2-kLzD3rg@mail.gmail.com>
Date:   Fri, 27 Nov 2020 21:29:33 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Geoff Levand <geoff@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Michael Ellerman <mpe@...erman.id.au>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v2 02/24] exec: Simplify unshare_files

On Wed, Nov 25, 2020 at 2:16 AM Eric W. Biederman <ebiederm@...ssion.com> wrote:
> > On 11/24/20 12:14 PM, Arnd Bergmann wrote:
> >
> > There are still PS3-Linux users out there.  They use 'Homebrew' firmware
> > released through 'Hacker' forums that allow them to run Linux on
> > non-supported systems.  They are generally hobbies who don't post to
> > Linux kernel mailing lists.  I get direct inquiries regularly asking
> > about how to update to a recent kernel.  One of the things that attract
> > them to the PS3 is the Cell processor and either using or programming
> > the SPUs.
> >
> > It is difficult to judge how much use the SPU core dump support gets,
> > but if it is not a cause of major problems I feel we should consider
> > keeping it.
>
> I just took a quick look to get a sense how much tool support there is.
>
> In the gdb tree I found this 2019 commit abf516c6931a ("Remove Cell
> Broadband Engine debugging support").  Which basically removes the code
> in gdb that made sense of the spu coredumps.

Ah, I had not realized this was gone already. The code in gdb for
seamlessly debugging programs across CPU and SPU was clearly
more complex than the kernel portion for the coredump, so it makes
sense this was removed eventually.

> I would not say the coredump support is a source major problems, but it
> is a challenge to understand.  One of the pieces of code in there that
> is necessary to make the coredump support work reliable, a call to
> unshare_files, Oleg whole essentially maintains the ptrace and coredump
> support did not know why it was there, and it was not at all obvious
> when I looked at the code.
>
> So we are certainly in maintainers loosing hours of time figuring out
> what is going on and spending time fixing fuzzer bugs related to the
> code.

I also spent some amount of time on this code earlier this year Christoph
did some refactoring, and we could both have used that time better.

> At the minimum I will add a few more comments so people reading the code
> can realize why it is there.   Perhaps putting the relevant code behind
> a Kconfig so it is only built into the kernel when spufs is present.
>
> I think we are at a point we we can start planning on removing the
> coredump support.  The tools to read it are going away.  None of what is
> there is bad, but it is definitely a special case, and it definitely has
> a maintenance cost.

How about adding a comment in the coredump code so it can get
removed the next time someone comes across it during refactoring,
or when they find a bug that can't easily be worked around?

That way there is still a chance of using it where needed, but
hopefully it won't waste anyone's time when it gets in the way.

If there are no objections, I can also send a patch to remove
CONFIG_PPC_CELL_NATIVE, PPC_IBM_CELL_BLADE and
everything that depends on those symbols, leaving only the
bits needed by ps3 in the arch/powerpc/platforms/cell directory.

      Arnd

Powered by blists - more mailing lists