2015-07-07 02:12:09

by Kinglong Mee

[permalink] [raw]
Subject: [PATCH 1/3] nfsd: Set lc_size_chg before ops->proc_layoutcommit

After proc_layoutcommit success, i_size_read(inode) always >= new_size.
Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit
failed, nfsd will skip the lc_size_chg, so it's no harm.

Signed-off-by: Kinglong Mee <[email protected]>
---
fs/nfsd/nfs4proc.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 90cfda7..d112c8a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1364,10 +1364,6 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
goto out;
}

- nfserr = ops->proc_layoutcommit(inode, lcp);
- if (nfserr)
- goto out_put_stid;
-
if (new_size > i_size_read(inode)) {
lcp->lc_size_chg = 1;
lcp->lc_newsize = new_size;
@@ -1375,7 +1371,7 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
lcp->lc_size_chg = 0;
}

-out_put_stid:
+ nfserr = ops->proc_layoutcommit(inode, lcp);
nfs4_put_stid(&ls->ls_stid);
out:
return nfserr;
--
2.4.3



2015-07-07 02:13:08

by Kinglong Mee

[permalink] [raw]
Subject: [PATCH 2/3] nfsd: Fix a memory leak in nfsd4_list_rec_dir()

If lookup_one_len() failed, nfsd should free those memory allocated for fname.

Signed-off-by: Kinglong Mee <[email protected]>
---
fs/nfsd/nfs4recover.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index d88ea7b..591bfbd 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -272,6 +272,7 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
.ctx.actor = nfsd4_build_namelist,
.names = LIST_HEAD_INIT(ctx.names)
};
+ struct name_list *entry, *tmp;
int status;

status = nfs4_save_creds(&original_cred);
@@ -286,9 +287,8 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)

status = iterate_dir(nn->rec_file, &ctx.ctx);
mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
- while (!list_empty(&ctx.names)) {
- struct name_list *entry;
- entry = list_entry(ctx.names.next, struct name_list, list);
+
+ list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
if (!status) {
struct dentry *dentry;
dentry = lookup_one_len(entry->name, dir, HEXDIR_LEN-1);
@@ -304,6 +304,12 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
}
mutex_unlock(&d_inode(dir)->i_mutex);
nfs4_reset_creds(original_cred);
+
+ list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
+ dprintk("NFSD: %s. Left entry %s\n", __func__, entry->name);
+ list_del(&entry->list);
+ kfree(entry);
+ }
return status;
}

--
2.4.3


2015-07-07 02:16:42

by Kinglong Mee

[permalink] [raw]
Subject: [PATCH 3/3] nfsd: Drop BUG_ON and don't reply SECLABEL when exports migrated

When setting refer for exports entry,

