* [PATCH] sunrpc: raise kernel RPC channel buffer size @ 2020-10-19 9:33 Roberto Bergantinos Corpas 2020-10-19 13:20 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Roberto Bergantinos Corpas @ 2020-10-19 9:33 UTC (permalink / raw) To: bfields; +Cc: linux-nfs Its possible that using AUTH_SYS and mountd manage-gids option a user may hit the 8k RPC channel buffer limit. This have been observed on field, causing unanswered RPCs on clients after mountd fails to write on channel : rpc.mountd[11231]: auth_unix_gid: error writing reply Userland nfs-utils uses a buffer size of 32k (RPC_CHAN_BUF_SIZE), so lets match those two. Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com> --- net/sunrpc/cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index baef5ee43dbb..08df4c599ab3 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -908,7 +908,7 @@ 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 */ + static char write_buf[32768]; /* protected by queue_io_mutex */ ssize_t ret = -EINVAL; if (count >= sizeof(write_buf)) -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc: raise kernel RPC channel buffer size 2020-10-19 9:33 [PATCH] sunrpc: raise kernel RPC channel buffer size Roberto Bergantinos Corpas @ 2020-10-19 13:20 ` J. Bruce Fields 2020-10-23 9:44 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2020-10-19 13:20 UTC (permalink / raw) To: Roberto Bergantinos Corpas; +Cc: linux-nfs On Mon, Oct 19, 2020 at 11:33:56AM +0200, Roberto Bergantinos Corpas wrote: > Its possible that using AUTH_SYS and mountd manage-gids option a > user may hit the 8k RPC channel buffer limit. This have been observed > on field, causing unanswered RPCs on clients after mountd fails to > write on channel : > > rpc.mountd[11231]: auth_unix_gid: error writing reply > > Userland nfs-utils uses a buffer size of 32k (RPC_CHAN_BUF_SIZE), so > lets match those two. Thanks, applying. That should allow about 4000 group memberships. If that doesn't do it then maybe it's time to rethink.... --b. > > Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > --- > net/sunrpc/cache.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > index baef5ee43dbb..08df4c599ab3 100644 > --- a/net/sunrpc/cache.c > +++ b/net/sunrpc/cache.c > @@ -908,7 +908,7 @@ 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 */ > + static char write_buf[32768]; /* protected by queue_io_mutex */ > ssize_t ret = -EINVAL; > > if (count >= sizeof(write_buf)) > -- > 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc: raise kernel RPC channel buffer size 2020-10-19 13:20 ` J. Bruce Fields @ 2020-10-23 9:44 ` Geert Uytterhoeven 2020-10-24 0:04 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2020-10-23 9:44 UTC (permalink / raw) To: J. Bruce Fields, Roberto Bergantinos Corpas; +Cc: linux-nfs, linux-kernel Hi Bruce, Roberto, On Mon, 19 Oct 2020, J. Bruce Fields wrote: > On Mon, Oct 19, 2020 at 11:33:56AM +0200, Roberto Bergantinos Corpas wrote: >> Its possible that using AUTH_SYS and mountd manage-gids option a >> user may hit the 8k RPC channel buffer limit. This have been observed >> on field, causing unanswered RPCs on clients after mountd fails to >> write on channel : >> >> rpc.mountd[11231]: auth_unix_gid: error writing reply >> >> Userland nfs-utils uses a buffer size of 32k (RPC_CHAN_BUF_SIZE), so >> lets match those two. > > Thanks, applying. > > That should allow about 4000 group memberships. If that doesn't do it > then maybe it's time to rethink.... > > --b. > >> >> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com> >> --- >> net/sunrpc/cache.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c >> index baef5ee43dbb..08df4c599ab3 100644 >> --- a/net/sunrpc/cache.c >> +++ b/net/sunrpc/cache.c >> @@ -908,7 +908,7 @@ 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 */ >> + static char write_buf[32768]; /* protected by queue_io_mutex */ >> ssize_t ret = -EINVAL; >> >> if (count >= sizeof(write_buf)) This is now commit 27a1e8a0f79e643d ("sunrpc: raise kernel RPC channel buffer size") upstream, and increases kernel size by 24 KiB, even if RPC is not used. Can this buffer allocated dynamically instead? This code path seems to be a slow path anyway. If it's critical, perhaps this buffer can be allocated on first use? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc: raise kernel RPC channel buffer size 2020-10-23 9:44 ` Geert Uytterhoeven @ 2020-10-24 0:04 ` J. Bruce Fields 2020-10-24 1:29 ` Roberto Bergantinos Corpas 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2020-10-24 0:04 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Roberto Bergantinos Corpas, linux-nfs, linux-kernel On Fri, Oct 23, 2020 at 11:44:38AM +0200, Geert Uytterhoeven wrote: > Hi Bruce, Roberto, > > On Mon, 19 Oct 2020, J. Bruce Fields wrote: > >On Mon, Oct 19, 2020 at 11:33:56AM +0200, Roberto Bergantinos Corpas wrote: > >>Its possible that using AUTH_SYS and mountd manage-gids option a > >>user may hit the 8k RPC channel buffer limit. This have been observed > >>on field, causing unanswered RPCs on clients after mountd fails to > >>write on channel : > >> > >>rpc.mountd[11231]: auth_unix_gid: error writing reply > >> > >>Userland nfs-utils uses a buffer size of 32k (RPC_CHAN_BUF_SIZE), so > >>lets match those two. > > > >Thanks, applying. > > > >That should allow about 4000 group memberships. If that doesn't do it > >then maybe it's time to rethink.... > > > >--b. > > > >> > >>Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > >>--- > >> net/sunrpc/cache.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > >>index baef5ee43dbb..08df4c599ab3 100644 > >>--- a/net/sunrpc/cache.c > >>+++ b/net/sunrpc/cache.c > >>@@ -908,7 +908,7 @@ 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 */ > >>+ static char write_buf[32768]; /* protected by queue_io_mutex */ > >> ssize_t ret = -EINVAL; > >> > >> if (count >= sizeof(write_buf)) > > This is now commit 27a1e8a0f79e643d ("sunrpc: raise kernel RPC channel > buffer size") upstream, and increases kernel size by 24 KiB, even if > RPC is not used. > > Can this buffer allocated dynamically instead? This code path seems to > be a slow path anyway. If it's critical, perhaps this buffer can be > allocated on first use? Sure. Actually it is using an allocated buffer typically, see cache_downcall(). Looking back at the history.... That was added by Trond in 2009 (da77005f0d64 "SUNRPC: Remove the global temporary write buffer in net/sunrpc/cache.c"). Before that there's a pre-git change from 2004 which replaced a simple kmalloc(PAGE_SIZE). So I guess the point was to be careful about higher-order allocations, but probably it was overkill. How about making it a kvmalloc? --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc: raise kernel RPC channel buffer size 2020-10-24 0:04 ` J. Bruce Fields @ 2020-10-24 1:29 ` Roberto Bergantinos Corpas 2020-10-24 2:09 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Roberto Bergantinos Corpas @ 2020-10-24 1:29 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Geert Uytterhoeven, linux-nfs, Linux Kernel Mailing List Good point Geert ! > How about making it a kvmalloc? I can post a new patch using kvmalloc, Bruce looks we can also prescind of queue_io_mutex, what do you think ? > > --b. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sunrpc: raise kernel RPC channel buffer size 2020-10-24 1:29 ` Roberto Bergantinos Corpas @ 2020-10-24 2:09 ` J. Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: J. Bruce Fields @ 2020-10-24 2:09 UTC (permalink / raw) To: Roberto Bergantinos Corpas Cc: Geert Uytterhoeven, linux-nfs, Linux Kernel Mailing List On Sat, Oct 24, 2020 at 03:29:25AM +0200, Roberto Bergantinos Corpas wrote: > Good point Geert ! > > > How about making it a kvmalloc? > > I can post a new patch using kvmalloc, Bruce looks we can also > prescind of queue_io_mutex, what do you think ? And revert da77005f0d64, I think. Maybe there's something I'm missing, but I don't think we need all that complexity. --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-24 2:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-19 9:33 [PATCH] sunrpc: raise kernel RPC channel buffer size Roberto Bergantinos Corpas 2020-10-19 13:20 ` J. Bruce Fields 2020-10-23 9:44 ` Geert Uytterhoeven 2020-10-24 0:04 ` J. Bruce Fields 2020-10-24 1:29 ` Roberto Bergantinos Corpas 2020-10-24 2:09 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox