* [PATCH 1/2] nfsd: cleanup setting of default max_block_size @ 2012-01-30 22:42 J. Bruce Fields 2012-01-30 22:42 ` [PATCH 2/2] nfsd: fix default iosize calculation on 32bit J. Bruce Fields 2012-01-31 1:11 ` [PATCH 1/2] nfsd: cleanup setting of default max_block_size Mi Jinlong 0 siblings, 2 replies; 9+ messages in thread From: J. Bruce Fields @ 2012-01-30 22:42 UTC (permalink / raw) To: linux-nfs; +Cc: J. Bruce Fields From: "J. Bruce Fields" <bfields@redhat.com> Move calculation of the default into a helper function. Get rid of an unused variable "err" while we're there. Signed-off-by: J. Bruce Fields <bfields@redhat.com> --- fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- 1 files changed, 24 insertions(+), 20 deletions(-) diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index eda7d7e..2ad5ffe 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -307,33 +307,37 @@ static void set_max_drc(void) dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); } -int nfsd_create_serv(void) +static int nfsd_get_default_max_blksize(void) { - int err = 0; + struct sysinfo i; + unsigned long target; + unsigned long bytes; + + si_meminfo(&i); + target = i.totalram << PAGE_SHIFT; + /* + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig + * machines, but only uses 32K on 128M machines. Bottom out at + * 8K on 32M and smaller. Of course, this is only a default. + */ + target <<= 12; + + bytes = NFSSVC_MAXBLKSIZE; + while (bytes > target && bytes >= 8*1024*2) + bytes /= 2; + return bytes; +} +int nfsd_create_serv(void) +{ WARN_ON(!mutex_is_locked(&nfsd_mutex)); if (nfsd_serv) { svc_get(nfsd_serv); return 0; } - if (nfsd_max_blksize == 0) { - /* choose a suitable default */ - struct sysinfo i; - si_meminfo(&i); - /* Aim for 1/4096 of memory per thread - * This gives 1MB on 4Gig machines - * But only uses 32K on 128M machines. - * Bottom out at 8K on 32M and smaller. - * Of course, this is only a default. - */ - nfsd_max_blksize = NFSSVC_MAXBLKSIZE; - i.totalram <<= PAGE_SHIFT - 12; - while (nfsd_max_blksize > i.totalram && - nfsd_max_blksize >= 8*1024*2) - nfsd_max_blksize /= 2; - } + if (nfsd_max_blksize == 0) + nfsd_max_blksize = nfsd_get_default_max_blksize(); nfsd_reset_versions(); - nfsd_serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, nfsd_last_thread, nfsd, THIS_MODULE); if (nfsd_serv == NULL) @@ -341,7 +345,7 @@ int nfsd_create_serv(void) set_max_drc(); do_gettimeofday(&nfssvc_boot); /* record boot time */ - return err; + return 0; } int nfsd_nrpools(void) -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] nfsd: fix default iosize calculation on 32bit 2012-01-30 22:42 [PATCH 1/2] nfsd: cleanup setting of default max_block_size J. Bruce Fields @ 2012-01-30 22:42 ` J. Bruce Fields 2012-01-30 22:47 ` J. Bruce Fields 2012-01-31 1:11 ` [PATCH 1/2] nfsd: cleanup setting of default max_block_size Mi Jinlong 1 sibling, 1 reply; 9+ messages in thread From: J. Bruce Fields @ 2012-01-30 22:42 UTC (permalink / raw) To: linux-nfs; +Cc: J. Bruce Fields From: "J. Bruce Fields" <bfields@redhat.com> The rpc buffers will be allocated out of low memory, so we should really only be taking that into account. Signed-off-by: J. Bruce Fields <bfields@redhat.com> --- fs/nfsd/nfssvc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 2ad5ffe..53c89f7 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -314,7 +314,7 @@ static int nfsd_get_default_max_blksize(void) unsigned long bytes; si_meminfo(&i); - target = i.totalram << PAGE_SHIFT; + target = (i.totalram - i.totalhigh) << PAGE_SHIFT; /* * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig * machines, but only uses 32K on 128M machines. Bottom out at -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] nfsd: fix default iosize calculation on 32bit 2012-01-30 22:42 ` [PATCH 2/2] nfsd: fix default iosize calculation on 32bit J. Bruce Fields @ 2012-01-30 22:47 ` J. Bruce Fields 0 siblings, 0 replies; 9+ messages in thread From: J. Bruce Fields @ 2012-01-30 22:47 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs On Mon, Jan 30, 2012 at 05:42:45PM -0500, J. Bruce Fields wrote: > From: "J. Bruce Fields" <bfields@redhat.com> > > The rpc buffers will be allocated out of low memory, so we should really > only be taking that into account. I could have sworm this problem had already been fixed, or at least discussed, but can't find any reference now. --b. > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfsd/nfssvc.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > index 2ad5ffe..53c89f7 100644 > --- a/fs/nfsd/nfssvc.c > +++ b/fs/nfsd/nfssvc.c > @@ -314,7 +314,7 @@ static int nfsd_get_default_max_blksize(void) > unsigned long bytes; > > si_meminfo(&i); > - target = i.totalram << PAGE_SHIFT; > + target = (i.totalram - i.totalhigh) << PAGE_SHIFT; > /* > * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig > * machines, but only uses 32K on 128M machines. Bottom out at > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-01-30 22:42 [PATCH 1/2] nfsd: cleanup setting of default max_block_size J. Bruce Fields 2012-01-30 22:42 ` [PATCH 2/2] nfsd: fix default iosize calculation on 32bit J. Bruce Fields @ 2012-01-31 1:11 ` Mi Jinlong 2012-02-01 21:46 ` J. Bruce Fields 1 sibling, 1 reply; 9+ messages in thread From: Mi Jinlong @ 2012-01-31 1:11 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs J. Bruce Fields 写道: > From: "J. Bruce Fields" <bfields@redhat.com> > > Move calculation of the default into a helper function. > > Get rid of an unused variable "err" while we're there. > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- > 1 files changed, 24 insertions(+), 20 deletions(-) > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > index eda7d7e..2ad5ffe 100644 > --- a/fs/nfsd/nfssvc.c > +++ b/fs/nfsd/nfssvc.c > @@ -307,33 +307,37 @@ static void set_max_drc(void) > dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); > } > > -int nfsd_create_serv(void) > +static int nfsd_get_default_max_blksize(void) > { > - int err = 0; > + struct sysinfo i; > + unsigned long target; > + unsigned long bytes; > + > + si_meminfo(&i); > + target = i.totalram << PAGE_SHIFT; > + /* > + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig > + * machines, but only uses 32K on 128M machines. Bottom out at > + * 8K on 32M and smaller. Of course, this is only a default. > + */ > + target <<= 12; Should use target = i.totalram << (PAGE_SHIFT - 12); target = i.totalram << PAGE_SHIFT; and target <<= 12; means target = i.totalram << (PAGE_SHIFT + 12); thanks, Mi Jinlong > + > + bytes = NFSSVC_MAXBLKSIZE; > + while (bytes > target && bytes >= 8*1024*2) > + bytes /= 2; > + return bytes; > +} > > +int nfsd_create_serv(void) > +{ > WARN_ON(!mutex_is_locked(&nfsd_mutex)); > if (nfsd_serv) { > svc_get(nfsd_serv); > return 0; > } > - if (nfsd_max_blksize == 0) { > - /* choose a suitable default */ > - struct sysinfo i; > - si_meminfo(&i); > - /* Aim for 1/4096 of memory per thread > - * This gives 1MB on 4Gig machines > - * But only uses 32K on 128M machines. > - * Bottom out at 8K on 32M and smaller. > - * Of course, this is only a default. > - */ > - nfsd_max_blksize = NFSSVC_MAXBLKSIZE; > - i.totalram <<= PAGE_SHIFT - 12; > - while (nfsd_max_blksize > i.totalram && > - nfsd_max_blksize >= 8*1024*2) > - nfsd_max_blksize /= 2; > - } > + if (nfsd_max_blksize == 0) > + nfsd_max_blksize = nfsd_get_default_max_blksize(); > nfsd_reset_versions(); > - > nfsd_serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, > nfsd_last_thread, nfsd, THIS_MODULE); > if (nfsd_serv == NULL) > @@ -341,7 +345,7 @@ int nfsd_create_serv(void) > > set_max_drc(); > do_gettimeofday(&nfssvc_boot); /* record boot time */ > - return err; > + return 0; > } > > int nfsd_nrpools(void) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-01-31 1:11 ` [PATCH 1/2] nfsd: cleanup setting of default max_block_size Mi Jinlong @ 2012-02-01 21:46 ` J. Bruce Fields 2012-02-03 20:49 ` J. Bruce Fields 0 siblings, 1 reply; 9+ messages in thread From: J. Bruce Fields @ 2012-02-01 21:46 UTC (permalink / raw) To: Mi Jinlong; +Cc: J. Bruce Fields, linux-nfs On Tue, Jan 31, 2012 at 09:11:48AM +0800, Mi Jinlong wrote: > > > J. Bruce Fields 写道: > > From: "J. Bruce Fields" <bfields@redhat.com> > > > > Move calculation of the default into a helper function. > > > > Get rid of an unused variable "err" while we're there. > > > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > --- > > fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- > > 1 files changed, 24 insertions(+), 20 deletions(-) > > > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > > index eda7d7e..2ad5ffe 100644 > > --- a/fs/nfsd/nfssvc.c > > +++ b/fs/nfsd/nfssvc.c > > @@ -307,33 +307,37 @@ static void set_max_drc(void) > > dprintk("%s nfsd_drc_max_mem %u ¥n", __func__, nfsd_drc_max_mem); > > } > > > > -int nfsd_create_serv(void) > > +static int nfsd_get_default_max_blksize(void) > > { > > - int err = 0; > > + struct sysinfo i; > > + unsigned long target; > > + unsigned long bytes; > > + > > + si_meminfo(&i); > > + target = i.totalram << PAGE_SHIFT; > > + /* > > + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig > > + * machines, but only uses 32K on 128M machines. Bottom out at > > + * 8K on 32M and smaller. Of course, this is only a default. > > + */ > > + target <<= 12; > > Should use target = i.totalram << (PAGE_SHIFT - 12); > > target = i.totalram << PAGE_SHIFT; and > target <<= 12; > means target = i.totalram << (PAGE_SHIFT + 12); Yes, thanks for catching that. Also, splitting up the calculation as I did above risks overflow at the first step. I'll fix that.... --b. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-02-01 21:46 ` J. Bruce Fields @ 2012-02-03 20:49 ` J. Bruce Fields 2012-02-03 16:24 ` Mi Jinlong 0 siblings, 1 reply; 9+ messages in thread From: J. Bruce Fields @ 2012-02-03 20:49 UTC (permalink / raw) To: Mi Jinlong; +Cc: J. Bruce Fields, linux-nfs On Wed, Feb 01, 2012 at 04:46:56PM -0500, bfields wrote: > On Tue, Jan 31, 2012 at 09:11:48AM +0800, Mi Jinlong wrote: > > Should use target = i.totalram << (PAGE_SHIFT - 12); > > > > target = i.totalram << PAGE_SHIFT; and > > target <<= 12; > > means target = i.totalram << (PAGE_SHIFT + 12); > > Yes, thanks for catching that. > > Also, splitting up the calculation as I did above risks overflow at the > first step. > > I'll fix that.... Here are the fixed patches.--b. >From 87b0fc7deb5feccf93b022f6a976e8441152dbb2 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" <bfields@redhat.com> Date: Mon, 30 Jan 2012 16:18:35 -0500 Subject: [PATCH 1/2] nfsd: cleanup setting of default max_block_size Move calculation of the default into a helper function. Get rid of an unused variable "err" while we're there. Thanks to Mi Jinlong for catching an arithmetic error in a previous version. Cc: Mi Jinlong <mijinlong@cn.fujitsu.com> Signed-off-by: J. Bruce Fields <bfields@redhat.com> --- fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- 1 files changed, 24 insertions(+), 20 deletions(-) diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index eda7d7e..e9eb408 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -307,33 +307,37 @@ static void set_max_drc(void) dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); } -int nfsd_create_serv(void) +static int nfsd_get_default_max_blksize(void) { - int err = 0; + struct sysinfo i; + unsigned long long target; + unsigned long ret; + + si_meminfo(&i); + target = i.totalram << PAGE_SHIFT; + /* + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig + * machines, but only uses 32K on 128M machines. Bottom out at + * 8K on 32M and smaller. Of course, this is only a default. + */ + target >>= 12; + + ret = NFSSVC_MAXBLKSIZE; + while (ret > target && ret >= 8*1024*2) + ret /= 2; + return ret; +} +int nfsd_create_serv(void) +{ WARN_ON(!mutex_is_locked(&nfsd_mutex)); if (nfsd_serv) { svc_get(nfsd_serv); return 0; } - if (nfsd_max_blksize == 0) { - /* choose a suitable default */ - struct sysinfo i; - si_meminfo(&i); - /* Aim for 1/4096 of memory per thread - * This gives 1MB on 4Gig machines - * But only uses 32K on 128M machines. - * Bottom out at 8K on 32M and smaller. - * Of course, this is only a default. - */ - nfsd_max_blksize = NFSSVC_MAXBLKSIZE; - i.totalram <<= PAGE_SHIFT - 12; - while (nfsd_max_blksize > i.totalram && - nfsd_max_blksize >= 8*1024*2) - nfsd_max_blksize /= 2; - } + if (nfsd_max_blksize == 0) + nfsd_max_blksize = nfsd_get_default_max_blksize(); nfsd_reset_versions(); - nfsd_serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, nfsd_last_thread, nfsd, THIS_MODULE); if (nfsd_serv == NULL) @@ -341,7 +345,7 @@ int nfsd_create_serv(void) set_max_drc(); do_gettimeofday(&nfssvc_boot); /* record boot time */ - return err; + return 0; } int nfsd_nrpools(void) -- 1.7.5.4 >From 508f92275624fc755104b17945bdc822936f1918 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" <bfields@redhat.com> Date: Mon, 30 Jan 2012 16:21:11 -0500 Subject: [PATCH 2/2] nfsd: fix default iosize calculation on 32bit The rpc buffers will be allocated out of low memory, so we should really only be taking that into account. Signed-off-by: J. Bruce Fields <bfields@redhat.com> --- fs/nfsd/nfssvc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index e9eb408..aacf1f4 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -314,7 +314,7 @@ static int nfsd_get_default_max_blksize(void) unsigned long ret; si_meminfo(&i); - target = i.totalram << PAGE_SHIFT; + target = (i.totalram - i.totalhigh) << PAGE_SHIFT; /* * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig * machines, but only uses 32K on 128M machines. Bottom out at -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-02-03 20:49 ` J. Bruce Fields @ 2012-02-03 16:24 ` Mi Jinlong 2012-02-04 2:16 ` J. Bruce Fields 0 siblings, 1 reply; 9+ messages in thread From: Mi Jinlong @ 2012-02-03 16:24 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs 于 2012-2-4 4:49, J. Bruce Fields 写道: > On Wed, Feb 01, 2012 at 04:46:56PM -0500, bfields wrote: >> On Tue, Jan 31, 2012 at 09:11:48AM +0800, Mi Jinlong wrote: >>> Should use target = i.totalram<< (PAGE_SHIFT - 12); >>> >>> target = i.totalram<< PAGE_SHIFT; and >>> target<<= 12; >>> means target = i.totalram<< (PAGE_SHIFT + 12); >> >> Yes, thanks for catching that. >> >> Also, splitting up the calculation as I did above risks overflow at the >> first step. >> >> I'll fix that.... > > Here are the fixed patches.--b. > >> From 87b0fc7deb5feccf93b022f6a976e8441152dbb2 Mon Sep 17 00:00:00 2001 > From: "J. Bruce Fields"<bfields@redhat.com> > Date: Mon, 30 Jan 2012 16:18:35 -0500 > Subject: [PATCH 1/2] nfsd: cleanup setting of default max_block_size > > Move calculation of the default into a helper function. > > Get rid of an unused variable "err" while we're there. > > Thanks to Mi Jinlong for catching an arithmetic error in a previous > version. > > Cc: Mi Jinlong<mijinlong@cn.fujitsu.com> > Signed-off-by: J. Bruce Fields<bfields@redhat.com> > --- > fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- > 1 files changed, 24 insertions(+), 20 deletions(-) > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > index eda7d7e..e9eb408 100644 > --- a/fs/nfsd/nfssvc.c > +++ b/fs/nfsd/nfssvc.c > @@ -307,33 +307,37 @@ static void set_max_drc(void) > dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); > } > > -int nfsd_create_serv(void) > +static int nfsd_get_default_max_blksize(void) > { > - int err = 0; > + struct sysinfo i; > + unsigned long long target; > + unsigned long ret; > + > + si_meminfo(&i); > + target = i.totalram<< PAGE_SHIFT; > + /* > + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig > + * machines, but only uses 32K on 128M machines. Bottom out at > + * 8K on 32M and smaller. Of course, this is only a default. > + */ > + target>>= 12; Why don't using target = i.totalram << (PAGE_SHIFT - 12) as before? The result of the two forms is more likely different. Is there some other reason ? thanks, Mi Jinlong > + > + ret = NFSSVC_MAXBLKSIZE; > + while (ret> target&& ret>= 8*1024*2) > + ret /= 2; > + return ret; > +} > > +int nfsd_create_serv(void) > +{ > WARN_ON(!mutex_is_locked(&nfsd_mutex)); > if (nfsd_serv) { > svc_get(nfsd_serv); > return 0; > } > - if (nfsd_max_blksize == 0) { > - /* choose a suitable default */ > - struct sysinfo i; > - si_meminfo(&i); > - /* Aim for 1/4096 of memory per thread > - * This gives 1MB on 4Gig machines > - * But only uses 32K on 128M machines. > - * Bottom out at 8K on 32M and smaller. > - * Of course, this is only a default. > - */ > - nfsd_max_blksize = NFSSVC_MAXBLKSIZE; > - i.totalram<<= PAGE_SHIFT - 12; > - while (nfsd_max_blksize> i.totalram&& > - nfsd_max_blksize>= 8*1024*2) > - nfsd_max_blksize /= 2; > - } > + if (nfsd_max_blksize == 0) > + nfsd_max_blksize = nfsd_get_default_max_blksize(); > nfsd_reset_versions(); > - > nfsd_serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, > nfsd_last_thread, nfsd, THIS_MODULE); > if (nfsd_serv == NULL) > @@ -341,7 +345,7 @@ int nfsd_create_serv(void) > > set_max_drc(); > do_gettimeofday(&nfssvc_boot); /* record boot time */ > - return err; > + return 0; > } > > int nfsd_nrpools(void) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-02-03 16:24 ` Mi Jinlong @ 2012-02-04 2:16 ` J. Bruce Fields 2012-02-04 12:14 ` Mi Jinlong 0 siblings, 1 reply; 9+ messages in thread From: J. Bruce Fields @ 2012-02-04 2:16 UTC (permalink / raw) To: Mi Jinlong; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs On Sat, Feb 04, 2012 at 12:24:31AM +0800, Mi Jinlong wrote: > 于 2012-2-4 4:49, J. Bruce Fields 写道: > >On Wed, Feb 01, 2012 at 04:46:56PM -0500, bfields wrote: > >>On Tue, Jan 31, 2012 at 09:11:48AM +0800, Mi Jinlong wrote: > >>> Should use target = i.totalram<< (PAGE_SHIFT - 12); > >>> > >>> target = i.totalram<< PAGE_SHIFT; and > >>> target<<= 12; > >>> means target = i.totalram<< (PAGE_SHIFT + 12); > >> > >>Yes, thanks for catching that. > >> > >>Also, splitting up the calculation as I did above risks overflow at the > >>first step. > >> > >>I'll fix that.... > > > >Here are the fixed patches.--b. > > > >>From 87b0fc7deb5feccf93b022f6a976e8441152dbb2 Mon Sep 17 00:00:00 2001 > >From: "J. Bruce Fields"<bfields@redhat.com> > >Date: Mon, 30 Jan 2012 16:18:35 -0500 > >Subject: [PATCH 1/2] nfsd: cleanup setting of default max_block_size > > > >Move calculation of the default into a helper function. > > > >Get rid of an unused variable "err" while we're there. > > > >Thanks to Mi Jinlong for catching an arithmetic error in a previous > >version. > > > >Cc: Mi Jinlong<mijinlong@cn.fujitsu.com> > >Signed-off-by: J. Bruce Fields<bfields@redhat.com> > >--- > > fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- > > 1 files changed, 24 insertions(+), 20 deletions(-) > > > >diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > >index eda7d7e..e9eb408 100644 > >--- a/fs/nfsd/nfssvc.c > >+++ b/fs/nfsd/nfssvc.c > >@@ -307,33 +307,37 @@ static void set_max_drc(void) > > dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); > > } > > > >-int nfsd_create_serv(void) > >+static int nfsd_get_default_max_blksize(void) > > { > >- int err = 0; > >+ struct sysinfo i; > >+ unsigned long long target; > >+ unsigned long ret; > >+ > >+ si_meminfo(&i); > >+ target = i.totalram<< PAGE_SHIFT; > >+ /* > >+ * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig > >+ * machines, but only uses 32K on 128M machines. Bottom out at > >+ * 8K on 32M and smaller. Of course, this is only a default. > >+ */ > >+ target>>= 12; > > Why don't using target = i.totalram << (PAGE_SHIFT - 12) as before? Dividing the calculation into two steps (first convert from pages to bytes, then divide by 4096) makes it more obvious, and allows me to stick the comment before the part of the calculation it explains. So the result seems easier to read. > The result of the two forms is more likely different. The result is the same as long as there's no overflow at the first step (which would require a machine with an exabyte of ram). Seem reasonable? --b. > Is there some other reason ? > > thanks, > Mi Jinlong > > > >+ > >+ ret = NFSSVC_MAXBLKSIZE; > >+ while (ret> target&& ret>= 8*1024*2) > >+ ret /= 2; > >+ return ret; > >+} > > > >+int nfsd_create_serv(void) > >+{ > > WARN_ON(!mutex_is_locked(&nfsd_mutex)); > > if (nfsd_serv) { > > svc_get(nfsd_serv); > > return 0; > > } > >- if (nfsd_max_blksize == 0) { > >- /* choose a suitable default */ > >- struct sysinfo i; > >- si_meminfo(&i); > >- /* Aim for 1/4096 of memory per thread > >- * This gives 1MB on 4Gig machines > >- * But only uses 32K on 128M machines. > >- * Bottom out at 8K on 32M and smaller. > >- * Of course, this is only a default. > >- */ > >- nfsd_max_blksize = NFSSVC_MAXBLKSIZE; > >- i.totalram<<= PAGE_SHIFT - 12; > >- while (nfsd_max_blksize> i.totalram&& > >- nfsd_max_blksize>= 8*1024*2) > >- nfsd_max_blksize /= 2; > >- } > >+ if (nfsd_max_blksize == 0) > >+ nfsd_max_blksize = nfsd_get_default_max_blksize(); > > nfsd_reset_versions(); > >- > > nfsd_serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, > > nfsd_last_thread, nfsd, THIS_MODULE); > > if (nfsd_serv == NULL) > >@@ -341,7 +345,7 @@ int nfsd_create_serv(void) > > > > set_max_drc(); > > do_gettimeofday(&nfssvc_boot); /* record boot time */ > >- return err; > >+ return 0; > > } > > > > int nfsd_nrpools(void) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] nfsd: cleanup setting of default max_block_size 2012-02-04 2:16 ` J. Bruce Fields @ 2012-02-04 12:14 ` Mi Jinlong 0 siblings, 0 replies; 9+ messages in thread From: Mi Jinlong @ 2012-02-04 12:14 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs 于 2012-2-4 10:16, J. Bruce Fields 写道: > On Sat, Feb 04, 2012 at 12:24:31AM +0800, Mi Jinlong wrote: >> 于 2012-2-4 4:49, J. Bruce Fields 写道: >>> On Wed, Feb 01, 2012 at 04:46:56PM -0500, bfields wrote: >>>> On Tue, Jan 31, 2012 at 09:11:48AM +0800, Mi Jinlong wrote: >>>>> Should use target = i.totalram<< (PAGE_SHIFT - 12); >>>>> >>>>> target = i.totalram<< PAGE_SHIFT; and >>>>> target<<= 12; >>>>> means target = i.totalram<< (PAGE_SHIFT + 12); >>>> >>>> Yes, thanks for catching that. >>>> >>>> Also, splitting up the calculation as I did above risks overflow at the >>>> first step. >>>> >>>> I'll fix that.... >>> >>> Here are the fixed patches.--b. >>> >>> > From 87b0fc7deb5feccf93b022f6a976e8441152dbb2 Mon Sep 17 00:00:00 2001 >>> From: "J. Bruce Fields"<bfields@redhat.com> >>> Date: Mon, 30 Jan 2012 16:18:35 -0500 >>> Subject: [PATCH 1/2] nfsd: cleanup setting of default max_block_size >>> >>> Move calculation of the default into a helper function. >>> >>> Get rid of an unused variable "err" while we're there. >>> >>> Thanks to Mi Jinlong for catching an arithmetic error in a previous >>> version. >>> >>> Cc: Mi Jinlong<mijinlong@cn.fujitsu.com> >>> Signed-off-by: J. Bruce Fields<bfields@redhat.com> >>> --- >>> fs/nfsd/nfssvc.c | 44 ++++++++++++++++++++++++-------------------- >>> 1 files changed, 24 insertions(+), 20 deletions(-) >>> >>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c >>> index eda7d7e..e9eb408 100644 >>> --- a/fs/nfsd/nfssvc.c >>> +++ b/fs/nfsd/nfssvc.c >>> @@ -307,33 +307,37 @@ static void set_max_drc(void) >>> dprintk("%s nfsd_drc_max_mem %u \n", __func__, nfsd_drc_max_mem); >>> } >>> >>> -int nfsd_create_serv(void) >>> +static int nfsd_get_default_max_blksize(void) >>> { >>> - int err = 0; >>> + struct sysinfo i; >>> + unsigned long long target; >>> + unsigned long ret; >>> + >>> + si_meminfo(&i); >>> + target = i.totalram<< PAGE_SHIFT; >>> + /* >>> + * Aim for 1/4096 of memory per thread This gives 1MB on 4Gig >>> + * machines, but only uses 32K on 128M machines. Bottom out at >>> + * 8K on 32M and smaller. Of course, this is only a default. >>> + */ >>> + target>>= 12; >> >> Why don't using target = i.totalram<< (PAGE_SHIFT - 12) as before? > > Dividing the calculation into two steps (first convert from pages to > bytes, then divide by 4096) makes it more obvious, and allows me to > stick the comment before the part of the calculation it explains. > > So the result seems easier to read. Yes, > >> The result of the two forms is more likely different. > > The result is the same as long as there's no overflow at the first step > (which would require a machine with an exabyte of ram). > > Seem reasonable? Yes, that is my only concern. thanks, Mi Jinlong ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-02-04 12:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-30 22:42 [PATCH 1/2] nfsd: cleanup setting of default max_block_size J. Bruce Fields 2012-01-30 22:42 ` [PATCH 2/2] nfsd: fix default iosize calculation on 32bit J. Bruce Fields 2012-01-30 22:47 ` J. Bruce Fields 2012-01-31 1:11 ` [PATCH 1/2] nfsd: cleanup setting of default max_block_size Mi Jinlong 2012-02-01 21:46 ` J. Bruce Fields 2012-02-03 20:49 ` J. Bruce Fields 2012-02-03 16:24 ` Mi Jinlong 2012-02-04 2:16 ` J. Bruce Fields 2012-02-04 12:14 ` Mi Jinlong
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).