public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Heiko Carstens <hca@linux.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Vasily Gorbik <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>
Subject: Re: [GIT PULL] s390 updates for 5.15-rc6
Date: Sun, 17 Oct 2021 10:42:58 +0200	[thread overview]
Message-ID: <YWviEupI518iFjOb@osiris> (raw)
In-Reply-To: <CAHk-=whoe211F8ND-9hZvfnib0UA4gga8DZJ+YaBZNbE4fubdg@mail.gmail.com>

On Sat, Oct 16, 2021 at 09:22:12AM -0700, Linus Torvalds wrote:
> On Sat, Oct 16, 2021 at 2:26 AM Vasily Gorbik <gor@linux.ibm.com> wrote:
> >
> > - Fix broken strrchr implementation.
> 
> I pulled this, but had to look at this commit just because it struck me as odd.
> 
> Is the generic strrchr() implementation so bad for s390 that it shows
> up? It looks like the s390 implementation wants to avoid checking
> separately against the final NUL character and basically uses the
> optimized strlen function to do so, but when strrchr has to walk the
> string _anyway_ this all looks a bit odd.

The difference is that it walks the string from the end to the start,
taking into account that searching for the end of the string can be
considered fast.

> Not a big deal. I just get the feeling that s390 (and probably other
> architectures) might be a bit too eager to make their own helper
> string functions for reasons that may be historical ("we didn't have
> generic string functions at all long ago") or misguided ("we'll do an
> architecture-optimized version even for things that don't matter").

This function exists for historical reasons, and in this case it is
probably more the "optimized version for things that don't matter".

The only caller which sort of gets called more frequently seems to be
kbasename().

Some numbers, running 1.000.000 strrchr on an 80 byte string:

 s390  generic   search for
 57us     55us   first character in string
 15us     55us   last character in string
 44us     55us   character in middle of string

So yes, we could probably just drop our own implementation, given that
it hardly matters in real life.

  reply	other threads:[~2021-10-17  8:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-16  9:26 [GIT PULL] s390 updates for 5.15-rc6 Vasily Gorbik
2021-10-16 16:22 ` Linus Torvalds
2021-10-17  8:42   ` Heiko Carstens [this message]
2021-10-16 16:23 ` pr-tracker-bot

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=YWviEupI518iFjOb@osiris \
    --to=hca@linux.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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