linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Filipe Manana <fdmanana@suse.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	David Hildenbrand <david@redhat.com>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-s390 <linux-s390@vger.kernel.org>,
	 Linux-MM <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	 linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: Buffered I/O broken on s390x with page faults disabled (gfs2)
Date: Wed, 9 Mar 2022 21:57:32 +0100	[thread overview]
Message-ID: <CAHc6FU4ix3bv20XeBEPaTsLZhKMn25DkcXBQNaVE-hqV7s677Q@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wgBOFg3brJbo-gcaPM+fxjzHwC4efhcM8tCKK3YUhYUug@mail.gmail.com>

On Wed, Mar 9, 2022 at 8:08 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Mar 9, 2022 at 10:42 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> > From what I took from the previous discussion, probing at a sub-page
> > granularity won't be necessary for bytewise copying: when the address
> > we're trying to access is poisoned, fault_in_*() will fail; when we get
> > a short result, that will take us to the poisoned address in the next
> > iteration.
>
> Sadly, that isn't actually the case.
>
> It's not the case for GUP (that page aligns things), and it's not the
> case for fault_in_writeable() itself (that also page aligns things).

As long as the fault_in_*() functions probe the exact start address,
they will detect when that address is poisoned. They can advance
page-wise after that and it doesn't matter if they skip poisoned
addresses. When the memory range is then accessed, that may fail at a
poisoned address. The next call to fault_in_*() will be with that
poisoned address as the start address.

So it's the unaligned probing at the start in the fault_in_*()
functions that's essential, and fault_in_readable(),
fault_in_writeable(), and fault_in_safe_writeable() now all do that
probing.

> But more importantly, it's not actually the case for the *users*
> either. Not all of the users are byte-stream oriented, and I think it
> was btrfs that had a case of "copy a struct at the beginning of the
> stream". And if that copy failed, it wouldn't advance by as many bytes
> as it got - it would require that struct to be all fetched, and start
> from the beginning.
>
> So we do need to probe at least a minimum set of bytes. Probably a
> fairly small minimum, but still...

There are only a few users like that, and they can be changed to pass
back the actual address that fails so that that address can be probed
instead of probing every 128 bytes of the entire buffer on arm64.

Thanks,
Andreas



  reply	other threads:[~2022-03-09 20:57 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-07 22:52 Andreas Gruenbacher
2022-03-07 23:18 ` Linus Torvalds
2022-03-08  8:21   ` David Hildenbrand
2022-03-08  8:37     ` David Hildenbrand
2022-03-08 12:11       ` David Hildenbrand
2022-03-08 12:24         ` David Hildenbrand
2022-03-08 13:20           ` Gerald Schaefer
2022-03-08 13:32             ` David Hildenbrand
2022-03-08 14:14               ` Gerald Schaefer
2022-03-08 17:23           ` David Hildenbrand
2022-03-08 17:26     ` Linus Torvalds
2022-03-08 17:40       ` Linus Torvalds
2022-03-08 19:27         ` Linus Torvalds
2022-03-08 20:03           ` Linus Torvalds
2022-03-08 23:24             ` Andreas Gruenbacher
2022-03-09  0:22               ` Linus Torvalds
2022-03-09 18:42                 ` Andreas Gruenbacher
2022-03-09 19:08                   ` Linus Torvalds
2022-03-09 20:57                     ` Andreas Gruenbacher [this message]
2022-03-09 21:08                     ` Andreas Gruenbacher
2022-03-10 12:13                       ` Filipe Manana
2022-03-09 19:21                   ` Linus Torvalds
2022-03-09 19:35                     ` Andreas Gruenbacher
2022-03-09 20:18                       ` Linus Torvalds
2022-03-09 20:36                         ` Andreas Gruenbacher
2022-03-09 20:48                           ` Linus Torvalds
2022-03-09 20:54                             ` Linus Torvalds
2022-03-10 17:13           ` David Hildenbrand
2022-03-10 18:00             ` Andreas Gruenbacher
2022-03-10 18:35             ` Linus Torvalds
2022-03-10 18:38               ` David Hildenbrand
2022-03-10 18:47               ` Andreas Gruenbacher
2022-03-10 19:22                 ` Linus Torvalds
2022-03-10 19:56                   ` Linus Torvalds
2022-03-10 20:23                     ` Andreas Gruenbacher
2022-03-08 17:47       ` David Hildenbrand

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=CAHc6FU4ix3bv20XeBEPaTsLZhKMn25DkcXBQNaVE-hqV7s677Q@mail.gmail.com \
    --to=agruenba@redhat.com \
    --cc=catalin.marinas@arm.com \
    --cc=david@redhat.com \
    --cc=fdmanana@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=torvalds@linux-foundation.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
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