[ 88.414272] kernel BUG at fs/nfsd/nfs4xdr.c:2249!
[ 88.414828] invalid opcode: 0000 [#1] SMP
[ 88.415368] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache nfsd xfs libcrc32c iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi iosf_mbi ppdev btrfs coretemp crct10dif_pclmul crc32_pclmul crc32c_intel xor ghash_clmulni_intel raid6_pq vmw_balloon parport_pc parport i2c_piix4 shpchp vmw_vmci acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc vmwgfx drm_kms_helper ttm drm mptspi mptscsih serio_raw mptbase e1000 scsi_transport_spi ata_generic pata_acpi [last unloaded: nfsd]
[ 88.417827] CPU: 0 PID: 2116 Comm: nfsd Not tainted 4.0.7-300.fc22.x86_64 #1
[ 88.418448] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/20/2014
[ 88.419093] task: ffff880079146d50 ti: ffff8800785d8000 task.ti: ffff8800785d8000
[ 88.419729] RIP: 0010:[<ffffffffa04b3c10>] [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
[ 88.420376] RSP: 0000:ffff8800785db998 EFLAGS: 00010206
[ 88.421027] RAX: 0000000000000001 RBX: 000000000018091a RCX: ffff88006668b980
[ 88.421676] RDX: 00000000fffef7fc RSI: 0000000000000000 RDI: ffff880078d05000
[ 88.422315] RBP: ffff8800785dbb58 R08: ffff880078d043f8 R09: ffff880078d4a000
[ 88.422968] R10: 0000000000010000 R11: 0000000000000002 R12: 0000000000b0a23a
[ 88.423612] R13: ffff880078d05000 R14: ffff880078683100 R15: ffff88006668b980
[ 88.424295] FS: 0000000000000000(0000) GS:ffff88007c600000(0000) knlGS:0000000000000000
[ 88.424944] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 88.425597] CR2: 00007f40bc370f90 CR3: 0000000035af5000 CR4: 00000000001407f0
[ 88.426285] Stack:
[ 88.426921] ffff8800785dbaa8 ffffffffa049e4af ffff8800785dba08 ffffffff813298f0
[ 88.427585] ffff880078683300 ffff8800769b0de8 0000089d00000001 0000000087f805e0
[ 88.428228] ffff880000000000 ffff880079434a00 0000000000000000 ffff88006668b980
[ 88.428877] Call Trace:
[ 88.429527] [<ffffffffa049e4af>] ? exp_get_by_name+0x7f/0xb0 [nfsd]
[ 88.430168] [<ffffffff813298f0>] ? inode_doinit_with_dentry+0x210/0x6a0
[ 88.430807] [<ffffffff8123833e>] ? d_lookup+0x2e/0x60
[ 88.431449] [<ffffffff81236133>] ? dput+0x33/0x230
[ 88.432097] [<ffffffff8123f214>] ? mntput+0x24/0x40
[ 88.432719] [<ffffffff812272b2>] ? path_put+0x22/0x30
[ 88.433340] [<ffffffffa049ac87>] ? nfsd_cross_mnt+0xb7/0x1c0 [nfsd]
[ 88.433954] [<ffffffffa04b54e0>] nfsd4_encode_dirent+0x1b0/0x3d0 [nfsd]
[ 88.434601] [<ffffffffa04b5330>] ? nfsd4_encode_getattr+0x40/0x40 [nfsd]
[ 88.435172] [<ffffffffa049c991>] nfsd_readdir+0x1c1/0x2a0 [nfsd]
[ 88.435710] [<ffffffffa049a530>] ? nfsd_direct_splice_actor+0x20/0x20 [nfsd]
[ 88.436447] [<ffffffffa04abf30>] nfsd4_encode_readdir+0x120/0x220 [nfsd]
[ 88.437011] [<ffffffffa04b58cd>] nfsd4_encode_operation+0x7d/0x190 [nfsd]
[ 88.437566] [<ffffffffa04aa6dd>] nfsd4_proc_compound+0x24d/0x6f0 [nfsd]
[ 88.438157] [<ffffffffa0496103>] nfsd_dispatch+0xc3/0x220 [nfsd]
[ 88.438680] [<ffffffffa006f0cb>] svc_process_common+0x43b/0x690 [sunrpc]
[ 88.439192] [<ffffffffa0070493>] svc_process+0x103/0x1b0 [sunrpc]
[ 88.439694] [<ffffffffa0495a57>] nfsd+0x117/0x190 [nfsd]
[ 88.440194] [<ffffffffa0495940>] ? nfsd_destroy+0x90/0x90 [nfsd]
[ 88.440697] [<ffffffff810bb728>] kthread+0xd8/0xf0
[ 88.441260] [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
[ 88.441762] [<ffffffff81789e58>] ret_from_fork+0x58/0x90
[ 88.442322] [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
[ 88.442879] Code: 0f 84 93 05 00 00 83 f8 ea c7 85 a0 fe ff ff 00 00 27 30 0f 84 ba fe ff ff 85 c0 0f 85 a5 fe ff ff e9 e3 f9 ff ff 0f 1f 44 00 00 <0f> 0b 66 0f 1f 44 00 00 be 04 00 00 00 4c 89 ef 4c 89 8d 68 fe
[ 88.444052] RIP [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
[ 88.444658] RSP <ffff8800785db998>
[ 88.445232] ---[ end trace 6cb9d0487d94a29f ]---

Signed-off-by: Kinglong Mee <[email protected]>
---
fs/nfsd/nfs4xdr.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5463385..75e0563 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2143,6 +2143,7 @@ nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
#define WORD0_ABSENT_FS_ATTRS (FATTR4_WORD0_FS_LOCATIONS | FATTR4_WORD0_FSID | \
FATTR4_WORD0_RDATTR_ERROR)
#define WORD1_ABSENT_FS_ATTRS FATTR4_WORD1_MOUNTED_ON_FILEID
+#define WORD2_ABSENT_FS_ATTRS 0

#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
static inline __be32
@@ -2171,7 +2172,7 @@ nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
{ return 0; }
#endif

-static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
+static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32 *rdattr_err)
{
/* As per referral draft: */
if (*bmval0 & ~WORD0_ABSENT_FS_ATTRS ||
@@ -2184,6 +2185,7 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
}
*bmval0 &= WORD0_ABSENT_FS_ATTRS;
*bmval1 &= WORD1_ABSENT_FS_ATTRS;
+ *bmval2 &= WORD2_ABSENT_FS_ATTRS;
return 0;
}

@@ -2246,8 +2248,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
BUG_ON(bmval2 & ~nfsd_suppattrs2(minorversion));

if (exp->ex_fslocs.migrated) {
- BUG_ON(bmval[2]);
- status = fattr_handle_absent_fs(&bmval0, &bmval1, &rdattr_err);
+ status = fattr_handle_absent_fs(&bmval0, &bmval1, &bmval2, &rdattr_err);
if (status)
goto out;
}
@@ -2286,8 +2287,8 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
}

#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
- if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) ||
- bmval[0] & FATTR4_WORD0_SUPPORTED_ATTRS) {
+ if ((bmval2 & FATTR4_WORD2_SECURITY_LABEL) ||
+ bmval0 & FATTR4_WORD0_SUPPORTED_ATTRS) {
err = security_inode_getsecctx(d_inode(dentry),
&context, &contextlen);
contextsupport = (err == 0);
--
2.4.3


2015-07-08 18:44:31

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] nfsd: Set lc_size_chg before ops->proc_layoutcommit

On Tue, Jul 07, 2015 at 10:12:03AM +0800, Kinglong Mee wrote:
> After proc_layoutcommit success, i_size_read(inode) always >= new_size.
> Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit
> failed, nfsd will skip the lc_size_chg, so it's no harm.

Looks right to me, though could probably use an ACK from Christoph.

(But looking at the spec I'm confused about how the client is supposed
to use this information.)

--b.

>
> Signed-off-by: Kinglong Mee <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 90cfda7..d112c8a 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1364,10 +1364,6 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
> goto out;
> }
>
> - nfserr = ops->proc_layoutcommit(inode, lcp);
> - if (nfserr)
> - goto out_put_stid;
> -
> if (new_size > i_size_read(inode)) {
> lcp->lc_size_chg = 1;
> lcp->lc_newsize = new_size;
> @@ -1375,7 +1371,7 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
> lcp->lc_size_chg = 0;
> }
>
> -out_put_stid:
> + nfserr = ops->proc_layoutcommit(inode, lcp);
> nfs4_put_stid(&ls->ls_stid);
> out:
> return nfserr;
> --
> 2.4.3

