* [PATCH] sunrpc : make RPC channel buffer dynamic for slow case @ 2020-10-26 15:05 Roberto Bergantinos Corpas 2020-11-06 21:51 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Roberto Bergantinos Corpas @ 2020-10-26 15:05 UTC (permalink / raw) To: bfields; +Cc: linux-nfs 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 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case 2020-10-26 15:05 [PATCH] sunrpc : make RPC channel buffer dynamic for slow case Roberto Bergantinos Corpas @ 2020-11-06 21:51 ` J. Bruce Fields 2020-11-21 10:54 ` Roberto Bergantinos Corpas 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2020-11-06 21:51 UTC (permalink / raw) To: Roberto Bergantinos Corpas; +Cc: linux-nfs 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case 2020-11-06 21:51 ` J. Bruce Fields @ 2020-11-21 10:54 ` Roberto Bergantinos Corpas 2020-11-23 15:36 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Roberto Bergantinos Corpas @ 2020-11-21 10:54 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Hi Bruce, Sorry for late response as well. Ok, here's a possible patch, let me know your thoughts diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index baef5ee43dbb..1347ecae9c84 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; @@ -905,44 +904,26 @@ static ssize_t cache_do_downcall(char *kaddr, const char __user *buf, return ret; } -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 */ - ssize_t ret = -EINVAL; - - if (count >= sizeof(write_buf)) - goto out; - mutex_lock(&queue_io_mutex); - ret = cache_do_downcall(write_buf, buf, count, cd); - mutex_unlock(&queue_io_mutex); -out: - return ret; -} - static ssize_t cache_downcall(struct address_space *mapping, const char __user *buf, size_t count, struct cache_detail *cd) { - struct page *page; - char *kaddr; + char *write_buf; ssize_t ret = -ENOMEM; - if (count >= PAGE_SIZE) - goto out_slow; + if (count >= 32768) { /* 32k is max userland buffer, lets check anyway */ + ret = -EINVAL; + goto out; + } - page = find_or_create_page(mapping, 0, GFP_KERNEL); - if (!page) - goto out_slow; + write_buf = kvmalloc(count + 1, GFP_KERNEL); + if (!write_buf) + goto out; - kaddr = kmap(page); - ret = cache_do_downcall(kaddr, buf, count, cd); - kunmap(page); - unlock_page(page); - put_page(page); + ret = cache_do_downcall(write_buf, buf, count, cd); + kvfree(write_buf); +out: return ret; -out_slow: - return cache_slow_downcall(buf, count, cd); } static ssize_t cache_write(struct file *filp, const char __user *buf, On Fri, Nov 6, 2020 at 10:51 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > 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 > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case 2020-11-21 10:54 ` Roberto Bergantinos Corpas @ 2020-11-23 15:36 ` J. Bruce Fields 2020-11-23 15:48 ` Chuck Lever 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2020-11-23 15:36 UTC (permalink / raw) To: Roberto Bergantinos Corpas; +Cc: linux-nfs On Sat, Nov 21, 2020 at 11:54:30AM +0100, Roberto Bergantinos Corpas wrote: > Hi Bruce, > > Sorry for late response as well. > > Ok, here's a possible patch, let me know your thoughts Looks good to me! Could you just submit with changelog and Signed-off-by? --b. > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > index baef5ee43dbb..1347ecae9c84 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; > @@ -905,44 +904,26 @@ static ssize_t cache_do_downcall(char *kaddr, > const char __user *buf, > return ret; > } > > -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 */ > - ssize_t ret = -EINVAL; > - > - if (count >= sizeof(write_buf)) > - goto out; > - mutex_lock(&queue_io_mutex); > - ret = cache_do_downcall(write_buf, buf, count, cd); > - mutex_unlock(&queue_io_mutex); > -out: > - return ret; > -} > - > static ssize_t cache_downcall(struct address_space *mapping, > const char __user *buf, > size_t count, struct cache_detail *cd) > { > - struct page *page; > - char *kaddr; > + char *write_buf; > ssize_t ret = -ENOMEM; > > - if (count >= PAGE_SIZE) > - goto out_slow; > + if (count >= 32768) { /* 32k is max userland buffer, lets > check anyway */ > + ret = -EINVAL; > + goto out; > + } > > - page = find_or_create_page(mapping, 0, GFP_KERNEL); > - if (!page) > - goto out_slow; > + write_buf = kvmalloc(count + 1, GFP_KERNEL); > + if (!write_buf) > + goto out; > > - kaddr = kmap(page); > - ret = cache_do_downcall(kaddr, buf, count, cd); > - kunmap(page); > - unlock_page(page); > - put_page(page); > + ret = cache_do_downcall(write_buf, buf, count, cd); > + kvfree(write_buf); > +out: > return ret; > -out_slow: > - return cache_slow_downcall(buf, count, cd); > } > > static ssize_t cache_write(struct file *filp, const char __user *buf, > > On Fri, Nov 6, 2020 at 10:51 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > > > 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case 2020-11-23 15:36 ` J. Bruce Fields @ 2020-11-23 15:48 ` Chuck Lever 2020-11-23 16:05 ` Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2020-11-23 15:48 UTC (permalink / raw) To: Bruce Fields; +Cc: Roberto Bergantinos Corpas, Linux NFS Mailing List > On Nov 23, 2020, at 10:36 AM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Sat, Nov 21, 2020 at 11:54:30AM +0100, Roberto Bergantinos Corpas wrote: >> Hi Bruce, >> >> Sorry for late response as well. >> >> Ok, here's a possible patch, let me know your thoughts > > Looks good to me! Could you just submit with changelog and > Signed-off-by? Bruce, are you taking this for v5.10-rc, or shall I include it with v5.11 ? -- Chuck Lever ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc : make RPC channel buffer dynamic for slow case 2020-11-23 15:48 ` Chuck Lever @ 2020-11-23 16:05 ` Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: Bruce Fields @ 2020-11-23 16:05 UTC (permalink / raw) To: Chuck Lever; +Cc: Roberto Bergantinos Corpas, Linux NFS Mailing List On Mon, Nov 23, 2020 at 10:48:02AM -0500, Chuck Lever wrote: > > > > On Nov 23, 2020, at 10:36 AM, J. Bruce Fields <bfields@fieldses.org> wrote: > > > > On Sat, Nov 21, 2020 at 11:54:30AM +0100, Roberto Bergantinos Corpas wrote: > >> Hi Bruce, > >> > >> Sorry for late response as well. > >> > >> Ok, here's a possible patch, let me know your thoughts > > > > Looks good to me! Could you just submit with changelog and > > Signed-off-by? > > Bruce, are you taking this for v5.10-rc, or shall I include it > with v5.11 ? I think the immediate problem was fixed by 27a1e8a0f79e and this is more clean-up, so it can wait for v5.11, if you don't mind taking it. --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-23 16:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-26 15:05 [PATCH] sunrpc : make RPC channel buffer dynamic for slow case Roberto Bergantinos Corpas 2020-11-06 21:51 ` J. Bruce Fields 2020-11-21 10:54 ` Roberto Bergantinos Corpas 2020-11-23 15:36 ` J. Bruce Fields 2020-11-23 15:48 ` Chuck Lever 2020-11-23 16:05 ` Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).