linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Evan Green <evan@rivosinc.com>
Cc: Conor Dooley <conor@kernel.org>,
	Palmer Dabbelt <palmer@rivosinc.com>,
	vineetg@rivosinc.com, heiko@sntech.de, slewis@rivosinc.com,
	Albert Ou <aou@eecs.berkeley.edu>,
	Andrew Bresticker <abrestic@rivosinc.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	Anup Patel <apatel@ventanamicro.com>,
	Arnd Bergmann <arnd@arndb.de>, Atish Patra <atishp@rivosinc.com>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Celeste Liu <coelacanthus@outlook.com>,
	Conor Dooley <conor.dooley@microchip.com>,
	Dao Lu <daolu@rivosinc.com>, Guo Ren <guoren@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Ruizhe Pan <c141028@gmail.com>,
	Sunil V L <sunilvl@ventanamicro.com>,
	Tobias Klauser <tklauser@distanz.ch>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 2/6] RISC-V: Add a syscall for HW probing
Date: Fri, 10 Feb 2023 07:50:04 +0100	[thread overview]
Message-ID: <Y+XpHNqnHyDEXbAj@kroah.com> (raw)
In-Reply-To: <CALs-HssV0=fcDKtUN_3S8T+_Qtq+6qCiNbfQU9SXicsh2KFt4w@mail.gmail.com>

On Thu, Feb 09, 2023 at 10:41:51AM -0800, Evan Green wrote:
> On Thu, Feb 9, 2023 at 9:13 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Feb 09, 2023 at 09:09:16AM -0800, Evan Green wrote:
> > > On Mon, Feb 6, 2023 at 10:32 PM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > Hey Evan, Greg,
> > > >
> > > >
> > > > On 7 February 2023 06:13:39 GMT, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >On Mon, Feb 06, 2023 at 12:14:51PM -0800, Evan Green wrote:
> > > > >> We don't have enough space for these all in ELF_HWCAP{,2} and there's no
> > > > >> system call that quite does this, so let's just provide an arch-specific
> > > > >> one to probe for hardware capabilities.  This currently just provides
> > > > >> m{arch,imp,vendor}id, but with the key-value pairs we can pass more in
> > > > >> the future.
> > > > >
> > > > >Ick, this is exactly what sysfs is designed to export in a sane way.
> > > > >Why not just use that instead?  The "key" would be the filename, and the
> > > > >value the value read from the filename.  If the key is not present, the
> > > > >file is not present and it's obvious what is happening, no fancy parsing
> > > > >and ABI issues at all.
> > > >
> > > > https://lore.kernel.org/linux-riscv/20221201160614.xpomlqq2fzpzfmcm@kamzik/
> > > >
> > > > This is the sysfs interface that I mentioned drew
> > > > suggested on the v1.
> > > > I think it fits ~perfectly with what Greg is suggesting too.
> > >
> > > Whoops, I'll admit I missed that comment when I reviewed the feedback
> > > from v1. I spent some time thinking about sysfs. The problem is this
> > > interface will be needed in places like very early program startup. If
> > > we're trying to use this in places like the ifunc selector to decide
> > > which memcpy to use, having to go open and read a fistful of files is
> > > going to be complex that early, and rough on performance.
> >
> > How is it going to be any different on "performance" than a syscall?  Or
> > complex?  It should be almost identical overall as this is all in-ram
> > and not any real I/o is happening.  You are limited only by the speed of
> > your cpu.
> 
> At best sysfs is 1 syscall per key, whereas this version of the
> interface lets you query all the keys you're interested in with a
> single syscall. With the
> proposed vdso version, we'd be down to ~0 syscalls for most queries.
> The complexity aspect is mostly a reference to having to do a bunch of
> open/read/parse/close operations at a time when mem* operations are
> still being set up. Since this is something that may get run on every
> program invocation, it seems worth it to be able to get fast and
> simple queries even if it's a slightly separated interface.

I'd be interested in the real benchmark numbers and seeing the userspace
and kernel code before arguing this too much.

