From: "J. Bruce Fields" <bfields@redhat.com>
To: linux-nfs@vger.kernel.org
Cc: "J. Bruce Fields" <bfields@redhat.com>
Subject: [PATCH 5/8] nfsd4: preallocate nfs4_file in process_open1()
Date: Mon, 17 Oct 2011 17:55:54 -0400 [thread overview]
Message-ID: <1318888557-14719-6-git-send-email-bfields@redhat.com> (raw)
In-Reply-To: <1318888557-14719-1-git-send-email-bfields@redhat.com>
From: "J. Bruce Fields" <bfields@redhat.com>
Creating a new file is an irrevocable step--once it's visible in the
filesystem, other processes may have seen it and done something with it,
and unlinking it wouldn't simply undo the effects of the create.
Therefore, in the case where OPEN creates a new file, we shouldn't do
the create until we know that the rest of the OPEN processing will
succeed.
For example, we should preallocate a struct file in case we need it
until waiting to allocate it till process_open2(), which is already too
late.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 60 ++++++++++++++++++++++++++++++--------------------
fs/nfsd/xdr4.h | 1 +
2 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2c9a1a2..ae5d250 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -104,6 +104,11 @@ opaque_hashval(const void *ptr, int nbytes)
static struct list_head del_recall_lru;
+static void nfsd4_free_file(struct nfs4_file *f)
+{
+ kmem_cache_free(file_slab, f);
+}
+
static inline void
put_nfs4_file(struct nfs4_file *fi)
{
@@ -111,7 +116,7 @@ put_nfs4_file(struct nfs4_file *fi)
list_del(&fi->fi_hash);
spin_unlock(&recall_lock);
iput(fi->fi_inode);
- kmem_cache_free(file_slab, fi);
+ nfsd4_free_file(fi);
}
}
@@ -2190,30 +2195,28 @@ out:
return status;
}
+static struct nfs4_file *nfsd4_alloc_file(void)
+{
+ return kmem_cache_alloc(file_slab, GFP_KERNEL);
+}
+
/* OPEN Share state helper functions */
-static inline struct nfs4_file *
-alloc_init_file(struct inode *ino)
+static void nfsd4_init_file(struct nfs4_file *fp, struct inode *ino)
{
- struct nfs4_file *fp;
unsigned int hashval = file_hashval(ino);
- fp = kmem_cache_alloc(file_slab, GFP_KERNEL);
- if (fp) {
- atomic_set(&fp->fi_ref, 1);
- INIT_LIST_HEAD(&fp->fi_hash);
- INIT_LIST_HEAD(&fp->fi_stateids);
- INIT_LIST_HEAD(&fp->fi_delegations);
- fp->fi_inode = igrab(ino);
- fp->fi_had_conflict = false;
- fp->fi_lease = NULL;
- memset(fp->fi_fds, 0, sizeof(fp->fi_fds));
- memset(fp->fi_access, 0, sizeof(fp->fi_access));
- spin_lock(&recall_lock);
- list_add(&fp->fi_hash, &file_hashtbl[hashval]);
- spin_unlock(&recall_lock);
- return fp;
- }
- return NULL;
+ atomic_set(&fp->fi_ref, 1);
+ INIT_LIST_HEAD(&fp->fi_hash);
+ INIT_LIST_HEAD(&fp->fi_stateids);
+ INIT_LIST_HEAD(&fp->fi_delegations);
+ fp->fi_inode = igrab(ino);
+ fp->fi_had_conflict = false;
+ fp->fi_lease = NULL;
+ memset(fp->fi_fds, 0, sizeof(fp->fi_fds));
+ memset(fp->fi_access, 0, sizeof(fp->fi_access));
+ spin_lock(&recall_lock);
+ list_add(&fp->fi_hash, &file_hashtbl[hashval]);
+ spin_unlock(&recall_lock);
}
static void
@@ -2509,6 +2512,13 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
if (STALE_CLIENTID(&open->op_clientid))
return nfserr_stale_clientid;
+ /*
+ * In case we need it later, after we've already created the
+ * file and don't want to risk a further failure:
+ */
+ open->op_file = nfsd4_alloc_file();
+ if (open->op_file == NULL)
+ return nfserr_jukebox;
strhashval = open_ownerstr_hashval(clientid->cl_id, &open->op_owner);
oo = find_openstateowner_str(strhashval, open);
@@ -2884,9 +2894,9 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
if (open->op_claim_type == NFS4_OPEN_CLAIM_DELEGATE_CUR)
goto out;
status = nfserr_jukebox;
- fp = alloc_init_file(ino);
- if (fp == NULL)
- goto out;
+ fp = open->op_file;
+ open->op_file = NULL;
+ nfsd4_init_file(fp, ino);
}
/*
@@ -2960,6 +2970,8 @@ void nfsd4_cleanup_open_state(struct nfsd4_open *open, __be32 status)
oo->oo_flags &= ~NFS4_OO_NEW;
}
}
+ if (open->op_file)
+ nfsd4_free_file(open->op_file);
}
__be32
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 32e6fd8..502dd43 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -228,6 +228,7 @@ struct nfsd4_open {
u32 op_rflags; /* response */
int op_truncate; /* used during processing */
struct nfs4_openowner *op_openowner; /* used during processing */
+ struct nfs4_file *op_file; /* used during processing */
struct nfs4_acl *op_acl;
};
#define op_iattr iattr
--
1.7.5.4
next prev parent reply other threads:[~2011-10-17 21:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-17 21:55 fix for open leaks, for 3.2 J. Bruce Fields
2011-10-17 21:55 ` [PATCH 1/8] nfsd4: centralize renew_client() calls J. Bruce Fields
2011-10-17 21:55 ` [PATCH 2/8] nfsd4: make is_open_owner boolean J. Bruce Fields
2011-10-17 21:55 ` [PATCH 3/8] nfsd4: simplify process_open1 logic J. Bruce Fields
2011-10-17 21:55 ` [PATCH 4/8] nfsd4: clean up open owners on OPEN failure J. Bruce Fields
2011-10-17 21:55 ` J. Bruce Fields [this message]
2011-10-17 21:55 ` [PATCH 6/8] nfsd4: do idr preallocation with stateid allocation J. Bruce Fields
2011-10-17 21:55 ` [PATCH 7/8] nfsd4: preallocate open stateid in process_open1() J. Bruce Fields
2011-10-17 21:55 ` [PATCH 8/8] nfsd4: warn on open failure after create J. Bruce Fields
2011-10-17 21:57 ` fix for open leaks, for 3.2 J. Bruce Fields
2011-10-17 22:00 ` Bryan Schumaker
2011-10-21 18:25 ` Bryan Schumaker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1318888557-14719-6-git-send-email-bfields@redhat.com \
--to=bfields@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).