* Empty core dumps on NFSv4 mounts
@ 2010-07-02 8:01 Arnaud Giersch
2010-07-02 13:03 ` Trond Myklebust
0 siblings, 1 reply; 5+ messages in thread
From: Arnaud Giersch @ 2010-07-02 8:01 UTC (permalink / raw)
To: linux-nfs
Hi,
On NFSv4 mounts, many core dumps are empty, although ulimit -c is
unlimited. An ls command shortly after the core dump often shows
4294967294 (2^32-2) as UID and GID for the "core" file.
This only happens when there was no "core" file before the dump. If a
"core" file owned by the current user is already present, it is
correctly filled.
After having done a git bisect, it seems that the problem was
introduced by commit 80e52aced138bb41b045a8595a87510f27d8d8c5
(NFSv4: Don't do idmapper upcalls for asynchronous RPC calls).
If I understand correctly what happens, do_coredump() [fs/exec.c] fails
because (inode->i_uid != current_fsuid()). In fact inode->i_uid equals
-2, because decode_attr_owner() [fs/nfs/nfs4xdr.c], which is called from
nfs4_xdr_dec_open() via decode_getfattr(), returns without calling
nfs_map_to_uid(), since its may_sleep parameter is false.
I however do not clearly understand what the aforementioned commit is
supposed to fix. I read the linux-nfs mailing list archive, and tried
some google search, but I didn't find anything.
Regards,
Arnaud Giersch
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Empty core dumps on NFSv4 mounts 2010-07-02 8:01 Empty core dumps on NFSv4 mounts Arnaud Giersch @ 2010-07-02 13:03 ` Trond Myklebust 2010-07-05 13:28 ` [PATCH/RFC 2.6.35-rc4] NFSv4: Don't do idmapper upcalls during XDR decode Arnaud Giersch 0 siblings, 1 reply; 5+ messages in thread From: Trond Myklebust @ 2010-07-02 13:03 UTC (permalink / raw) To: Arnaud Giersch; +Cc: linux-nfs On Fri, 2010-07-02 at 10:01 +0200, Arnaud Giersch wrote: > Hi, > > On NFSv4 mounts, many core dumps are empty, although ulimit -c is > unlimited. An ls command shortly after the core dump often shows > 4294967294 (2^32-2) as UID and GID for the "core" file. > > This only happens when there was no "core" file before the dump. If a > "core" file owned by the current user is already present, it is > correctly filled. > > After having done a git bisect, it seems that the problem was > introduced by commit 80e52aced138bb41b045a8595a87510f27d8d8c5 > (NFSv4: Don't do idmapper upcalls for asynchronous RPC calls). > > If I understand correctly what happens, do_coredump() [fs/exec.c] fails > because (inode->i_uid != current_fsuid()). In fact inode->i_uid equals > -2, because decode_attr_owner() [fs/nfs/nfs4xdr.c], which is called from > nfs4_xdr_dec_open() via decode_getfattr(), returns without calling > nfs_map_to_uid(), since its may_sleep parameter is false. > > I however do not clearly understand what the aforementioned commit is > supposed to fix. I read the linux-nfs mailing list archive, and tried > some google search, but I didn't find anything. rpciod threads should never do anything that can cause them to sleep. Doing an upcall as part of an XDR decode is therefore verboten. We should ideally rather be saving the owner/group strings and doing the upcall after the XDR is done. While that wasn't feasible when 'fattr' structs were being allocated on the stack, now that they are dynamically allocated, maybe we can rethink that... Cheers Trond ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH/RFC 2.6.35-rc4] NFSv4: Don't do idmapper upcalls during XDR decode 2010-07-02 13:03 ` Trond Myklebust @ 2010-07-05 13:28 ` Arnaud Giersch 2010-08-12 10:15 ` [PATCH 2.6.35] " Arnaud Giersch 0 siblings, 1 reply; 5+ messages in thread From: Arnaud Giersch @ 2010-07-05 13:28 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs From: Arnaud Giersch <arnaud.giersch@free.fr> Move the name to id mapping out of the XDR decode functions. Add two new fields to struct nfs_fattr: user_name and group_name, that are defined iff a name to id mapping should be done. A new function, nfs_fattr_name_to_id(), is defined to call the idmapper if needed. This also fixes the empty core dumps that may be produced since commit 80e52aced138bb41b045a8595a87510f27d8d8c5. Signed-off-by: Arnaud Giersch <arnaud.giersch@free.fr> --- Trond, did you think about something like this when you wrote "saving the owner/group strings and doing the upcall after the XDR is done"? I tagged the patch RFC mainly because I am not sure of the following: * if the kmalloc() call if OK inside decode_attr_owner() and decode_attr_group(); * the calls to nfs_fattr_name_to_id() are done at the proper places. Regards, Arnaud fs/nfs/inode.c | 35 ++++++++++++ fs/nfs/nfs4xdr.c | 109 ++++++++++++++----------------------- include/linux/nfs_fs.h | 2 + include/linux/nfs_xdr.h | 7 +++ 4 files changed, 86 insertions(+), 67 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 099b351..6df59ad 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -227,6 +227,35 @@ nfs_init_locked(struct inode *inode, void *opaque) return 0; } +static void +nfs_fattr_name_to_id(struct nfs_client *clp, struct nfs_fattr *fattr) +{ + if (fattr->user_name) { + struct nfs_name *name = fattr->user_name; + if (nfs_map_name_to_uid(clp, name->buf, name->len, + &fattr->uid) == 0) + fattr->valid |= NFS_ATTR_FATTR_OWNER; + else + dprintk("%s: nfs_map_name_to_uid failed!\n", + __func__); + kfree(fattr->user_name); + fattr->user_name = NULL; + dprintk("%s: uid=%d\n", __func__, (int)fattr->uid); + } + if (fattr->group_name) { + struct nfs_name *name = fattr->group_name; + if (nfs_map_group_to_gid(clp, name->buf, name->len, + &fattr->gid) == 0) + fattr->valid |= NFS_ATTR_FATTR_GROUP; + else + dprintk("%s: nfs_map_group_to_gid failed!\n", + __func__); + kfree(fattr->group_name); + fattr->group_name = NULL; + dprintk("%s: gid=%d\n", __func__, (int)fattr->gid); + } +} + /* Don't use READDIRPLUS on directories that we believe are too large */ #define NFS_LIMIT_READDIRPLUS (8*PAGE_SIZE) @@ -256,6 +285,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) inode = ERR_PTR(-ENOMEM); goto out_no_inode; } + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if (inode->i_state & I_NEW) { struct nfs_inode *nfsi = NFS_I(inode); @@ -930,6 +960,8 @@ unsigned long nfs_inc_attr_generation_counter(void) void nfs_fattr_init(struct nfs_fattr *fattr) { fattr->valid = 0; + fattr->user_name = NULL; + fattr->group_name = NULL; fattr->time_start = jiffies; fattr->gencount = nfs_inc_attr_generation_counter(); } @@ -1004,6 +1036,7 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if ((fattr->valid & NFS_ATTR_FATTR) == 0) return 0; spin_lock(&inode->i_lock); @@ -1043,6 +1076,7 @@ int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); status = nfs_post_op_update_inode_locked(inode, fattr); spin_unlock(&inode->i_lock); @@ -1064,6 +1098,7 @@ int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fa { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); /* Don't do a WCC update if these attributes are already stale */ if ((fattr->valid & NFS_ATTR_FATTR) == 0 || diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 65c8dae..b4c2735 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -3227,13 +3227,13 @@ out_overflow: } static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *uid, int may_sleep) + struct nfs_client *clp, struct nfs_name **owner) { uint32_t len; __be32 *p; int ret = 0; - *uid = -2; + BUG_ON(*owner); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER)) { @@ -3244,20 +3244,18 @@ static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_name_to_uid(clp, (char *)p, len, uid) == 0) - ret = NFS_ATTR_FATTR_OWNER; - else - dprintk("%s: nfs_map_name_to_uid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *owner = kmalloc((sizeof **owner) + len, GFP_NOFS); + if (*owner) { + (*owner)->len = len; + memcpy((*owner)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER; } - dprintk("%s: uid=%d\n", __func__, (int)*uid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3265,13 +3263,13 @@ out_overflow: } static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *gid, int may_sleep) + struct nfs_client *clp, struct nfs_name **group) { uint32_t len; __be32 *p; int ret = 0; - *gid = -2; + BUG_ON(*group); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER_GROUP - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER_GROUP)) { @@ -3282,20 +3280,18 @@ static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_group_to_gid(clp, (char *)p, len, gid) == 0) - ret = NFS_ATTR_FATTR_GROUP; - else - dprintk("%s: nfs_map_group_to_gid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *group = kmalloc((sizeof **group) + len, GFP_NOFS); + if (*group) { + (*group)->len = len; + memcpy((*group)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER_GROUP; } - dprintk("%s: gid=%d\n", __func__, (int)*gid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3701,7 +3697,7 @@ xdr_error: } static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, - const struct nfs_server *server, int may_sleep) + const struct nfs_server *server) { __be32 *savep; uint32_t attrlen, @@ -3774,13 +3770,13 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, fattr->valid |= status; status = decode_attr_owner(xdr, bitmap, server->nfs_client, - &fattr->uid, may_sleep); + &fattr->user_name); if (status < 0) goto xdr_error; fattr->valid |= status; status = decode_attr_group(xdr, bitmap, server->nfs_client, - &fattr->gid, may_sleep); + &fattr->group_name); if (status < 0) goto xdr_error; fattr->valid |= status; @@ -4708,8 +4704,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct status = decode_open_downgrade(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4736,8 +4731,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac status = decode_access(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4764,8 +4758,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server - ,!RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4789,8 +4782,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf if ((status = decode_putrootfh(&xdr)) != 0) goto out; if ((status = decode_getfh(&xdr, res->fh)) == 0) - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4815,8 +4807,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem goto out; if ((status = decode_remove(&xdr, &res->cinfo)) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -4846,13 +4837,11 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re if ((status = decode_rename(&xdr, &res->old_cinfo, &res->new_cinfo)) != 0) goto out; /* Current FH is target directory */ - if (decode_getfattr(&xdr, res->new_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->new_fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->old_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->old_fattr, res->server); out: return status; } @@ -4885,13 +4874,11 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link * Note order: OP_LINK leaves the directory as the current * filehandle. */ - if (decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->dir_attr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4920,13 +4907,11 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - if (decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->dir_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_fattr, res->server); out: return status; } @@ -4958,8 +4943,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g status = decode_putfh(&xdr); if (status) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5066,8 +5050,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos * an ESTALE error. Shouldn't be a problem, * though, since fattr->valid will remain unset. */ - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5099,13 +5082,11 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr goto out; if (decode_getfh(&xdr, &res->fh) != 0) goto out; - if (decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->f_attr, res->server) != 0) goto out; if (decode_restorefh(&xdr) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -5153,8 +5134,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_open(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->f_attr, res->server); out: return status; } @@ -5181,8 +5161,7 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se status = decode_setattr(&xdr); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5356,8 +5335,7 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ status = decode_write(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); if (!status) status = res->count; out: @@ -5386,8 +5364,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri status = decode_commit(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5553,8 +5530,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_delegreturn(&xdr); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5582,8 +5558,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, goto out; xdr_enter_page(&xdr, PAGE_SIZE); status = decode_getfattr(&xdr, &res->fs_locations->fattr, - res->fs_locations->server, - !RPC_IS_ASYNC(req->rq_task)); + res->fs_locations->server); out: return status; } diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 77c2ae5..fd4cea0 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -360,6 +360,8 @@ extern struct nfs_fattr *nfs_alloc_fattr(void); static inline void nfs_free_fattr(const struct nfs_fattr *fattr) { + kfree(fattr->user_name); + kfree(fattr->group_name); kfree(fattr); } diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 51914d7..074c3db 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -27,10 +27,17 @@ static inline int nfs_fsid_equal(const struct nfs_fsid *a, const struct nfs_fsid return a->major == b->major && a->minor == b->minor; } +struct nfs_name { + size_t len; + char buf[]; +}; + struct nfs_fattr { unsigned int valid; /* which fields are valid */ umode_t mode; __u32 nlink; + struct nfs_name * user_name; + struct nfs_name * group_name; __u32 uid; __u32 gid; dev_t rdev; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2.6.35] NFSv4: Don't do idmapper upcalls during XDR decode 2010-07-05 13:28 ` [PATCH/RFC 2.6.35-rc4] NFSv4: Don't do idmapper upcalls during XDR decode Arnaud Giersch @ 2010-08-12 10:15 ` Arnaud Giersch 2010-09-21 11:17 ` [PATCH resend] " Arnaud Giersch 0 siblings, 1 reply; 5+ messages in thread From: Arnaud Giersch @ 2010-08-12 10:15 UTC (permalink / raw) To: linux-nfs, linux-kernel; +Cc: Trond Myklebust From: Arnaud Giersch <arnaud.giersch@free.fr> Move the name to id mapping out of the XDR decode functions. Add two new fields to struct nfs_fattr: user_name and group_name, that are defined iff a name to id mapping should be done. A new function, nfs_fattr_name_to_id(), is defined to call the idmapper if needed. This also fixes the empty core dumps that may be produced on NFSv4 mounts since commit 80e52aced138bb41b045a8595a87510f27d8d8c5. Signed-off-by: Arnaud Giersch <arnaud.giersch@free.fr> --- Hi, The purpose of this patch is to avoid empty core dumps on NFS4 mounts, as reported in http://article.gmane.org/gmane.linux.kernel/998617, or http://article.gmane.org/gmane.linux.nfs/33390. This is my attempt to implement the suggestion made by Trond Myklebust in http://article.gmane.org/gmane.linux.nfs/33391. A previous submission (http://article.gmane.org/gmane.linux.nfs/33422) did not get any feedback. The only difference with this first patch is the test in nfs_free_fattr(), as the fattr parameter may be NULL. I am currently running a patched kernel on my workstation, and did not notice any problem. Regards, Arnaud Giersch fs/nfs/inode.c | 35 ++++++++++++ fs/nfs/nfs4xdr.c | 109 ++++++++++++++----------------------- include/linux/nfs_fs.h | 4 ++ include/linux/nfs_xdr.h | 7 +++ 4 files changed, 88 insertions(+), 67 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 7d2d6c7..b1f7799 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -234,6 +234,35 @@ nfs_init_locked(struct inode *inode, void *opaque) return 0; } +static void +nfs_fattr_name_to_id(struct nfs_client *clp, struct nfs_fattr *fattr) +{ + if (fattr->user_name) { + struct nfs_name *name = fattr->user_name; + if (nfs_map_name_to_uid(clp, name->buf, name->len, + &fattr->uid) == 0) + fattr->valid |= NFS_ATTR_FATTR_OWNER; + else + dprintk("%s: nfs_map_name_to_uid failed!\n", + __func__); + kfree(fattr->user_name); + fattr->user_name = NULL; + dprintk("%s: uid=%d\n", __func__, (int)fattr->uid); + } + if (fattr->group_name) { + struct nfs_name *name = fattr->group_name; + if (nfs_map_group_to_gid(clp, name->buf, name->len, + &fattr->gid) == 0) + fattr->valid |= NFS_ATTR_FATTR_GROUP; + else + dprintk("%s: nfs_map_group_to_gid failed!\n", + __func__); + kfree(fattr->group_name); + fattr->group_name = NULL; + dprintk("%s: gid=%d\n", __func__, (int)fattr->gid); + } +} + /* Don't use READDIRPLUS on directories that we believe are too large */ #define NFS_LIMIT_READDIRPLUS (8*PAGE_SIZE) @@ -263,6 +292,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) inode = ERR_PTR(-ENOMEM); goto out_no_inode; } + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if (inode->i_state & I_NEW) { struct nfs_inode *nfsi = NFS_I(inode); @@ -997,6 +1027,8 @@ unsigned long nfs_inc_attr_generation_counter(void) void nfs_fattr_init(struct nfs_fattr *fattr) { fattr->valid = 0; + fattr->user_name = NULL; + fattr->group_name = NULL; fattr->time_start = jiffies; fattr->gencount = nfs_inc_attr_generation_counter(); } @@ -1071,6 +1103,7 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if ((fattr->valid & NFS_ATTR_FATTR) == 0) return 0; spin_lock(&inode->i_lock); @@ -1110,6 +1143,7 @@ int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); status = nfs_post_op_update_inode_locked(inode, fattr); spin_unlock(&inode->i_lock); @@ -1131,6 +1165,7 @@ int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fa { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); /* Don't do a WCC update if these attributes are already stale */ if ((fattr->valid & NFS_ATTR_FATTR) == 0 || diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 08ef912..c6e7197 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -3271,13 +3271,13 @@ out_overflow: } static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *uid, int may_sleep) + struct nfs_client *clp, struct nfs_name **owner) { uint32_t len; __be32 *p; int ret = 0; - *uid = -2; + BUG_ON(*owner); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER)) { @@ -3288,20 +3288,18 @@ static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_name_to_uid(clp, (char *)p, len, uid) == 0) - ret = NFS_ATTR_FATTR_OWNER; - else - dprintk("%s: nfs_map_name_to_uid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *owner = kmalloc((sizeof **owner) + len, GFP_NOFS); + if (*owner) { + (*owner)->len = len; + memcpy((*owner)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER; } - dprintk("%s: uid=%d\n", __func__, (int)*uid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3309,13 +3307,13 @@ out_overflow: } static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *gid, int may_sleep) + struct nfs_client *clp, struct nfs_name **group) { uint32_t len; __be32 *p; int ret = 0; - *gid = -2; + BUG_ON(*group); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER_GROUP - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER_GROUP)) { @@ -3326,20 +3324,18 @@ static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_group_to_gid(clp, (char *)p, len, gid) == 0) - ret = NFS_ATTR_FATTR_GROUP; - else - dprintk("%s: nfs_map_group_to_gid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *group = kmalloc((sizeof **group) + len, GFP_NOFS); + if (*group) { + (*group)->len = len; + memcpy((*group)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER_GROUP; } - dprintk("%s: gid=%d\n", __func__, (int)*gid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3745,7 +3741,7 @@ xdr_error: } static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, - const struct nfs_server *server, int may_sleep) + const struct nfs_server *server) { __be32 *savep; uint32_t attrlen, @@ -3818,13 +3814,13 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, fattr->valid |= status; status = decode_attr_owner(xdr, bitmap, server->nfs_client, - &fattr->uid, may_sleep); + &fattr->user_name); if (status < 0) goto xdr_error; fattr->valid |= status; status = decode_attr_group(xdr, bitmap, server->nfs_client, - &fattr->gid, may_sleep); + &fattr->group_name); if (status < 0) goto xdr_error; fattr->valid |= status; @@ -4757,8 +4753,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct status = decode_open_downgrade(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4785,8 +4780,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac status = decode_access(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4813,8 +4807,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server - ,!RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4838,8 +4831,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf if ((status = decode_putrootfh(&xdr)) != 0) goto out; if ((status = decode_getfh(&xdr, res->fh)) == 0) - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4864,8 +4856,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem goto out; if ((status = decode_remove(&xdr, &res->cinfo)) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -4895,13 +4886,11 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re if ((status = decode_rename(&xdr, &res->old_cinfo, &res->new_cinfo)) != 0) goto out; /* Current FH is target directory */ - if (decode_getfattr(&xdr, res->new_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->new_fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->old_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->old_fattr, res->server); out: return status; } @@ -4934,13 +4923,11 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link * Note order: OP_LINK leaves the directory as the current * filehandle. */ - if (decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->dir_attr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4969,13 +4956,11 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - if (decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->dir_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_fattr, res->server); out: return status; } @@ -5007,8 +4992,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g status = decode_putfh(&xdr); if (status) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5115,8 +5099,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos * an ESTALE error. Shouldn't be a problem, * though, since fattr->valid will remain unset. */ - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5148,13 +5131,11 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr goto out; if (decode_getfh(&xdr, &res->fh) != 0) goto out; - if (decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->f_attr, res->server) != 0) goto out; if (decode_restorefh(&xdr) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -5202,8 +5183,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_open(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->f_attr, res->server); out: return status; } @@ -5230,8 +5210,7 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se status = decode_setattr(&xdr); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5418,8 +5397,7 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ status = decode_write(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); if (!status) status = res->count; out: @@ -5448,8 +5426,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri status = decode_commit(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5615,8 +5592,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_delegreturn(&xdr); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5644,8 +5620,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, goto out; xdr_enter_page(&xdr, PAGE_SIZE); status = decode_getfattr(&xdr, &res->fs_locations->fattr, - res->fs_locations->server, - !RPC_IS_ASYNC(req->rq_task)); + res->fs_locations->server); out: return status; } diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 508f8cf..6ce7a08 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -369,6 +369,10 @@ extern struct nfs_fattr *nfs_alloc_fattr(void); static inline void nfs_free_fattr(const struct nfs_fattr *fattr) { + if (!fattr) + return; + kfree(fattr->user_name); + kfree(fattr->group_name); kfree(fattr); } diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index fc46192..fc58d5d 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -27,10 +27,17 @@ static inline int nfs_fsid_equal(const struct nfs_fsid *a, const struct nfs_fsid return a->major == b->major && a->minor == b->minor; } +struct nfs_name { + size_t len; + char buf[]; +}; + struct nfs_fattr { unsigned int valid; /* which fields are valid */ umode_t mode; __u32 nlink; + struct nfs_name *user_name; + struct nfs_name *group_name; __u32 uid; __u32 gid; dev_t rdev; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH resend] NFSv4: Don't do idmapper upcalls during XDR decode 2010-08-12 10:15 ` [PATCH 2.6.35] " Arnaud Giersch @ 2010-09-21 11:17 ` Arnaud Giersch 0 siblings, 0 replies; 5+ messages in thread From: Arnaud Giersch @ 2010-09-21 11:17 UTC (permalink / raw) To: linux-nfs; +Cc: linux-kernel, Trond Myklebust From: Arnaud Giersch <arnaud.giersch@free.fr> Move the name to id mapping out of the XDR decode functions. Add two new fields to struct nfs_fattr: user_name and group_name, that are defined iff a name to id mapping should be done. A new function, nfs_fattr_name_to_id(), is defined to call the idmapper if needed. This also fixes the empty core dumps that may be produced on NFSv4 mounts since commit 80e52aced138bb41b045a8595a87510f27d8d8c5. Signed-off-by: Arnaud Giersch <arnaud.giersch@free.fr> --- Hi, [ Resend, since a previous sending on August 12 did not get any answer. ] The purpose of this patch is to avoid empty core dumps on NFS4 mounts, as reported in http://article.gmane.org/gmane.linux.kernel/998617, or http://article.gmane.org/gmane.linux.nfs/33390. This is my attempt to implement the suggestion made by Trond Myklebust in http://article.gmane.org/gmane.linux.nfs/33391. A previous submission (http://article.gmane.org/gmane.linux.nfs/33422) did not get any feedback. The only difference with this first patch is the test in nfs_free_fattr(), as the fattr parameter may be NULL. I am currently running a patched kernel on my workstation, and did not notice any problem. Regards, Arnaud Giersch fs/nfs/inode.c | 35 ++++++++++++ fs/nfs/nfs4xdr.c | 109 ++++++++++++++----------------------- include/linux/nfs_fs.h | 4 ++ include/linux/nfs_xdr.h | 7 +++ 4 files changed, 88 insertions(+), 67 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 7d2d6c7..b1f7799 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -234,6 +234,35 @@ nfs_init_locked(struct inode *inode, void *opaque) return 0; } +static void +nfs_fattr_name_to_id(struct nfs_client *clp, struct nfs_fattr *fattr) +{ + if (fattr->user_name) { + struct nfs_name *name = fattr->user_name; + if (nfs_map_name_to_uid(clp, name->buf, name->len, + &fattr->uid) == 0) + fattr->valid |= NFS_ATTR_FATTR_OWNER; + else + dprintk("%s: nfs_map_name_to_uid failed!\n", + __func__); + kfree(fattr->user_name); + fattr->user_name = NULL; + dprintk("%s: uid=%d\n", __func__, (int)fattr->uid); + } + if (fattr->group_name) { + struct nfs_name *name = fattr->group_name; + if (nfs_map_group_to_gid(clp, name->buf, name->len, + &fattr->gid) == 0) + fattr->valid |= NFS_ATTR_FATTR_GROUP; + else + dprintk("%s: nfs_map_group_to_gid failed!\n", + __func__); + kfree(fattr->group_name); + fattr->group_name = NULL; + dprintk("%s: gid=%d\n", __func__, (int)fattr->gid); + } +} + /* Don't use READDIRPLUS on directories that we believe are too large */ #define NFS_LIMIT_READDIRPLUS (8*PAGE_SIZE) @@ -263,6 +292,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) inode = ERR_PTR(-ENOMEM); goto out_no_inode; } + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if (inode->i_state & I_NEW) { struct nfs_inode *nfsi = NFS_I(inode); @@ -997,6 +1027,8 @@ unsigned long nfs_inc_attr_generation_counter(void) void nfs_fattr_init(struct nfs_fattr *fattr) { fattr->valid = 0; + fattr->user_name = NULL; + fattr->group_name = NULL; fattr->time_start = jiffies; fattr->gencount = nfs_inc_attr_generation_counter(); } @@ -1071,6 +1103,7 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); if ((fattr->valid & NFS_ATTR_FATTR) == 0) return 0; spin_lock(&inode->i_lock); @@ -1110,6 +1143,7 @@ int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr) { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); status = nfs_post_op_update_inode_locked(inode, fattr); spin_unlock(&inode->i_lock); @@ -1131,6 +1165,7 @@ int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fa { int status; + nfs_fattr_name_to_id(NFS_SERVER(inode)->nfs_client, fattr); spin_lock(&inode->i_lock); /* Don't do a WCC update if these attributes are already stale */ if ((fattr->valid & NFS_ATTR_FATTR) == 0 || diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 08ef912..c6e7197 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -3271,13 +3271,13 @@ out_overflow: } static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *uid, int may_sleep) + struct nfs_client *clp, struct nfs_name **owner) { uint32_t len; __be32 *p; int ret = 0; - *uid = -2; + BUG_ON(*owner); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER)) { @@ -3288,20 +3288,18 @@ static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_name_to_uid(clp, (char *)p, len, uid) == 0) - ret = NFS_ATTR_FATTR_OWNER; - else - dprintk("%s: nfs_map_name_to_uid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *owner = kmalloc((sizeof **owner) + len, GFP_NOFS); + if (*owner) { + (*owner)->len = len; + memcpy((*owner)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER; } - dprintk("%s: uid=%d\n", __func__, (int)*uid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3309,13 +3307,13 @@ out_overflow: } static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs_client *clp, uint32_t *gid, int may_sleep) + struct nfs_client *clp, struct nfs_name **group) { uint32_t len; __be32 *p; int ret = 0; - *gid = -2; + BUG_ON(*group); if (unlikely(bitmap[1] & (FATTR4_WORD1_OWNER_GROUP - 1U))) return -EIO; if (likely(bitmap[1] & FATTR4_WORD1_OWNER_GROUP)) { @@ -3326,20 +3324,18 @@ static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap, p = xdr_inline_decode(xdr, len); if (unlikely(!p)) goto out_overflow; - if (!may_sleep) { - /* do nothing */ - } else if (len < XDR_MAX_NETOBJ) { - if (nfs_map_group_to_gid(clp, (char *)p, len, gid) == 0) - ret = NFS_ATTR_FATTR_GROUP; - else - dprintk("%s: nfs_map_group_to_gid failed!\n", - __func__); + if (len < XDR_MAX_NETOBJ) { + *group = kmalloc((sizeof **group) + len, GFP_NOFS); + if (*group) { + (*group)->len = len; + memcpy((*group)->buf, p, len); + } else + dprintk("%s: kmalloc failed!\n", __func__); } else dprintk("%s: name too long (%u)!\n", __func__, len); bitmap[1] &= ~FATTR4_WORD1_OWNER_GROUP; } - dprintk("%s: gid=%d\n", __func__, (int)*gid); return ret; out_overflow: print_overflow_msg(__func__, xdr); @@ -3745,7 +3741,7 @@ xdr_error: } static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, - const struct nfs_server *server, int may_sleep) + const struct nfs_server *server) { __be32 *savep; uint32_t attrlen, @@ -3818,13 +3814,13 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, fattr->valid |= status; status = decode_attr_owner(xdr, bitmap, server->nfs_client, - &fattr->uid, may_sleep); + &fattr->user_name); if (status < 0) goto xdr_error; fattr->valid |= status; status = decode_attr_group(xdr, bitmap, server->nfs_client, - &fattr->gid, may_sleep); + &fattr->group_name); if (status < 0) goto xdr_error; fattr->valid |= status; @@ -4757,8 +4753,7 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, __be32 *p, struct status = decode_open_downgrade(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4785,8 +4780,7 @@ static int nfs4_xdr_dec_access(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_ac status = decode_access(&xdr, res); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4813,8 +4807,7 @@ static int nfs4_xdr_dec_lookup(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_lo goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server - ,!RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4838,8 +4831,7 @@ static int nfs4_xdr_dec_lookup_root(struct rpc_rqst *rqstp, __be32 *p, struct nf if ((status = decode_putrootfh(&xdr)) != 0) goto out; if ((status = decode_getfh(&xdr, res->fh)) == 0) - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4864,8 +4856,7 @@ static int nfs4_xdr_dec_remove(struct rpc_rqst *rqstp, __be32 *p, struct nfs_rem goto out; if ((status = decode_remove(&xdr, &res->cinfo)) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -4895,13 +4886,11 @@ static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_re if ((status = decode_rename(&xdr, &res->old_cinfo, &res->new_cinfo)) != 0) goto out; /* Current FH is target directory */ - if (decode_getfattr(&xdr, res->new_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->new_fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->old_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->old_fattr, res->server); out: return status; } @@ -4934,13 +4923,11 @@ static int nfs4_xdr_dec_link(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_link * Note order: OP_LINK leaves the directory as the current * filehandle. */ - if (decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->dir_attr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -4969,13 +4956,11 @@ static int nfs4_xdr_dec_create(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_cr goto out; if ((status = decode_getfh(&xdr, res->fh)) != 0) goto out; - if (decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->fattr, res->server) != 0) goto out; if ((status = decode_restorefh(&xdr)) != 0) goto out; - decode_getfattr(&xdr, res->dir_fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_fattr, res->server); out: return status; } @@ -5007,8 +4992,7 @@ static int nfs4_xdr_dec_getattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_g status = decode_putfh(&xdr); if (status) goto out; - status = decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + status = decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5115,8 +5099,7 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos * an ESTALE error. Shouldn't be a problem, * though, since fattr->valid will remain unset. */ - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5148,13 +5131,11 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, __be32 *p, struct nfs_openr goto out; if (decode_getfh(&xdr, &res->fh) != 0) goto out; - if (decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)) != 0) + if (decode_getfattr(&xdr, res->f_attr, res->server) != 0) goto out; if (decode_restorefh(&xdr) != 0) goto out; - decode_getfattr(&xdr, res->dir_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->dir_attr, res->server); out: return status; } @@ -5202,8 +5183,7 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_open(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->f_attr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->f_attr, res->server); out: return status; } @@ -5230,8 +5210,7 @@ static int nfs4_xdr_dec_setattr(struct rpc_rqst *rqstp, __be32 *p, struct nfs_se status = decode_setattr(&xdr); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5418,8 +5397,7 @@ static int nfs4_xdr_dec_write(struct rpc_rqst *rqstp, __be32 *p, struct nfs_writ status = decode_write(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); if (!status) status = res->count; out: @@ -5448,8 +5426,7 @@ static int nfs4_xdr_dec_commit(struct rpc_rqst *rqstp, __be32 *p, struct nfs_wri status = decode_commit(&xdr, res); if (status) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5615,8 +5592,7 @@ static int nfs4_xdr_dec_delegreturn(struct rpc_rqst *rqstp, __be32 *p, struct nf status = decode_delegreturn(&xdr); if (status != 0) goto out; - decode_getfattr(&xdr, res->fattr, res->server, - !RPC_IS_ASYNC(rqstp->rq_task)); + decode_getfattr(&xdr, res->fattr, res->server); out: return status; } @@ -5644,8 +5620,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req, __be32 *p, goto out; xdr_enter_page(&xdr, PAGE_SIZE); status = decode_getfattr(&xdr, &res->fs_locations->fattr, - res->fs_locations->server, - !RPC_IS_ASYNC(req->rq_task)); + res->fs_locations->server); out: return status; } diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 508f8cf..6ce7a08 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -369,6 +369,10 @@ extern struct nfs_fattr *nfs_alloc_fattr(void); static inline void nfs_free_fattr(const struct nfs_fattr *fattr) { + if (!fattr) + return; + kfree(fattr->user_name); + kfree(fattr->group_name); kfree(fattr); } diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index fc46192..fc58d5d 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -27,10 +27,17 @@ static inline int nfs_fsid_equal(const struct nfs_fsid *a, const struct nfs_fsid return a->major == b->major && a->minor == b->minor; } +struct nfs_name { + size_t len; + char buf[]; +}; + struct nfs_fattr { unsigned int valid; /* which fields are valid */ umode_t mode; __u32 nlink; + struct nfs_name *user_name; + struct nfs_name *group_name; __u32 uid; __u32 gid; dev_t rdev; ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-21 11:25 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-02 8:01 Empty core dumps on NFSv4 mounts Arnaud Giersch 2010-07-02 13:03 ` Trond Myklebust 2010-07-05 13:28 ` [PATCH/RFC 2.6.35-rc4] NFSv4: Don't do idmapper upcalls during XDR decode Arnaud Giersch 2010-08-12 10:15 ` [PATCH 2.6.35] " Arnaud Giersch 2010-09-21 11:17 ` [PATCH resend] " Arnaud Giersch
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).