2015-07-08 20:43:45

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/3] nfsd: Fix a memory leak in nfsd4_list_rec_dir()

On Tue, Jul 07, 2015 at 10:13:02AM +0800, Kinglong Mee wrote:
> If lookup_one_len() failed, nfsd should free those memory allocated for fname.

Thanks, applying for 4.3.

--b.

>
> Signed-off-by: Kinglong Mee <[email protected]>
> ---
> fs/nfsd/nfs4recover.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index d88ea7b..591bfbd 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -272,6 +272,7 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
> .ctx.actor = nfsd4_build_namelist,
> .names = LIST_HEAD_INIT(ctx.names)
> };
> + struct name_list *entry, *tmp;
> int status;
>
> status = nfs4_save_creds(&original_cred);
> @@ -286,9 +287,8 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
>
> status = iterate_dir(nn->rec_file, &ctx.ctx);
> mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
> - while (!list_empty(&ctx.names)) {
> - struct name_list *entry;
> - entry = list_entry(ctx.names.next, struct name_list, list);
> +
> + list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
> if (!status) {
> struct dentry *dentry;
> dentry = lookup_one_len(entry->name, dir, HEXDIR_LEN-1);
> @@ -304,6 +304,12 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
> }
> mutex_unlock(&d_inode(dir)->i_mutex);
> nfs4_reset_creds(original_cred);
> +
> + list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
> + dprintk("NFSD: %s. Left entry %s\n", __func__, entry->name);
> + list_del(&entry->list);
> + kfree(entry);
> + }
> return status;
> }
>
> --
> 2.4.3

