Re: [PATCH net-next v2 10/10] crypto: af_alg/hash: Support MSG_SPLICE_PAGES

[David Howells]

Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:

> > -	if (limit > sk->sk_sndbuf)
> > -		limit = sk->sk_sndbuf;
> > +	/* Don't limit to ALG_MAX_PAGES if the pages are all already pinned. */
> > +	if (!user_backed_iter(&msg->msg_iter))
> > +		max_pages = INT_MAX;

If the iov_iter is a kernel-backed type (BVEC, KVEC, XARRAY) then (a) all the
pages it refers to must already be pinned in memory and (b) the caller must
have limited it in some way (splice is limited by the pipe capacity, for
instance).  In which case, it seems pointless taking more than one pass of the
while loop if we can avoid it - at least from the point of view of memory
handling; granted there might be other criteria such as hogging crypto offload
hardware.

> > +	else
> > +		max_pages = min_t(size_t, max_pages,
> > +				  DIV_ROUND_UP(sk->sk_sndbuf, PAGE_SIZE));
> 
> What's the purpose of relaxing this limit?

If the iov_iter is a user-backed type (IOVEC or UBUF) then it's not relaxed.
max_pages is ALG_MAX_PAGES here (actually, I should just move that here so
that it's clearer).

I am, however, applying the sk_sndbuf limit here also - there's no point
extracting more pages than we need to if ALG_MAX_PAGES of whole pages would
overrun the byte limit.

> Even if there is a reason for this shouldn't this be in a patch by itself?

I suppose I could do it as a follow-on patch; use ALG_MAX_PAGES and sk_sndbuf
before that as for user-backed iterators.

Actually, is it worth paying attention to sk_sndbuf for kernel-backed
iterators?

David