Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-27 Thread Andrew Morton
On Wed, 27 Jun 2007 22:57:39 -0400 "J. Bruce Fields" <[EMAIL PROTECTED]> wrote:

> On Wed, Jun 27, 2007 at 07:36:13PM -0700, Andrew Morton wrote:
> > How's this?
> 
> Makes sense to me, thanks.  I guess fs/nsfd/nfssvc:nfsd_create_serv()
> should be similarly modified?  It calculates the size of per-nfsd-thread
> buffers used to hold requests, which will eventually be allocated with
> an alloc_page(GFP_KERNEL), and it bases the calculation on the same
> .totalram field from struct sysinfo.

Yup, it'd be better to use nr_free_buffer_pages() there too.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-27 Thread J. Bruce Fields
On Wed, Jun 27, 2007 at 07:36:13PM -0700, Andrew Morton wrote:
> How's this?

Makes sense to me, thanks.  I guess fs/nsfd/nfssvc:nfsd_create_serv()
should be similarly modified?  It calculates the size of per-nfsd-thread
buffers used to hold requests, which will eventually be allocated with
an alloc_page(GFP_KERNEL), and it bases the calculation on the same
.totalram field from struct sysinfo.

--b.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-27 Thread Andrew Morton
On Wed, 27 Jun 2007 22:15:52 -0400 "J. Bruce Fields" <[EMAIL PROTECTED]> wrote:

> + /*
> +  * Allow at most 4 delegations per megabyte of RAM.  Quick
> +  * estimates suggest that in the worst case (where every delegation
> +  * is for a different inode), a delegation could take about 1.5K,
> +  * giving a worst case usage of about 6% of memory.
> +  */
>   sys.totalram >>= (18 - PAGE_SHIFT);

6% of 32GB is 2GB is too much for a large highmem machine.  We should base
this on the amount of kernel-addressable memory (aka lowmem).

How's this?

static void
set_max_delegations(void)
{
/*
 * Allow at most 4 delegations per megabyte of RAM.  Quick
 * estimates suggest that in the worst case (where every delegation
 * is for a different inode), a delegation could take about 1.5K,
 * giving a worst case usage of about 6% of memory.
 */
max_delegations = nr_free_buffer_pages() >> (20 - 2 - PAGE_SHIFT);
}


 fs/nfsd/nfs4state.c |7 ++-
 mm/page_alloc.c |1 +

diff -puN 
fs/nfsd/nfs4state.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
 fs/nfsd/nfs4state.c
--- 
a/fs/nfsd/nfs4state.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
+++ a/fs/nfsd/nfs4state.c
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -3210,17 +3211,13 @@ get_nfs4_grace_period(void)
 static void
 set_max_delegations(void)
 {
-   struct sysinfo sys;
-
-   si_meminfo(&sys);
/*
 * Allow at most 4 delegations per megabyte of RAM.  Quick
 * estimates suggest that in the worst case (where every delegation
 * is for a different inode), a delegation could take about 1.5K,
 * giving a worst case usage of about 6% of memory.
 */
-   sys.totalram >>= (18 - PAGE_SHIFT);
-   max_delegations = (unsigned int) sys.totalram;
+   max_delegations = nr_free_buffer_pages() >> (20 - 2 - PAGE_SHIFT);
 }
 
 /* initialization to perform when the nfsd service is started: */
diff -puN 
mm/page_alloc.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
 mm/page_alloc.c
--- 
a/mm/page_alloc.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
+++ a/mm/page_alloc.c
@@ -1729,6 +1729,7 @@ unsigned int nr_free_buffer_pages(void)
 {
return nr_free_zone_pages(gfp_zone(GFP_USER));
 }
+EXPORT_SYMBOL_GPL(nr_free_buffer_pages);
 
 /*
  * Amount of free RAM allocatable within all zones
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-27 Thread J. Bruce Fields
This code would benefit from a little more explanation.

Also, I screwed up the previous patch and forgot to delete a line, so the
calculation here is off; fix it.

Signed-off-by: J. Bruce Fields <[EMAIL PROTECTED]>
---

 fs/nfsd/nfs4state.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

On Mon, Jun 25, 2007 at 08:52:44PM -0700, Andrew Morton wrote:
>
> Please put yourself in the position of the reader who happens across this
> code and wonders why it is that way.
> 
> They could of course go hunt it out of the git repo but I do think it's
> quite a bit more reader-friendly to explain the thinking in code comments.
>

Yup, sorry about that.  Could you apply this?

--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fbbbcab..1b2657f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3203,13 +3203,27 @@ get_nfs4_grace_period(void)
return max(user_lease_time, lease_time) * HZ;
 }
 
+/*
+ * Since the lifetime of a delegation isn't limited to that of an open, a
+ * client may quite reasonably hang on to a delegation as long as it has
+ * the inode cached.  This becomes an obvious problem the first time a
+ * client's inode cache approaches the size of the server's total memory.
+ *
+ * For now we avoid this problem by imposing a hard limit on the number
+ * of delegations, which varies according to the server's memory size.
+ */
 static void
 set_max_delegations(void)
 {
struct sysinfo sys;
 
si_meminfo(&sys);
-   sys.totalram *= sys.mem_unit;
+   /*
+* Allow at most 4 delegations per megabyte of RAM.  Quick
+* estimates suggest that in the worst case (where every delegation
+* is for a different inode), a delegation could take about 1.5K,
+* giving a worst case usage of about 6% of memory.
+*/
sys.totalram >>= (18 - PAGE_SHIFT);
max_delegations = (unsigned int) sys.totalram;
 }
-- 
1.5.2.58.g98ee

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-25 Thread Andrew Morton
On Thu, 21 Jun 2007 14:31:12 +1000 NeilBrown <[EMAIL PROTECTED]> wrote:

> 
> From: "J. Bruce Fields" <[EMAIL PROTECTED]>
> 
> 
> Our original NFSv4 delegation policy was to give out a read delegation
> on any open when it was possible to.
> 
> Since the lifetime of a delegation isn't limited to that of an open, a
> client may quite reasonably hang on to a delegation as long as it has
> the inode cached.  This becomes an obvious problem the first time a
> client's inode cache approaches the size of the server's total memory.
> 
> Our first quick solution was to add a hard-coded limit.  This patch
> makes a mild incremental improvement by varying that limit according to
> the server's total memory size, allowing at most 4 delegations per
> megabyte of RAM.
> 
> My quick back-of-the-envelope calculation finds that in the worst case
> (where every delegation is for a different inode), a delegation could
> take about 1.5K, which would make the worst case usage about 6% of
> memory.  The new limit works out to be about the same as the old on a
> 1-gig server.
> 
> ...
>  
> +static void
> +set_max_delegations()
> +{
> + struct sysinfo sys;
> +
> + si_meminfo(&sys);
> + sys.totalram *= sys.mem_unit;
> + sys.totalram >>= (18 - PAGE_SHIFT);
> + max_delegations = (unsigned int) sys.totalram;
> +}

Please put yourself in the position of the reader who happens across this
code and wonders why it is that way.

They could of course go hunt it out of the git repo but I do think it's
quite a bit more reader-friendly to explain the thinking in code comments.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size

2007-06-21 Thread J. Bruce Fields
I missed a compile warning here, apologies.

--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c79f742..fbbbcab 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3204,7 +3204,7 @@ get_nfs4_grace_period(void)
 }
 
 static void
-set_max_delegations()
+set_max_delegations(void)
 {
struct sysinfo sys;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/