2015-07-08 20:44:04

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/3] nfsd: Drop BUG_ON and don't reply SECLABEL when exports migrated

Thanks, applying for 4.2 and stable.--b.

On Tue, Jul 07, 2015 at 10:16:37AM +0800, Kinglong Mee wrote:
> When setting refer for exports entry,
>
> [ 88.414272] kernel BUG at fs/nfsd/nfs4xdr.c:2249!
> [ 88.414828] invalid opcode: 0000 [#1] SMP
> [ 88.415368] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache nfsd xfs libcrc32c iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi iosf_mbi ppdev btrfs coretemp crct10dif_pclmul crc32_pclmul crc32c_intel xor ghash_clmulni_intel raid6_pq vmw_balloon parport_pc parport i2c_piix4 shpchp vmw_vmci acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc vmwgfx drm_kms_helper ttm drm mptspi mptscsih serio_raw mptbase e1000 scsi_transport_spi ata_generic pata_acpi [last unloaded: nfsd]
> [ 88.417827] CPU: 0 PID: 2116 Comm: nfsd Not tainted 4.0.7-300.fc22.x86_64 #1
> [ 88.418448] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/20/2014
> [ 88.419093] task: ffff880079146d50 ti: ffff8800785d8000 task.ti: ffff8800785d8000
> [ 88.419729] RIP: 0010:[<ffffffffa04b3c10>] [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
> [ 88.420376] RSP: 0000:ffff8800785db998 EFLAGS: 00010206
> [ 88.421027] RAX: 0000000000000001 RBX: 000000000018091a RCX: ffff88006668b980
> [ 88.421676] RDX: 00000000fffef7fc RSI: 0000000000000000 RDI: ffff880078d05000
> [ 88.422315] RBP: ffff8800785dbb58 R08: ffff880078d043f8 R09: ffff880078d4a000
> [ 88.422968] R10: 0000000000010000 R11: 0000000000000002 R12: 0000000000b0a23a
> [ 88.423612] R13: ffff880078d05000 R14: ffff880078683100 R15: ffff88006668b980
> [ 88.424295] FS: 0000000000000000(0000) GS:ffff88007c600000(0000) knlGS:0000000000000000
> [ 88.424944] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 88.425597] CR2: 00007f40bc370f90 CR3: 0000000035af5000 CR4: 00000000001407f0
> [ 88.426285] Stack:
> [ 88.426921] ffff8800785dbaa8 ffffffffa049e4af ffff8800785dba08 ffffffff813298f0
> [ 88.427585] ffff880078683300 ffff8800769b0de8 0000089d00000001 0000000087f805e0
> [ 88.428228] ffff880000000000 ffff880079434a00 0000000000000000 ffff88006668b980
> [ 88.428877] Call Trace:
> [ 88.429527] [<ffffffffa049e4af>] ? exp_get_by_name+0x7f/0xb0 [nfsd]
> [ 88.430168] [<ffffffff813298f0>] ? inode_doinit_with_dentry+0x210/0x6a0
> [ 88.430807] [<ffffffff8123833e>] ? d_lookup+0x2e/0x60
> [ 88.431449] [<ffffffff81236133>] ? dput+0x33/0x230
> [ 88.432097] [<ffffffff8123f214>] ? mntput+0x24/0x40
> [ 88.432719] [<ffffffff812272b2>] ? path_put+0x22/0x30
> [ 88.433340] [<ffffffffa049ac87>] ? nfsd_cross_mnt+0xb7/0x1c0 [nfsd]
> [ 88.433954] [<ffffffffa04b54e0>] nfsd4_encode_dirent+0x1b0/0x3d0 [nfsd]
> [ 88.434601] [<ffffffffa04b5330>] ? nfsd4_encode_getattr+0x40/0x40 [nfsd]
> [ 88.435172] [<ffffffffa049c991>] nfsd_readdir+0x1c1/0x2a0 [nfsd]
> [ 88.435710] [<ffffffffa049a530>] ? nfsd_direct_splice_actor+0x20/0x20 [nfsd]
> [ 88.436447] [<ffffffffa04abf30>] nfsd4_encode_readdir+0x120/0x220 [nfsd]
> [ 88.437011] [<ffffffffa04b58cd>] nfsd4_encode_operation+0x7d/0x190 [nfsd]
> [ 88.437566] [<ffffffffa04aa6dd>] nfsd4_proc_compound+0x24d/0x6f0 [nfsd]
> [ 88.438157] [<ffffffffa0496103>] nfsd_dispatch+0xc3/0x220 [nfsd]
> [ 88.438680] [<ffffffffa006f0cb>] svc_process_common+0x43b/0x690 [sunrpc]
> [ 88.439192] [<ffffffffa0070493>] svc_process+0x103/0x1b0 [sunrpc]
> [ 88.439694] [<ffffffffa0495a57>] nfsd+0x117/0x190 [nfsd]
> [ 88.440194] [<ffffffffa0495940>] ? nfsd_destroy+0x90/0x90 [nfsd]
> [ 88.440697] [<ffffffff810bb728>] kthread+0xd8/0xf0
> [ 88.441260] [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
> [ 88.441762] [<ffffffff81789e58>] ret_from_fork+0x58/0x90
> [ 88.442322] [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
> [ 88.442879] Code: 0f 84 93 05 00 00 83 f8 ea c7 85 a0 fe ff ff 00 00 27 30 0f 84 ba fe ff ff 85 c0 0f 85 a5 fe ff ff e9 e3 f9 ff ff 0f 1f 44 00 00 <0f> 0b 66 0f 1f 44 00 00 be 04 00 00 00 4c 89 ef 4c 89 8d 68 fe
> [ 88.444052] RIP [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
> [ 88.444658] RSP <ffff8800785db998>
> [ 88.445232] ---[ end trace 6cb9d0487d94a29f ]---
>
> Signed-off-by: Kinglong Mee <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 5463385..75e0563 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2143,6 +2143,7 @@ nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
> #define WORD0_ABSENT_FS_ATTRS (FATTR4_WORD0_FS_LOCATIONS | FATTR4_WORD0_FSID | \
> FATTR4_WORD0_RDATTR_ERROR)
> #define WORD1_ABSENT_FS_ATTRS FATTR4_WORD1_MOUNTED_ON_FILEID
> +#define WORD2_ABSENT_FS_ATTRS 0
>
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> static inline __be32
> @@ -2171,7 +2172,7 @@ nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
> { return 0; }
> #endif
>
> -static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
> +static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32 *rdattr_err)
> {
> /* As per referral draft: */
> if (*bmval0 & ~WORD0_ABSENT_FS_ATTRS ||
> @@ -2184,6 +2185,7 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
> }
> *bmval0 &= WORD0_ABSENT_FS_ATTRS;
> *bmval1 &= WORD1_ABSENT_FS_ATTRS;
> + *bmval2 &= WORD2_ABSENT_FS_ATTRS;
> return 0;
> }
>
> @@ -2246,8 +2248,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> BUG_ON(bmval2 & ~nfsd_suppattrs2(minorversion));
>
> if (exp->ex_fslocs.migrated) {
> - BUG_ON(bmval[2]);
> - status = fattr_handle_absent_fs(&bmval0, &bmval1, &rdattr_err);
> + status = fattr_handle_absent_fs(&bmval0, &bmval1, &bmval2, &rdattr_err);
> if (status)
> goto out;
> }
> @@ -2286,8 +2287,8 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> }
>
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> - if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) ||
> - bmval[0] & FATTR4_WORD0_SUPPORTED_ATTRS) {
> + if ((bmval2 & FATTR4_WORD2_SECURITY_LABEL) ||
> + bmval0 & FATTR4_WORD0_SUPPORTED_ATTRS) {
> err = security_inode_getsecctx(d_inode(dentry),
> &context, &contextlen);
> contextsupport = (err == 0);
> --
> 2.4.3

2015-07-09 08:04:20

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/3] nfsd: Set lc_size_chg before ops->proc_layoutcommit

On Wed, Jul 08, 2015 at 02:44:31PM -0400, J. Bruce Fields wrote:
> On Tue, Jul 07, 2015 at 10:12:03AM +0800, Kinglong Mee wrote:
> > After proc_layoutcommit success, i_size_read(inode) always >= new_size.
> > Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit
> > failed, nfsd will skip the lc_size_chg, so it's no harm.
>
> Looks right to me, though could probably use an ACK from Christoph.
>
> (But looking at the spec I'm confused about how the client is supposed
> to use this information.)

I don;t think its possible to use, as there it is inherently racy.

If the test passes xfstests I'm fine with it.

2015-07-09 09:28:51

by Kinglong Mee

[permalink] [raw]
Subject: Re: [PATCH 1/3] nfsd: Set lc_size_chg before ops->proc_layoutcommit

On 7/9/2015 16:04, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 02:44:31PM -0400, J. Bruce Fields wrote:
>> On Tue, Jul 07, 2015 at 10:12:03AM +0800, Kinglong Mee wrote:
>>> After proc_layoutcommit success, i_size_read(inode) always >= new_size.
>>> Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit
>>> failed, nfsd will skip the lc_size_chg, so it's no harm.
>>
>> Looks right to me, though could probably use an ACK from Christoph.
>>
>> (But looking at the spec I'm confused about how the client is supposed
>> to use this information.)
>
> I don;t think its possible to use, as there it is inherently racy.
>
> If the test passes xfstests I'm fine with it.

nfs client just drop this value directly in decode_layoutcommit(),

if (sizechanged) {
/* throw away new size */
p = xdr_inline_decode(xdr, 8);
if (unlikely(!p))
goto out_overflow;
}

The only problem is after ops->proc_layoutcommit(), new_size will never
larger than inode's size, at most it is equal to.

if (new_size > i_size_read(inode)) {
lcp->lc_size_chg = 1;
lcp->lc_newsize = new_size;
} else {
lcp->lc_size_chg = 0;
}

thanks,
Kinglong Mee

2015-07-09 13:52:46

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] nfsd: Set lc_size_chg before ops->proc_layoutcommit

On Thu, Jul 09, 2015 at 01:04:20AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 02:44:31PM -0400, J. Bruce Fields wrote:
> > On Tue, Jul 07, 2015 at 10:12:03AM +0800, Kinglong Mee wrote:
> > > After proc_layoutcommit success, i_size_read(inode) always >= new_size.
> > > Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit
> > > failed, nfsd will skip the lc_size_chg, so it's no harm.
> >
> > Looks right to me, though could probably use an ACK from Christoph.
> >
> > (But looking at the spec I'm confused about how the client is supposed
> > to use this information.)
>
> I don;t think its possible to use, as there it is inherently racy.
>
> If the test passes xfstests I'm fine with it.

OK, thanks.--b.