Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 6 Nov 2020 16:51:28 -0500
From:   "J. Bruce Fields" <bfields@fieldses.org>
To:     Roberto Bergantinos Corpas <rbergant@redhat.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case
Message-ID: <20201106215128.GD26028@fieldses.org>
References: <20201026150530.29019-1-rbergant@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201026150530.29019-1-rbergant@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Mon, Oct 26, 2020 at 04:05:30PM +0100, Roberto Bergantinos Corpas wrote:
> RPC channel buffer size for slow case (user buffer bigger than
> one page) can be converted into dymanic and also allows us to
> prescind from queue_io_mutex

Sorry for the slow response.

Let's just remove cache_slow_downcall and the find_or_create_page()
thing and just do a kvmalloc() from the start.  I don't understand why
we need to be more complicated.

--b.

> 
> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> ---
>  net/sunrpc/cache.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index baef5ee43dbb..325393f75e17 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -777,7 +777,6 @@ void cache_clean_deferred(void *owner)
>   */
>  
>  static DEFINE_SPINLOCK(queue_lock);
> -static DEFINE_MUTEX(queue_io_mutex);
>  
>  struct cache_queue {
>  	struct list_head	list;
> @@ -908,14 +907,18 @@ static ssize_t cache_do_downcall(char *kaddr, const char __user *buf,
>  static ssize_t cache_slow_downcall(const char __user *buf,
>  				   size_t count, struct cache_detail *cd)
>  {
> -	static char write_buf[8192]; /* protected by queue_io_mutex */
> +	char *write_buf;
>  	ssize_t ret = -EINVAL;
>  
> -	if (count >= sizeof(write_buf))
> +	if (count >= 32768) /* 32k is max userland buffer, lets check anyway */
>  		goto out;
> -	mutex_lock(&queue_io_mutex);
> +
> +	write_buf = kvmalloc(count + 1, GFP_KERNEL);
> +	if (!write_buf)
> +		return -ENOMEM;
> +
>  	ret = cache_do_downcall(write_buf, buf, count, cd);
> -	mutex_unlock(&queue_io_mutex);
> +	kvfree(write_buf);
>  out:
>  	return ret;
>  }
> -- 
> 2.21.0