Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 12 Jun 2022 23:32:38 -0700
From:   Joe Damato <jdamato@fastly.com>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [RFC,iov_iter v2 3/8] iov_iter: add copyin_iovec helper
Message-ID: <20220613063237.GA24325@fastly.com>
References: <1655024280-23827-1-git-send-email-jdamato@fastly.com>
 <1655024280-23827-4-git-send-email-jdamato@fastly.com>
 <Yqa8Q7O0Fmnex8Ev@zeniv-ca.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yqa8Q7O0Fmnex8Ev@zeniv-ca.linux.org.uk>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Mon, Jun 13, 2022 at 04:25:39AM +0000, Al Viro wrote:
> On Sun, Jun 12, 2022 at 01:57:52AM -0700, Joe Damato wrote:
> > copyin_iovec is a helper which wraps copyin and selects the right copy
> > method based on the iter_copy_type.
> > 
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > ---
> >  lib/iov_iter.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> > index d32d7e5..6720cb2 100644
> > --- a/lib/iov_iter.c
> > +++ b/lib/iov_iter.c
> > @@ -168,6 +168,15 @@ static int copyin(void *to, const void __user *from, size_t n)
> >  	return n;
> >  }
> >  
> > +static int copyin_iovec(void *to, const void __user *from, size_t n,
> > +			struct iov_iter *i)
> > +{
> > +	if (unlikely(iov_iter_copy_is_nt(i)))
> > +		return __copy_from_user_nocache(to, from, n);
> > +	else
> > +		return copyin(to, from, n);
> > +}
> 
> Just a sanity check - your testing is *not* with KASAN/KCSAN, right?

Yes, that is correct.

> And BTW, why is that only on the userland side?  If you are doing
> that at all, it would make sense to cover the memcpy() side as
> well...

I assume here you mean the memcpy() in the splice() path? I do have a
separate change I've been testing which does this, but I thought that can
be sent separately.

This RFC basically takes an existing kernel feature (tx-nocache-copy) and
makes it applicable to more protocols and more fine grained so that it does
not need to be enabled interface-wide. The memcpy() change you mention is,
in my mind, a separate change which adds a new feature and can be sent if
this change is accepted upstream.

Let me know if that makes sense and if there are any issues you think I
should address before I send a v1 for consideration.

Thanks for taking a look!