Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'David Hildenbrand' <david@redhat.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"Nick Desaulniers" <ndesaulniers@google.com>
Cc: Christoph Hellwig <hch@lst.de>,
	"kernel-team@android.com" <kernel-team@android.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Jens Axboe" <axboe@kernel.dk>, Arnd Bergmann <arnd@arndb.de>,
	David Howells <dhowells@redhat.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
	"linux-parisc@vger.kernel.org" <linux-parisc@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"sparclinux@vger.kernel.org" <sparclinux@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-aio@kvack.org" <linux-aio@kvack.org>,
	"io-uring@vger.kernel.org" <io-uring@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>
Subject: RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
Date: Thu, 22 Oct 2020 08:40:32 +0000
Message-ID: <5d2ecb24db1e415b8ff88261435386ec@AcuMS.aculab.com> (raw)
In-Reply-To: <80a2e5fa-718a-8433-1ab0-dd5b3e3b5416@redhat.com>

From: David Hildenbrand
> Sent: 22 October 2020 09:35
> 
> On 22.10.20 10:26, Greg KH wrote:
> > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> >>>> From: David Laight <David.Laight@ACULAB.COM>
> >>>>
> >>>> This lets the compiler inline it into import_iovec() generating
> >>>> much better code.
> >>>>
> >>>> Signed-off-by: David Laight <david.laight@aculab.com>
> >>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >>>> ---
> >>>>  fs/read_write.c | 179 ------------------------------------------------
> >>>>  lib/iov_iter.c  | 176 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  2 files changed, 176 insertions(+), 179 deletions(-)
> >>>
> >>> Strangely, this commit causes a regression in Linus's tree right now.
> >>>
> >>> I can't really figure out what the regression is, only that this commit
> >>> triggers a "large Android system binary" from working properly.  There's
> >>> no kernel log messages anywhere, and I don't have any way to strace the
> >>> thing in the testing framework, so any hints that people can provide
> >>> would be most appreciated.
> >>
> >> It's a pure move - modulo changed line breaks in the argument lists
> >> the functions involved are identical before and after that (just checked
> >> that directly, by checking out the trees before and after, extracting two
> >> functions in question from fs/read_write.c and lib/iov_iter.c (before and
> >> after, resp.) and checking the diff between those.
> >>
> >> How certain is your bisection?
> >
> > The bisection is very reproducable.
> >
> > But, this looks now to be a compiler bug.  I'm using the latest version
> > of clang and if I put "noinline" at the front of the function,
> > everything works.
> 
> Well, the compiler can do more invasive optimizations when inlining. If
> you have buggy code that relies on some unspecified behavior, inlining
> can change the behavior ... but going over that code, there isn't too
> much action going on. At least nothing screamed at me.

Apart from all the optimisations that get rid off the 'pass be reference'
parameters and strange conditional tests.
Plenty of scope for the compiler getting it wrong.
But nothing even vaguely illegal.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25  4:51 let import_iovec deal with compat_iovecs as well v4 Christoph Hellwig
2020-09-25  4:51 ` [PATCH 1/9] compat.h: fix a spelling error in <linux/compat.h> Christoph Hellwig
2020-09-25  4:51 ` [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c Christoph Hellwig
2020-10-21 16:13   ` Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c" Greg KH
2020-10-21 20:59     ` David Laight
2020-10-21 23:39     ` Al Viro
2020-10-22  8:26       ` Greg KH
2020-10-22  8:35         ` David Hildenbrand
2020-10-22  8:40           ` David Laight [this message]
2020-10-22  8:48             ` David Hildenbrand
2020-10-22  9:01               ` Greg KH
2020-10-22  9:06                 ` David Laight
2020-10-22  9:19                 ` David Hildenbrand
2020-10-22  9:25                   ` David Hildenbrand
2020-10-22  9:32                     ` David Laight
2020-10-22  9:36                       ` David Hildenbrand
2020-10-22 10:48                         ` Greg KH
2020-10-22 12:18                           ` Greg KH
2020-10-22 12:42                             ` David Hildenbrand
2020-10-22 12:57                               ` Greg KH
2020-10-22 13:50                                 ` Greg KH
     [not found]                                   ` <CAK8P3a1B7OVdyzW0-97JwzZiwp0D0fnSfyete16QTvPp_1m07A@mail.gmail.com>
2020-10-22 14:40                                     ` Greg KH
2020-10-22 16:15                                       ` David Laight
2020-10-23 12:46                                   ` David Laight
2020-10-23 13:09                                     ` David Hildenbrand
2020-10-23 14:33                                       ` David Hildenbrand
2020-10-23 14:39                                         ` David Laight
2020-10-23 14:47                                           ` 'Greg KH'
2020-10-23 16:33                                             ` David Hildenbrand
2020-11-02  9:06                                             ` David Laight
2020-11-02 13:52                                               ` 'Greg KH'
2020-11-02 18:23                                                 ` David Laight
2020-10-23 17:58                                       ` Al Viro
2020-10-23 18:27                                         ` Segher Boessenkool
2020-10-23 21:28                                           ` David Laight
2020-10-24 17:29                                             ` Segher Boessenkool
2020-10-24 21:12                                               ` David Laight
     [not found]                                     ` <CAK8P3a1n+b8hOMhNQSDzgic03dyXbmpccfTJ3C1bGKvzsgMXbg@mail.gmail.com>
2020-10-23 13:28                                       ` David Laight
2020-10-22 13:23                         ` Christoph Hellwig
2020-10-22 16:35                           ` David Laight
2020-10-22 16:40                             ` Matthew Wilcox
2020-10-22 16:50                               ` David Laight
2020-10-22 17:00                               ` Nick Desaulniers
2020-10-22 20:59                                 ` Eric Biggers
2020-10-22 21:28                                   ` Al Viro
2020-10-22 18:19                               ` Al Viro
2020-10-22 17:54                             ` Nick Desaulniers
     [not found]                               ` <CAK8P3a3LjG+ZvmQrkb9zpgov8xBkQQWrkHBPgjfYSqBKGrwT4w@mail.gmail.com>
2020-10-22 19:04                                 ` Nick Desaulniers
2020-10-22 19:24                                   ` Al Viro
2020-10-22 19:27                                     ` Al Viro
2020-10-22 20:06                                     ` Al Viro
2020-10-22 20:09                                       ` Al Viro
2020-10-22 20:11                                     ` Nick Desaulniers
2020-10-22 22:07                                     ` David Laight
2020-10-23 13:12                                     ` David Hildenbrand
2020-10-22 22:04                                   ` David Laight
2020-10-22  9:28                   ` David Laight
2020-10-22  9:02               ` David Laight
2020-09-25  4:51 ` [PATCH 3/9] iov_iter: refactor rw_copy_check_uvector and import_iovec Christoph Hellwig
2020-09-25  4:51 ` [PATCH 4/9] iov_iter: transparently handle compat iovecs in import_iovec Christoph Hellwig
2020-09-25  4:51 ` [PATCH 5/9] fs: remove various compat readv/writev helpers Christoph Hellwig
2020-09-25  4:51 ` [PATCH 6/9] fs: remove the compat readv/writev syscalls Christoph Hellwig
2020-09-25  4:51 ` [PATCH 7/9] fs: remove compat_sys_vmsplice Christoph Hellwig
2020-09-25  4:51 ` [PATCH 8/9] mm: remove compat_process_vm_{readv,writev} Christoph Hellwig
2020-09-25  4:51 ` [PATCH 9/9] security/keys: remove compat_keyctl_instantiate_key_iov Christoph Hellwig
2020-09-25 15:23 ` let import_iovec deal with compat_iovecs as well v4 Al Viro

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=5d2ecb24db1e415b8ff88261435386ec@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=david@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=io-uring@vger.kernel.org \
    --cc=kernel-team@android.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-aio@kvack.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git