Again, ignoring the lessons of the past is generally considered an
unwise decision, but hey, you do you, it's something that you are going
to have to maintain for 40+ years going forward, not me :)

good luck!

greg k-h

  reply	other threads:[~2023-02-10  6:50 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-06 20:14 [PATCH v2 0/6] RISC-V Hardware Probing User Interface Evan Green
2023-02-06 20:14 ` [PATCH v2 1/6] RISC-V: Move struct riscv_cpuinfo to new header Evan Green
2023-02-14 21:38   ` Conor Dooley
2023-02-14 21:57     ` Evan Green
2023-02-06 20:14 ` [PATCH v2 2/6] RISC-V: Add a syscall for HW probing Evan Green
2023-02-07  6:13   ` Greg KH
2023-02-07  6:32     ` Conor Dooley
2023-02-09 17:09       ` Evan Green
2023-02-09 17:13         ` Greg KH
2023-02-09 17:22           ` Jessica Clarke
2023-02-10  6:48             ` Greg KH
2023-02-09 18:41           ` Evan Green
2023-02-10  6:50             ` Greg KH [this message]
2023-02-07 23:16   ` kernel test robot
2023-02-14 23:51   ` Conor Dooley
2023-02-15  8:04     ` Andrew Jones
2023-02-15 20:49     ` Evan Green
2023-02-15 21:10       ` Conor Dooley
2023-02-15  9:56   ` Arnd Bergmann
2023-02-15 21:14     ` Evan Green
2023-02-15 22:43       ` Jessica Clarke
2023-02-16 13:28         ` Arnd Bergmann
2023-02-16 23:18           ` Evan Green
2023-02-06 20:14 ` [PATCH v2 3/6] RISC-V: hwprobe: Add support for RISCV_HWPROBE_BASE_BEHAVIOR_IMA Evan Green
2023-02-15 21:25   ` Conor Dooley
2023-02-15 22:09   ` Conor Dooley
2023-02-06 20:14 ` [PATCH v2 4/6] dt-bindings: Add RISC-V misaligned access performance Evan Green
2023-02-06 21:49   ` Rob Herring
2023-02-07 17:05   ` Rob Herring
2023-02-08 12:45     ` David Laight
2023-02-09 16:51       ` Palmer Dabbelt
2023-02-28 14:56         ` Rob Herring
2023-02-14 21:26   ` Conor Dooley
2023-02-15 20:50     ` Evan Green
2023-02-06 20:14 ` [PATCH v2 5/6] RISC-V: hwprobe: Support probing of " Evan Green
2023-02-07  7:02   ` kernel test robot
2023-02-15 21:57   ` Conor Dooley
2023-02-18  0:15     ` Evan Green
2023-02-06 20:14 ` [PATCH v2 6/6] selftests: Test the new RISC-V hwprobe interface Evan Green
2023-02-06 21:27   ` Mark Brown
2023-02-09 18:44     ` Evan Green
2023-02-06 21:11 ` [PATCH v2 0/6] RISC-V Hardware Probing User Interface Jessica Clarke
2023-02-06 22:47   ` Heinrich Schuchardt
2023-02-09 16:56     ` Palmer Dabbelt
2023-02-06 22:32 ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+XpHNqnHyDEXbAj@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=abrestic@rivosinc.com \
    --cc=ajones@ventanamicro.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=apatel@ventanamicro.com \
    --cc=arnd@arndb.de \
    --cc=atishp@rivosinc.com \
    --cc=bagasdotme@gmail.com \
    --cc=c141028@gmail.com \
    --cc=coelacanthus@outlook.com \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=corbet@lwn.net \
    --cc=daolu@rivosinc.com \
    --cc=evan@rivosinc.com \
    --cc=guoren@kernel.org \
    --cc=heiko@sntech.de \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=palmer@rivosinc.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rdunlap@infradead.org \
    --cc=slewis@rivosinc.com \
    --cc=sunilvl@ventanamicro.com \
    --cc=tklauser@distanz.ch \
    --cc=vineetg@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).