Re: [RFC][PATCHSET] iov_iter work

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	David Sterba <dsterba@suse.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Anton Altaparmakov <anton@tuxera.com>,
	David Howells <dhowells@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Pavel Begunkov <asml.silence@gmail.com>
Subject: Re: [RFC][PATCHSET] iov_iter work
Date: Mon, 7 Jun 2021 15:01:01 -0700	[thread overview]
Message-ID: <CAHk-=wgr3o6cKTNpU9wg7fj_+OUh5kFwrD29Lg0n2=-1nhvoZA@mail.gmail.com> (raw)
In-Reply-To: <YL6KdoHzYiBOsu5t@zeniv-ca.linux.org.uk>

On Mon, Jun 7, 2021 at 2:07 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Anyway, what I'm going to do is
>
>         if (WARN_ON(uaccess_kernel())) {
>                 // shouldn't be any such callers left...
>                 iov_iter_kvec(i, direction, (const struct kvec *)iov,
>                               nr_segs, count);
>                 return;
>         }

Yeah, this looks better to me simply because it makes it obvious how
that kvec case is a legacy special case.

But make that a WARN_ON_ONCE() - if it ever triggers, we don't want it
to spam the logs.

I guess 32-bit arm is still CONFIG_SET_FS, so we should get some
fairly timely testing of this on the various arm farms.

I wonder if it's even worth trying to make any such cases work any
more. In addition to the warning, maybe we might as well just not fill
in the kvec at all, and we should just do

        iov_iter_kvec(i, direction, NULL, 0, 0);

for that case too.

Having looked around quickly, I think

 (a) none of the actual set_fs(KERNEL_DS) users seem to do anything
remotely related to iovecs

 (b) on all the common non-SET_FS architectures, kernel threads using
iov_iter_init() wouldn't work anyway, because on those architectures
it would always fill the thing in with an iov, not a kvec.

So I'm starting to agree with Christoph that this case can't actually
happen. It would have to be some architecture-specific kernel thread
that does this, and I don't think we _have_ any architecture-specific
ones.

My first thought was that we probably still had some odd core-dumping
code or other that might still use that set_fs() model, but that seems
to have all gotten cleaned up. Wonder of wonders.

So I'd _almost_ just remove this all right now, and if not removing it
I think making it an empty kvec might be preferable to that cast from
an iovec to a kvec.

But no hugely strong opinions. I'm ok with your version too, modulo
that WARN_ON_ONCE comment.

              Linus