On Wed, 2011-08-10 at 08:44 +0100, Berni Elbourn wrote:
> Instead of switching to cifs, I tried swapiness=100, and just doubling 
> up on the backup job:
> 
> 25 23 * * * disk-backup || (sleep 100; disk-backup)
> 
> Last night the first backup failed, second backup worked. Yippee.
> 
> The first backup failed by a simple test for the presence of a file the 
> nfs mount. I wonder is there some kind of cleanup event that the first 
> nfs mount failure causes that I could run on this system periodically?

I think the allocation attempt and failure will likely trigger flushing
of buffers and/or swapping to disk, which gradually allows memory to be
freed up.  By the time of the second attempt, the large block needed (64
KB of physically contiguous memory) may be free.

Although the new idmapper may be the way forward, we're stuck with the
old implementation for now.  We should still be able to get rid of the
huge allocation though.  If you're prepared to take the risk of a crash,
perhaps you could test the attached patch?  I haven't tested it at all,
but I *think* I know what I'm doing. :-)

Instructions for building a patched kernel package are at
<http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official>.

Ben.

From 82d6f77356d78411732b039055a487c47f36a9ca Mon Sep 17 00:00:00 2001
From: Ben Hutchings <b...@decadent.org.uk>
Date: Wed, 7 Sep 2011 03:14:15 +0100
Subject: [PATCH] NFS: Avoid huge memory allocation for old idmapper

struct idmap includes two hash tables implemented as simple arrays (no
indirection).  This makes it about 40K in size, requiring an order-4
allocation which is reasonably likely to fail on a busy machine.  Try
to avoid this by making the hash tables arrays of pointers and
allocating entries lazily.

Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
---
 fs/nfs/idmap.c |   36 +++++++++++++++++++++++++++---------
 1 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 21a84d4..dde6376 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -80,7 +80,7 @@ struct idmap_hashent {
 
 struct idmap_hashtable {
 	__u8			h_type;
-	struct idmap_hashent	h_entries[IDMAP_HASH_SZ];
+	struct idmap_hashent	*h_entries[IDMAP_HASH_SZ];
 };
 
 struct idmap {
@@ -141,18 +141,23 @@ void
 nfs_idmap_delete(struct nfs_client *clp)
 {
 	struct idmap *idmap = clp->cl_idmap;
+	unsigned int i;
 
 	if (!idmap)
 		return;
 	rpc_unlink(idmap->idmap_dentry);
 	clp->cl_idmap = NULL;
+	for (i = 0; i < IDMAP_HASH_SZ; i++)
+		kfree(idmap->idmap_user_hash.h_entries[i]);
+	for (i = 0; i < IDMAP_HASH_SZ; i++)
+		kfree(idmap->idmap_group_hash.h_entries[i]);
 	kfree(idmap);
 }
 
 /*
  * Helper routines for manipulating the hashtable
  */
-static inline struct idmap_hashent *
+static inline struct idmap_hashent **
 idmap_name_hash(struct idmap_hashtable* h, const char *name, size_t len)
 {
 	return &h->h_entries[fnvhash32(name, len) % IDMAP_HASH_SZ];
@@ -161,16 +166,16 @@ idmap_name_hash(struct idmap_hashtable* h, const char *name, size_t len)
 static struct idmap_hashent *
 idmap_lookup_name(struct idmap_hashtable *h, const char *name, size_t len)
 {
-	struct idmap_hashent *he = idmap_name_hash(h, name, len);
+	struct idmap_hashent *he = *idmap_name_hash(h, name, len);
 
-	if (he->ih_namelen != len || memcmp(he->ih_name, name, len) != 0)
+	if (!he || he->ih_namelen != len || memcmp(he->ih_name, name, len) != 0)
 		return NULL;
 	if (time_after(jiffies, he->ih_expires))
 		return NULL;
 	return he;
 }
 
-static inline struct idmap_hashent *
+static inline struct idmap_hashent **
 idmap_id_hash(struct idmap_hashtable* h, __u32 id)
 {
 	return &h->h_entries[fnvhash32(&id, sizeof(id)) % IDMAP_HASH_SZ];
@@ -179,8 +184,9 @@ idmap_id_hash(struct idmap_hashtable* h, __u32 id)
 static struct idmap_hashent *
 idmap_lookup_id(struct idmap_hashtable *h, __u32 id)
 {
-	struct idmap_hashent *he = idmap_id_hash(h, id);
-	if (he->ih_id != id || he->ih_namelen == 0)
+	struct idmap_hashent *he = *idmap_id_hash(h, id);
+
+	if (!he || he->ih_id != id || he->ih_namelen == 0)
 		return NULL;
 	if (time_after(jiffies, he->ih_expires))
 		return NULL;
@@ -193,15 +199,27 @@ idmap_lookup_id(struct idmap_hashtable *h, __u32 id)
  * pretty trivial.
  */
 static inline struct idmap_hashent *
+idmap_alloc_bucket(struct idmap_hashent **bucket)
+{
+	struct idmap_hashent *he = *bucket;
+
+	if (!he) {
+		he = kzalloc(sizeof(*he), GFP_NOFS);
+		*bucket = he;
+	}
+	return he;
+}
+
+static inline struct idmap_hashent *
 idmap_alloc_name(struct idmap_hashtable *h, char *name, size_t len)
 {
-	return idmap_name_hash(h, name, len);
+	return idmap_alloc_bucket(idmap_name_hash(h, name, len));
 }
 
 static inline struct idmap_hashent *
 idmap_alloc_id(struct idmap_hashtable *h, __u32 id)
 {
-	return idmap_id_hash(h, id);
+	return idmap_alloc_bucket(idmap_id_hash(h, id));
 }
 
 static void
-- 
1.7.5.4

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to