All of lore.kernel.org
 help / color / mirror / Atom feed
* [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
@ 2012-10-15  2:12 Chen Gang
  2012-10-15  4:27   ` Myklebust, Trond
  0 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2012-10-15  2:12 UTC (permalink / raw)
  To: Jeff Layton, Trond.Myklebust, linux-nfs, linux-kernel

Hello Trond Myklebust, Jeff Layton:

1) Root Cause:
   A) begin truncate to smaller, after async read finish starting.
   B) async read done come, after truncate operation change inode size.
   C) in nfs_inode_attrs_need_update, nfs_size_need_update return true.
      i)   the bigger size is the original old size of client itself.
      ii)  the smaller size is the current true size.
      iii) nfs_inode_attrs_need_update not consider this situation.

2) Fix nfs_size_need_update:
   A) delete it:
      i)   it is for performance, not necessary (not for correctness).
      ii)  if it was necessary, it should use "!=" instead of '>'.
      iii) it is the simplest way to fix this bug (maybe not best way).
   B) consider this situation in it:
      i)   it is the best way.
      ii)  it is a little complex (need think of)
      iii) sorry for I do not know how to fix it (at least now).
   C) not touch it:
      i)   correct another place (such as nfs_update_inode)
      ii)  it is a bad idea (at least, I think it is)
      iii) we need keep the source code as clearer as possible.

3) Test Result:
   A) it is one client and one server separately, under 3.6-rc5 x86_32.
   B) use one process (fsx-linux) test (only one user mode thread).
   C) only use read, truncate, llseek, fstat operation for one file.

   Before delete nfs_size_need_update, it causes issue.
   After delete nfs_size_need_update, it is ok.


User Mode Log:
-------------------------------------------------------------------------
<<<test_start>>>
tag=nfsx-linux stime=1350202875
cmdline="export VERSION SOCKET_TYPE; TCbin=$LTPROOT/testcases/bin fsx.sh"
contacts=""
analysis=exit
<<<test_output>>>

Test Options:
 VERSION: 2
 RHOST: dhcp122.asianux.net
 ITERATIONS: 50000
 SOCKET_TYPE: udp
 NFS_TYPE: nfs
Setting up remote machine: dhcp122.asianux.net
Mounting NFS filesystem dhcp122.asianux.net:/tmp/fsx1447.testdir on
/opt/ltp/testcases/bin/fsx1447 with options '-o proto=udp,vers=2 '
fsx-linux -N 50000 /opt/ltp/testcases/bin/fsx1447/testfile Starting
truncating to largest ever: 0x13e76
truncating to largest ever: 0x2e52c
truncating to largest ever: 0x3c2c2
truncating to largest ever: 0x3f15f
truncating to largest ever: 0x3fcb9
truncating to largest ever: 0x3fe96
truncating to largest ever: 0x3ff9d
Size error: expected 0x36ef9 stat 0x3bbca seek 0x36ef9
LOG DUMP (5652 total operations):

...

5636: 1350203089.781599 READ     0x143b6 thru 0x21ccb (0xd916 bytes)
5637: 1350203090.028214 READ     0x2a629 thru 0x2d0a1 (0x2a79 bytes)
5638: 1350203090.072029 TRUNCATE DOWN   from 0x2d0a2 to 0x1bb35
5639: 1350203090.087401 READ     0x11a05 thru 0x1bb34 (0xa130 bytes)
5640: 1350203090.223985 READ     0x508c thru 0xa9da (0x594f bytes)
5641: 1350203090.245717 TRUNCATE DOWN   from 0x1bb35 to 0x8830
5642: 1350203090.353502 READ     0x548f thru 0x882f (0x33a1 bytes)
5643: 1350203090.366596 READ     0x5802 thru 0x882f (0x302e bytes)
5644: 1350203090.366629 TRUNCATE UP     from 0x8830 to 0x20011
5645: 1350203090.379476 TRUNCATE DOWN   from 0x20011 to 0x134f4
5646: 1350203090.396234 READ     0x124a0 thru 0x134f3 (0x1054 bytes)
5647: 1350203090.401805 READ     0x880b thru 0x1189d (0x9093 bytes)
5648: 1350203090.532050 READ     0x134c7 thru 0x134f3 (0x2d bytes)
5649: 1350203090.532057 TRUNCATE UP     from 0x134f4 to 0x3bbca
5650: 1350203090.546373 READ     0x2944c thru 0x2c1d6 (0x2d8b bytes)
5651: 1350203090.561228 READ     0xdbe1 thru 0x16260 (0x8680 bytes)
5652: 1350203090.751937 TRUNCATE DOWN   from 0x3bbca to 0x36ef9
Correct content saved for comparison
(maybe hexdump "/opt/ltp/testcases/bin/fsx1447/testfile" vs
"/opt/ltp/testcases/bin/fsx1447/testfile.fsxgood")
fsx-linux -N 50000 /opt/ltp/testcases/bin/fsx1447/testfile Finished
Cleaning up testcase
Unmounting /opt/ltp/testcases/bin/fsx1447
Test Failed: Errors have resulted from this test
incrementing stop
<<<execution_status>>>
initiation_status="ok"
duration=218 termination_type=exited termination_id=1 corefile=no
cutime=43 cstime=82
<<<test_end>>>


-------------------------------------------------------------------------


Kernel Mode Log: (using printk which I add)
-------------------------------------------------------------------------
Time:          My Mark:   Task ptr:  comments (include function name):
[  280.883701] gchen_tag: f5c30000, nfs_read_done call
nfs_refresh_inode, cur=0x3bbca, new=0x3bbca
[  280.890677] gchen_tag: f5c30000, nfs_read_done call
nfs_refresh_inode, cur=0x3bbca, new=0x3bbca
[  280.897437] gchen_tag: f5c30000, nfs_read_done call
nfs_refresh_inode, cur=0x3bbca, new=0x3bbca
[  280.897441] gchen_tag: f5e48c90, nfs_setattr_update_inode, cur=3bbca,
new=36ef9
[  280.897450] gchen_tag: f5e48c90, nfs_setattr
[  280.897462] gchen_tag: hit, f5c30000, nfs_refresh_inode_locked,
cur=36ef9, new=3bbca
[  280.897469] gchen_tag: f5c30000, nfs_update_inode, change size,
cur=36ef9, new=3bbca
[  280.898129] gchen_tag: f5e48c90, nfs_update_inode, change size,
cur=3bbca, new=36ef9
[  280.977915] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=2000, pages=55
[  281.019879] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=a000, pages=53
[  281.070325] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=e000, pages=45
[  281.087103] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=12000, pages=43
[  281.129061] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=16000, pages=41
[  281.163012] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=18000, pages=37
[  281.213481] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=1c000, pages=33
[  281.255727] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=22000, pages=31
[  281.306177] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=24000, pages=27
[  281.356888] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=2e000, pages=21
[  281.398859] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=36ef9, new=36000, pages=19
[  281.585491] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=104d8, new=2000, pages=17
[  281.644207] gchen_tag: f5c30000, nfs_update_inode, not change size,
cur=104d8, new=10000, pages=15
-------------------------------------------------------------------------


Thanks.

--
Chen Gang

Asianux Corporation


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-15  2:12 [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size Chen Gang
@ 2012-10-15  4:27   ` Myklebust, Trond
  0 siblings, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-15  4:27 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2456 bytes --]

On Mon, 2012-10-15 at 10:12 +0800, Chen Gang wrote:
> Hello Trond Myklebust, Jeff Layton:
> 
> 1) Root Cause:
>    A) begin truncate to smaller, after async read finish starting.
>    B) async read done come, after truncate operation change inode size.
>    C) in nfs_inode_attrs_need_update, nfs_size_need_update return true.
>       i)   the bigger size is the original old size of client itself.
>       ii)  the smaller size is the current true size.
>       iii) nfs_inode_attrs_need_update not consider this situation.
> 
> 2) Fix nfs_size_need_update:
>    A) delete it:
>       i)   it is for performance, not necessary (not for correctness).
>       ii)  if it was necessary, it should use "!=" instead of '>'.
>       iii) it is the simplest way to fix this bug (maybe not best way).
>    B) consider this situation in it:
>       i)   it is the best way.
>       ii)  it is a little complex (need think of)
>       iii) sorry for I do not know how to fix it (at least now).
>    C) not touch it:
>       i)   correct another place (such as nfs_update_inode)
>       ii)  it is a bad idea (at least, I think it is)
>       iii) we need keep the source code as clearer as possible.
> 
> 3) Test Result:
>    A) it is one client and one server separately, under 3.6-rc5 x86_32.
>    B) use one process (fsx-linux) test (only one user mode thread).
>    C) only use read, truncate, llseek, fstat operation for one file.
> 
>    Before delete nfs_size_need_update, it causes issue.
>    After delete nfs_size_need_update, it is ok.

nfs_size_need_update is not about performance. It is a heuristic that is
entirely about ensuring correctness when faced with the fact that most
Linux filesystems are utterly incapable of reporting with modifications
that occur within < 1 second intervals because their mtime/ctime is
limited to 1 second resolutions.

Now, what are the conditions of your test setup? The above bug report is
meaningless unless it includes a description of what is being exported
by the server (including a proper listing of the contents
of /etc/exports and /proc/mounts). It should also include a description
of the NFS client mount options (see /proc/mounts on the client).

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
@ 2012-10-15  4:27   ` Myklebust, Trond
  0 siblings, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-15  4:27 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

T24gTW9uLCAyMDEyLTEwLTE1IGF0IDEwOjEyICswODAwLCBDaGVuIEdhbmcgd3JvdGU6DQo+IEhl
bGxvIFRyb25kIE15a2xlYnVzdCwgSmVmZiBMYXl0b246DQo+IA0KPiAxKSBSb290IENhdXNlOg0K
PiAgICBBKSBiZWdpbiB0cnVuY2F0ZSB0byBzbWFsbGVyLCBhZnRlciBhc3luYyByZWFkIGZpbmlz
aCBzdGFydGluZy4NCj4gICAgQikgYXN5bmMgcmVhZCBkb25lIGNvbWUsIGFmdGVyIHRydW5jYXRl
IG9wZXJhdGlvbiBjaGFuZ2UgaW5vZGUgc2l6ZS4NCj4gICAgQykgaW4gbmZzX2lub2RlX2F0dHJz
X25lZWRfdXBkYXRlLCBuZnNfc2l6ZV9uZWVkX3VwZGF0ZSByZXR1cm4gdHJ1ZS4NCj4gICAgICAg
aSkgICB0aGUgYmlnZ2VyIHNpemUgaXMgdGhlIG9yaWdpbmFsIG9sZCBzaXplIG9mIGNsaWVudCBp
dHNlbGYuDQo+ICAgICAgIGlpKSAgdGhlIHNtYWxsZXIgc2l6ZSBpcyB0aGUgY3VycmVudCB0cnVl
IHNpemUuDQo+ICAgICAgIGlpaSkgbmZzX2lub2RlX2F0dHJzX25lZWRfdXBkYXRlIG5vdCBjb25z
aWRlciB0aGlzIHNpdHVhdGlvbi4NCj4gDQo+IDIpIEZpeCBuZnNfc2l6ZV9uZWVkX3VwZGF0ZToN
Cj4gICAgQSkgZGVsZXRlIGl0Og0KPiAgICAgICBpKSAgIGl0IGlzIGZvciBwZXJmb3JtYW5jZSwg
bm90IG5lY2Vzc2FyeSAobm90IGZvciBjb3JyZWN0bmVzcykuDQo+ICAgICAgIGlpKSAgaWYgaXQg
d2FzIG5lY2Vzc2FyeSwgaXQgc2hvdWxkIHVzZSAiIT0iIGluc3RlYWQgb2YgJz4nLg0KPiAgICAg
ICBpaWkpIGl0IGlzIHRoZSBzaW1wbGVzdCB3YXkgdG8gZml4IHRoaXMgYnVnIChtYXliZSBub3Qg
YmVzdCB3YXkpLg0KPiAgICBCKSBjb25zaWRlciB0aGlzIHNpdHVhdGlvbiBpbiBpdDoNCj4gICAg
ICAgaSkgICBpdCBpcyB0aGUgYmVzdCB3YXkuDQo+ICAgICAgIGlpKSAgaXQgaXMgYSBsaXR0bGUg
Y29tcGxleCAobmVlZCB0aGluayBvZikNCj4gICAgICAgaWlpKSBzb3JyeSBmb3IgSSBkbyBub3Qg
a25vdyBob3cgdG8gZml4IGl0IChhdCBsZWFzdCBub3cpLg0KPiAgICBDKSBub3QgdG91Y2ggaXQ6
DQo+ICAgICAgIGkpICAgY29ycmVjdCBhbm90aGVyIHBsYWNlIChzdWNoIGFzIG5mc191cGRhdGVf
aW5vZGUpDQo+ICAgICAgIGlpKSAgaXQgaXMgYSBiYWQgaWRlYSAoYXQgbGVhc3QsIEkgdGhpbmsg
aXQgaXMpDQo+ICAgICAgIGlpaSkgd2UgbmVlZCBrZWVwIHRoZSBzb3VyY2UgY29kZSBhcyBjbGVh
cmVyIGFzIHBvc3NpYmxlLg0KPiANCj4gMykgVGVzdCBSZXN1bHQ6DQo+ICAgIEEpIGl0IGlzIG9u
ZSBjbGllbnQgYW5kIG9uZSBzZXJ2ZXIgc2VwYXJhdGVseSwgdW5kZXIgMy42LXJjNSB4ODZfMzIu
DQo+ICAgIEIpIHVzZSBvbmUgcHJvY2VzcyAoZnN4LWxpbnV4KSB0ZXN0IChvbmx5IG9uZSB1c2Vy
IG1vZGUgdGhyZWFkKS4NCj4gICAgQykgb25seSB1c2UgcmVhZCwgdHJ1bmNhdGUsIGxsc2Vlaywg
ZnN0YXQgb3BlcmF0aW9uIGZvciBvbmUgZmlsZS4NCj4gDQo+ICAgIEJlZm9yZSBkZWxldGUgbmZz
X3NpemVfbmVlZF91cGRhdGUsIGl0IGNhdXNlcyBpc3N1ZS4NCj4gICAgQWZ0ZXIgZGVsZXRlIG5m
c19zaXplX25lZWRfdXBkYXRlLCBpdCBpcyBvay4NCg0KbmZzX3NpemVfbmVlZF91cGRhdGUgaXMg
bm90IGFib3V0IHBlcmZvcm1hbmNlLiBJdCBpcyBhIGhldXJpc3RpYyB0aGF0IGlzDQplbnRpcmVs
eSBhYm91dCBlbnN1cmluZyBjb3JyZWN0bmVzcyB3aGVuIGZhY2VkIHdpdGggdGhlIGZhY3QgdGhh
dCBtb3N0DQpMaW51eCBmaWxlc3lzdGVtcyBhcmUgdXR0ZXJseSBpbmNhcGFibGUgb2YgcmVwb3J0
aW5nIHdpdGggbW9kaWZpY2F0aW9ucw0KdGhhdCBvY2N1ciB3aXRoaW4gPCAxIHNlY29uZCBpbnRl
cnZhbHMgYmVjYXVzZSB0aGVpciBtdGltZS9jdGltZSBpcw0KbGltaXRlZCB0byAxIHNlY29uZCBy
ZXNvbHV0aW9ucy4NCg0KTm93LCB3aGF0IGFyZSB0aGUgY29uZGl0aW9ucyBvZiB5b3VyIHRlc3Qg
c2V0dXA/IFRoZSBhYm92ZSBidWcgcmVwb3J0IGlzDQptZWFuaW5nbGVzcyB1bmxlc3MgaXQgaW5j
bHVkZXMgYSBkZXNjcmlwdGlvbiBvZiB3aGF0IGlzIGJlaW5nIGV4cG9ydGVkDQpieSB0aGUgc2Vy
dmVyIChpbmNsdWRpbmcgYSBwcm9wZXIgbGlzdGluZyBvZiB0aGUgY29udGVudHMNCm9mIC9ldGMv
ZXhwb3J0cyBhbmQgL3Byb2MvbW91bnRzKS4gSXQgc2hvdWxkIGFsc28gaW5jbHVkZSBhIGRlc2Ny
aXB0aW9uDQpvZiB0aGUgTkZTIGNsaWVudCBtb3VudCBvcHRpb25zIChzZWUgL3Byb2MvbW91bnRz
IG9uIHRoZSBjbGllbnQpLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVu
dCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5u
ZXRhcHAuY29tDQo=

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-15  4:27   ` Myklebust, Trond
  (?)
@ 2012-10-15  4:52   ` Chen Gang
  2012-10-15  5:39     ` Chen Gang
  2012-10-15 12:32       ` Myklebust, Trond
  -1 siblings, 2 replies; 15+ messages in thread
From: Chen Gang @ 2012-10-15  4:52 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Jeff Layton, linux-nfs, linux-kernel

于 2012年10月15日 12:27, Myklebust, Trond 写道:
> nfs_size_need_update is not about performance. It is a heuristic that is
> entirely about ensuring correctness when faced with the fact that most
> Linux filesystems are utterly incapable of reporting with modifications
> that occur within < 1 second intervals because their mtime/ctime is
> limited to 1 second resolutions.
> 

if truly it was for correctness, why not use "!=" instead of '>' ?

> Now, what are the conditions of your test setup? The above bug report is
> meaningless unless it includes a description of what is being exported
> by the server (including a proper listing of the contents
> of /etc/exports and /proc/mounts). It should also include a description
> of the NFS client mount options (see /proc/mounts on the client).

they are below, if you need additional information, please tell me again.

for server:
(nfsx-linux using rsh auto exportfs in cmd line, not in /etc/exports)
--------------------------------------------------------------------
root@dhcp122:~# exportfs
/tmp/fsx18251.testdir
		<world>
/tmp          	<world>
root@dhcp122:~#
root@dhcp122:~# cat /etc/exports
# /etc/exports: the access control list for filesystems which may be
exported
#		to NFS clients.  See exports(5).
#
# Example for NFSv2 and NFSv3:
# /srv/homes       hostname1(rw,sync,no_subtree_check)
hostname2(ro,sync,no_subtree_check)
#
# Example for NFSv4:
# /srv/nfs4        gss/krb5i(rw,sync,fsid=0,crossmnt,no_subtree_check)
# /srv/nfs4/homes  gss/krb5i(rw,sync,no_subtree_check)
#
/tmp *(rw,sync,no_root_squash,no_subtree_check)
root@dhcp122:~#
root@dhcp122:~# cat /proc/mounts
rootfs / rootfs rw 0 0
sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0
proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
udev /dev devtmpfs rw,relatime,size=1229628k,nr_inodes=189901,mode=755 0 0
devpts /dev/pts devpts
rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
tmpfs /run tmpfs rw,nosuid,relatime,size=516280k,mode=755 0 0
/dev/disk/by-uuid/e843c57e-98ce-44cc-8e02-6d8e8d8a01b6 / ext4
rw,relatime,errors=remount-ro,data=ordered 0 0
cgroup /sys/fs/cgroup tmpfs rw,relatime,mode=755 0 0
cgroup /sys/fs/cgroup/cpuset cgroup rw,relatime,cpuset 0 0
cgroup /sys/fs/cgroup/cpu cgroup rw,relatime,cpu 0 0
cgroup /sys/fs/cgroup/cpuacct cgroup rw,relatime,cpuacct 0 0
cgroup /sys/fs/cgroup/devices cgroup rw,relatime,devices 0 0
cgroup /sys/fs/cgroup/freezer cgroup rw,relatime,freezer 0 0
cgroup /sys/fs/cgroup/blkio cgroup rw,relatime,blkio 0 0
cgroup /sys/fs/cgroup/perf_event cgroup rw,relatime,perf_event 0 0
none /sys/fs/fuse/connections fusectl rw,relatime 0 0
none /sys/kernel/debug debugfs rw,relatime 0 0
none /sys/kernel/security securityfs rw,relatime 0 0
none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0
none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0
rpc_pipefs /run/rpc_pipefs rpc_pipefs rw,relatime 0 0
nfsd /proc/fs/nfsd nfsd rw,relatime 0 0
root@dhcp122:~#
-----------------------------------------------------------------------

for client:
-----------------------------------------------------------------------

root@dhcp159:/opt/ltp/testscripts# cat /proc/mounts
rootfs / rootfs rw 0 0
sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0
proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
udev /dev devtmpfs rw,relatime,size=1103700k,nr_inodes=190392,mode=755 0 0
devpts /dev/pts devpts
rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
tmpfs /run tmpfs rw,nosuid,relatime,size=465908k,mode=755 0 0
/dev/disk/by-uuid/418ec1f1-ed9d-4cae-9336-6c742accf538 / ext4
rw,relatime,errors=remount-ro,data=ordered 0 0
cgroup /sys/fs/cgroup tmpfs rw,relatime,mode=755 0 0
cgroup /sys/fs/cgroup/cpuset cgroup rw,relatime,cpuset 0 0
cgroup /sys/fs/cgroup/cpu cgroup rw,relatime,cpu 0 0
cgroup /sys/fs/cgroup/cpuacct cgroup rw,relatime,cpuacct 0 0
cgroup /sys/fs/cgroup/devices cgroup rw,relatime,devices 0 0
cgroup /sys/fs/cgroup/freezer cgroup rw,relatime,freezer 0 0
cgroup /sys/fs/cgroup/blkio cgroup rw,relatime,blkio 0 0
cgroup /sys/fs/cgroup/perf_event cgroup rw,relatime,perf_event 0 0
none /sys/fs/fuse/connections fusectl rw,relatime 0 0
none /sys/kernel/debug debugfs rw,relatime 0 0
none /sys/kernel/security securityfs rw,relatime 0 0
none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0
none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0
rpc_pipefs /run/rpc_pipefs rpc_pipefs rw,relatime 0 0
nfsd /proc/fs/nfsd nfsd rw,relatime 0 0
/dev/sda1 /mnt/sda1 ext3
rw,relatime,errors=continue,user_xattr,acl,barrier=1,data=ordered 0 0
dhcp122.asianux.net:/tmp/fsx18251.testdir/
/opt/ltp/testcases/bin/fsx18251 nfs
rw,relatime,vers=2,rsize=8192,wsize=8192,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,mountaddr=10.1.0.139,mountvers=1,mountport=39973,mountproto=udp,local_lock=none,addr=10.1.0.139
0 0
root@dhcp159:/opt/ltp/testscripts#


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-15  4:52   ` Chen Gang
@ 2012-10-15  5:39     ` Chen Gang
  2012-10-15 12:32       ` Myklebust, Trond
  1 sibling, 0 replies; 15+ messages in thread
From: Chen Gang @ 2012-10-15  5:39 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Jeff Layton, linux-nfs, linux-kernel

于 2012年10月15日 12:52, Chen Gang 写道:
>> Now, what are the conditions of your test setup? The above bug report is
>> > meaningless unless it includes a description of what is being exported
>> > by the server (including a proper listing of the contents
>> > of /etc/exports and /proc/mounts). It should also include a description
>> > of the NFS client mount options (see /proc/mounts on the client).

for exportfs command line is:
  rsh -n $RHOST "/usr/sbin/exportfs -i -o no_root_squash,rw *:$TESTDIR"

  $RHOST is dhcp122.asianux.net (10.1.0.122, not need input password)
  $TESTDIR just the mount dir.


> they are below, if you need additional information, please tell me again.
> 


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-15  4:52   ` Chen Gang
@ 2012-10-15 12:32       ` Myklebust, Trond
  2012-10-15 12:32       ` Myklebust, Trond
  1 sibling, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-15 12:32 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5657 bytes --]

On Mon, 2012-10-15 at 12:52 +0800, Chen Gang wrote:
> 于 2012年10月15日 12:27, Myklebust, Trond 写道:
> > nfs_size_need_update is not about performance. It is a heuristic that is
> > entirely about ensuring correctness when faced with the fact that most
> > Linux filesystems are utterly incapable of reporting with modifications
> > that occur within < 1 second intervals because their mtime/ctime is
> > limited to 1 second resolutions.
> > 
> 
> if truly it was for correctness, why not use "!=" instead of '>' ?

RPC is not ordered. The fact that we get one RPC reply before another
does not mean that the server sent them in that order.

This is doubly true when you use UDP as the transport protocol.

> > Now, what are the conditions of your test setup? The above bug report is
> > meaningless unless it includes a description of what is being exported
> > by the server (including a proper listing of the contents
> > of /etc/exports and /proc/mounts). It should also include a description
> > of the NFS client mount options (see /proc/mounts on the client).
> 
> they are below, if you need additional information, please tell me again.
> 
> for server:
> (nfsx-linux using rsh auto exportfs in cmd line, not in /etc/exports)
> --------------------------------------------------------------------
> root@dhcp122:~# exportfs
> /tmp/fsx18251.testdir
> 		<world>
> /tmp          	<world>
> root@dhcp122:~#
> root@dhcp122:~# cat /etc/exports
> # /etc/exports: the access control list for filesystems which may be
> exported
> #		to NFS clients.  See exports(5).
> #
> # Example for NFSv2 and NFSv3:
> # /srv/homes       hostname1(rw,sync,no_subtree_check)
> hostname2(ro,sync,no_subtree_check)
> #
> # Example for NFSv4:
> # /srv/nfs4        gss/krb5i(rw,sync,fsid=0,crossmnt,no_subtree_check)
> # /srv/nfs4/homes  gss/krb5i(rw,sync,no_subtree_check)
> #
> /tmp *(rw,sync,no_root_squash,no_subtree_check)
> root@dhcp122:~#
> root@dhcp122:~# cat /proc/mounts
> rootfs / rootfs rw 0 0
> sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0
> proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
> udev /dev devtmpfs rw,relatime,size=1229628k,nr_inodes=189901,mode=755 0 0
> devpts /dev/pts devpts
> rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
> tmpfs /run tmpfs rw,nosuid,relatime,size=516280k,mode=755 0 0
> /dev/disk/by-uuid/e843c57e-98ce-44cc-8e02-6d8e8d8a01b6 / ext4

OK. So the export is part of this ext4 filesystem?

> rw,relatime,errors=remount-ro,data=ordered 0 0
> cgroup /sys/fs/cgroup tmpfs rw,relatime,mode=755 0 0
> cgroup /sys/fs/cgroup/cpuset cgroup rw,relatime,cpuset 0 0
> cgroup /sys/fs/cgroup/cpu cgroup rw,relatime,cpu 0 0
> cgroup /sys/fs/cgroup/cpuacct cgroup rw,relatime,cpuacct 0 0
> cgroup /sys/fs/cgroup/devices cgroup rw,relatime,devices 0 0
> cgroup /sys/fs/cgroup/freezer cgroup rw,relatime,freezer 0 0
> cgroup /sys/fs/cgroup/blkio cgroup rw,relatime,blkio 0 0
> cgroup /sys/fs/cgroup/perf_event cgroup rw,relatime,perf_event 0 0
> none /sys/fs/fuse/connections fusectl rw,relatime 0 0
> none /sys/kernel/debug debugfs rw,relatime 0 0
> none /sys/kernel/security securityfs rw,relatime 0 0
> none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0
> none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0
> rpc_pipefs /run/rpc_pipefs rpc_pipefs rw,relatime 0 0
> nfsd /proc/fs/nfsd nfsd rw,relatime 0 0
> root@dhcp122:~#
> -----------------------------------------------------------------------
> 
> for client:
> -----------------------------------------------------------------------
> 
> root@dhcp159:/opt/ltp/testscripts# cat /proc/mounts
> rootfs / rootfs rw 0 0
> sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0
> proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
> udev /dev devtmpfs rw,relatime,size=1103700k,nr_inodes=190392,mode=755 0 0
> devpts /dev/pts devpts
> rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
> tmpfs /run tmpfs rw,nosuid,relatime,size=465908k,mode=755 0 0
> /dev/disk/by-uuid/418ec1f1-ed9d-4cae-9336-6c742accf538 / ext4
> rw,relatime,errors=remount-ro,data=ordered 0 0
> cgroup /sys/fs/cgroup tmpfs rw,relatime,mode=755 0 0
> cgroup /sys/fs/cgroup/cpuset cgroup rw,relatime,cpuset 0 0
> cgroup /sys/fs/cgroup/cpu cgroup rw,relatime,cpu 0 0
> cgroup /sys/fs/cgroup/cpuacct cgroup rw,relatime,cpuacct 0 0
> cgroup /sys/fs/cgroup/devices cgroup rw,relatime,devices 0 0
> cgroup /sys/fs/cgroup/freezer cgroup rw,relatime,freezer 0 0
> cgroup /sys/fs/cgroup/blkio cgroup rw,relatime,blkio 0 0
> cgroup /sys/fs/cgroup/perf_event cgroup rw,relatime,perf_event 0 0
> none /sys/fs/fuse/connections fusectl rw,relatime 0 0
> none /sys/kernel/debug debugfs rw,relatime 0 0
> none /sys/kernel/security securityfs rw,relatime 0 0
> none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0
> none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0
> rpc_pipefs /run/rpc_pipefs rpc_pipefs rw,relatime 0 0
> nfsd /proc/fs/nfsd nfsd rw,relatime 0 0
> /dev/sda1 /mnt/sda1 ext3
> rw,relatime,errors=continue,user_xattr,acl,barrier=1,data=ordered 0 0
> dhcp122.asianux.net:/tmp/fsx18251.testdir/
> /opt/ltp/testcases/bin/fsx18251 nfs
> rw,relatime,vers=2,rsize=8192,wsize=8192,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,mountaddr=10.1.0.139,mountvers=1,mountport=39973,mountproto=udp,local_lock=none,addr=10.1.0.139
> 0 0
> root@dhcp159:/opt/ltp/testscripts#

...and you are using NFSv2 with UDP?

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
@ 2012-10-15 12:32       ` Myklebust, Trond
  0 siblings, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-15 12:32 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

T24gTW9uLCAyMDEyLTEwLTE1IGF0IDEyOjUyICswODAwLCBDaGVuIEdhbmcgd3JvdGU6DQo+IOS6
jiAyMDEy5bm0MTDmnIgxNeaXpSAxMjoyNywgTXlrbGVidXN0LCBUcm9uZCDlhpnpgZM6DQo+ID4g
bmZzX3NpemVfbmVlZF91cGRhdGUgaXMgbm90IGFib3V0IHBlcmZvcm1hbmNlLiBJdCBpcyBhIGhl
dXJpc3RpYyB0aGF0IGlzDQo+ID4gZW50aXJlbHkgYWJvdXQgZW5zdXJpbmcgY29ycmVjdG5lc3Mg
d2hlbiBmYWNlZCB3aXRoIHRoZSBmYWN0IHRoYXQgbW9zdA0KPiA+IExpbnV4IGZpbGVzeXN0ZW1z
IGFyZSB1dHRlcmx5IGluY2FwYWJsZSBvZiByZXBvcnRpbmcgd2l0aCBtb2RpZmljYXRpb25zDQo+
ID4gdGhhdCBvY2N1ciB3aXRoaW4gPCAxIHNlY29uZCBpbnRlcnZhbHMgYmVjYXVzZSB0aGVpciBt
dGltZS9jdGltZSBpcw0KPiA+IGxpbWl0ZWQgdG8gMSBzZWNvbmQgcmVzb2x1dGlvbnMuDQo+ID4g
DQo+IA0KPiBpZiB0cnVseSBpdCB3YXMgZm9yIGNvcnJlY3RuZXNzLCB3aHkgbm90IHVzZSAiIT0i
IGluc3RlYWQgb2YgJz4nID8NCg0KUlBDIGlzIG5vdCBvcmRlcmVkLiBUaGUgZmFjdCB0aGF0IHdl
IGdldCBvbmUgUlBDIHJlcGx5IGJlZm9yZSBhbm90aGVyDQpkb2VzIG5vdCBtZWFuIHRoYXQgdGhl
IHNlcnZlciBzZW50IHRoZW0gaW4gdGhhdCBvcmRlci4NCg0KVGhpcyBpcyBkb3VibHkgdHJ1ZSB3
aGVuIHlvdSB1c2UgVURQIGFzIHRoZSB0cmFuc3BvcnQgcHJvdG9jb2wuDQoNCj4gPiBOb3csIHdo
YXQgYXJlIHRoZSBjb25kaXRpb25zIG9mIHlvdXIgdGVzdCBzZXR1cD8gVGhlIGFib3ZlIGJ1ZyBy
ZXBvcnQgaXMNCj4gPiBtZWFuaW5nbGVzcyB1bmxlc3MgaXQgaW5jbHVkZXMgYSBkZXNjcmlwdGlv
biBvZiB3aGF0IGlzIGJlaW5nIGV4cG9ydGVkDQo+ID4gYnkgdGhlIHNlcnZlciAoaW5jbHVkaW5n
IGEgcHJvcGVyIGxpc3Rpbmcgb2YgdGhlIGNvbnRlbnRzDQo+ID4gb2YgL2V0Yy9leHBvcnRzIGFu
ZCAvcHJvYy9tb3VudHMpLiBJdCBzaG91bGQgYWxzbyBpbmNsdWRlIGEgZGVzY3JpcHRpb24NCj4g
PiBvZiB0aGUgTkZTIGNsaWVudCBtb3VudCBvcHRpb25zIChzZWUgL3Byb2MvbW91bnRzIG9uIHRo
ZSBjbGllbnQpLg0KPiANCj4gdGhleSBhcmUgYmVsb3csIGlmIHlvdSBuZWVkIGFkZGl0aW9uYWwg
aW5mb3JtYXRpb24sIHBsZWFzZSB0ZWxsIG1lIGFnYWluLg0KPiANCj4gZm9yIHNlcnZlcjoNCj4g
KG5mc3gtbGludXggdXNpbmcgcnNoIGF1dG8gZXhwb3J0ZnMgaW4gY21kIGxpbmUsIG5vdCBpbiAv
ZXRjL2V4cG9ydHMpDQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+IHJvb3RAZGhjcDEyMjp+IyBleHBvcnRmcw0K
PiAvdG1wL2ZzeDE4MjUxLnRlc3RkaXINCj4gCQk8d29ybGQ+DQo+IC90bXAgICAgICAgICAgCTx3
b3JsZD4NCj4gcm9vdEBkaGNwMTIyOn4jDQo+IHJvb3RAZGhjcDEyMjp+IyBjYXQgL2V0Yy9leHBv
cnRzDQo+ICMgL2V0Yy9leHBvcnRzOiB0aGUgYWNjZXNzIGNvbnRyb2wgbGlzdCBmb3IgZmlsZXN5
c3RlbXMgd2hpY2ggbWF5IGJlDQo+IGV4cG9ydGVkDQo+ICMJCXRvIE5GUyBjbGllbnRzLiAgU2Vl
IGV4cG9ydHMoNSkuDQo+ICMNCj4gIyBFeGFtcGxlIGZvciBORlN2MiBhbmQgTkZTdjM6DQo+ICMg
L3Nydi9ob21lcyAgICAgICBob3N0bmFtZTEocncsc3luYyxub19zdWJ0cmVlX2NoZWNrKQ0KPiBo
b3N0bmFtZTIocm8sc3luYyxub19zdWJ0cmVlX2NoZWNrKQ0KPiAjDQo+ICMgRXhhbXBsZSBmb3Ig
TkZTdjQ6DQo+ICMgL3Nydi9uZnM0ICAgICAgICBnc3Mva3JiNWkocncsc3luYyxmc2lkPTAsY3Jv
c3NtbnQsbm9fc3VidHJlZV9jaGVjaykNCj4gIyAvc3J2L25mczQvaG9tZXMgIGdzcy9rcmI1aShy
dyxzeW5jLG5vX3N1YnRyZWVfY2hlY2spDQo+ICMNCj4gL3RtcCAqKHJ3LHN5bmMsbm9fcm9vdF9z
cXVhc2gsbm9fc3VidHJlZV9jaGVjaykNCj4gcm9vdEBkaGNwMTIyOn4jDQo+IHJvb3RAZGhjcDEy
Mjp+IyBjYXQgL3Byb2MvbW91bnRzDQo+IHJvb3RmcyAvIHJvb3RmcyBydyAwIDANCj4gc3lzZnMg
L3N5cyBzeXNmcyBydyxub3N1aWQsbm9kZXYsbm9leGVjLHJlbGF0aW1lIDAgMA0KPiBwcm9jIC9w
cm9jIHByb2Mgcncsbm9zdWlkLG5vZGV2LG5vZXhlYyxyZWxhdGltZSAwIDANCj4gdWRldiAvZGV2
IGRldnRtcGZzIHJ3LHJlbGF0aW1lLHNpemU9MTIyOTYyOGssbnJfaW5vZGVzPTE4OTkwMSxtb2Rl
PTc1NSAwIDANCj4gZGV2cHRzIC9kZXYvcHRzIGRldnB0cw0KPiBydyxub3N1aWQsbm9leGVjLHJl
bGF0aW1lLGdpZD01LG1vZGU9NjIwLHB0bXhtb2RlPTAwMCAwIDANCj4gdG1wZnMgL3J1biB0bXBm
cyBydyxub3N1aWQscmVsYXRpbWUsc2l6ZT01MTYyODBrLG1vZGU9NzU1IDAgMA0KPiAvZGV2L2Rp
c2svYnktdXVpZC9lODQzYzU3ZS05OGNlLTQ0Y2MtOGUwMi02ZDhlOGQ4YTAxYjYgLyBleHQ0DQoN
Ck9LLiBTbyB0aGUgZXhwb3J0IGlzIHBhcnQgb2YgdGhpcyBleHQ0IGZpbGVzeXN0ZW0/DQoNCj4g
cncscmVsYXRpbWUsZXJyb3JzPXJlbW91bnQtcm8sZGF0YT1vcmRlcmVkIDAgMA0KPiBjZ3JvdXAg
L3N5cy9mcy9jZ3JvdXAgdG1wZnMgcncscmVsYXRpbWUsbW9kZT03NTUgMCAwDQo+IGNncm91cCAv
c3lzL2ZzL2Nncm91cC9jcHVzZXQgY2dyb3VwIHJ3LHJlbGF0aW1lLGNwdXNldCAwIDANCj4gY2dy
b3VwIC9zeXMvZnMvY2dyb3VwL2NwdSBjZ3JvdXAgcncscmVsYXRpbWUsY3B1IDAgMA0KPiBjZ3Jv
dXAgL3N5cy9mcy9jZ3JvdXAvY3B1YWNjdCBjZ3JvdXAgcncscmVsYXRpbWUsY3B1YWNjdCAwIDAN
Cj4gY2dyb3VwIC9zeXMvZnMvY2dyb3VwL2RldmljZXMgY2dyb3VwIHJ3LHJlbGF0aW1lLGRldmlj
ZXMgMCAwDQo+IGNncm91cCAvc3lzL2ZzL2Nncm91cC9mcmVlemVyIGNncm91cCBydyxyZWxhdGlt
ZSxmcmVlemVyIDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAvYmxraW8gY2dyb3VwIHJ3LHJl
bGF0aW1lLGJsa2lvIDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAvcGVyZl9ldmVudCBjZ3Jv
dXAgcncscmVsYXRpbWUscGVyZl9ldmVudCAwIDANCj4gbm9uZSAvc3lzL2ZzL2Z1c2UvY29ubmVj
dGlvbnMgZnVzZWN0bCBydyxyZWxhdGltZSAwIDANCj4gbm9uZSAvc3lzL2tlcm5lbC9kZWJ1ZyBk
ZWJ1Z2ZzIHJ3LHJlbGF0aW1lIDAgMA0KPiBub25lIC9zeXMva2VybmVsL3NlY3VyaXR5IHNlY3Vy
aXR5ZnMgcncscmVsYXRpbWUgMCAwDQo+IG5vbmUgL3J1bi9sb2NrIHRtcGZzIHJ3LG5vc3VpZCxu
b2Rldixub2V4ZWMscmVsYXRpbWUsc2l6ZT01MTIwayAwIDANCj4gbm9uZSAvcnVuL3NobSB0bXBm
cyBydyxub3N1aWQsbm9kZXYscmVsYXRpbWUgMCAwDQo+IHJwY19waXBlZnMgL3J1bi9ycGNfcGlw
ZWZzIHJwY19waXBlZnMgcncscmVsYXRpbWUgMCAwDQo+IG5mc2QgL3Byb2MvZnMvbmZzZCBuZnNk
IHJ3LHJlbGF0aW1lIDAgMA0KPiByb290QGRoY3AxMjI6fiMNCj4gLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4g
DQo+IGZvciBjbGllbnQ6DQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+IA0KPiByb290QGRoY3AxNTk6L29w
dC9sdHAvdGVzdHNjcmlwdHMjIGNhdCAvcHJvYy9tb3VudHMNCj4gcm9vdGZzIC8gcm9vdGZzIHJ3
IDAgMA0KPiBzeXNmcyAvc3lzIHN5c2ZzIHJ3LG5vc3VpZCxub2Rldixub2V4ZWMscmVsYXRpbWUg
MCAwDQo+IHByb2MgL3Byb2MgcHJvYyBydyxub3N1aWQsbm9kZXYsbm9leGVjLHJlbGF0aW1lIDAg
MA0KPiB1ZGV2IC9kZXYgZGV2dG1wZnMgcncscmVsYXRpbWUsc2l6ZT0xMTAzNzAwayxucl9pbm9k
ZXM9MTkwMzkyLG1vZGU9NzU1IDAgMA0KPiBkZXZwdHMgL2Rldi9wdHMgZGV2cHRzDQo+IHJ3LG5v
c3VpZCxub2V4ZWMscmVsYXRpbWUsZ2lkPTUsbW9kZT02MjAscHRteG1vZGU9MDAwIDAgMA0KPiB0
bXBmcyAvcnVuIHRtcGZzIHJ3LG5vc3VpZCxyZWxhdGltZSxzaXplPTQ2NTkwOGssbW9kZT03NTUg
MCAwDQo+IC9kZXYvZGlzay9ieS11dWlkLzQxOGVjMWYxLWVkOWQtNGNhZS05MzM2LTZjNzQyYWNj
ZjUzOCAvIGV4dDQNCj4gcncscmVsYXRpbWUsZXJyb3JzPXJlbW91bnQtcm8sZGF0YT1vcmRlcmVk
IDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAgdG1wZnMgcncscmVsYXRpbWUsbW9kZT03NTUg
MCAwDQo+IGNncm91cCAvc3lzL2ZzL2Nncm91cC9jcHVzZXQgY2dyb3VwIHJ3LHJlbGF0aW1lLGNw
dXNldCAwIDANCj4gY2dyb3VwIC9zeXMvZnMvY2dyb3VwL2NwdSBjZ3JvdXAgcncscmVsYXRpbWUs
Y3B1IDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAvY3B1YWNjdCBjZ3JvdXAgcncscmVsYXRp
bWUsY3B1YWNjdCAwIDANCj4gY2dyb3VwIC9zeXMvZnMvY2dyb3VwL2RldmljZXMgY2dyb3VwIHJ3
LHJlbGF0aW1lLGRldmljZXMgMCAwDQo+IGNncm91cCAvc3lzL2ZzL2Nncm91cC9mcmVlemVyIGNn
cm91cCBydyxyZWxhdGltZSxmcmVlemVyIDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAvYmxr
aW8gY2dyb3VwIHJ3LHJlbGF0aW1lLGJsa2lvIDAgMA0KPiBjZ3JvdXAgL3N5cy9mcy9jZ3JvdXAv
cGVyZl9ldmVudCBjZ3JvdXAgcncscmVsYXRpbWUscGVyZl9ldmVudCAwIDANCj4gbm9uZSAvc3lz
L2ZzL2Z1c2UvY29ubmVjdGlvbnMgZnVzZWN0bCBydyxyZWxhdGltZSAwIDANCj4gbm9uZSAvc3lz
L2tlcm5lbC9kZWJ1ZyBkZWJ1Z2ZzIHJ3LHJlbGF0aW1lIDAgMA0KPiBub25lIC9zeXMva2VybmVs
L3NlY3VyaXR5IHNlY3VyaXR5ZnMgcncscmVsYXRpbWUgMCAwDQo+IG5vbmUgL3J1bi9sb2NrIHRt
cGZzIHJ3LG5vc3VpZCxub2Rldixub2V4ZWMscmVsYXRpbWUsc2l6ZT01MTIwayAwIDANCj4gbm9u
ZSAvcnVuL3NobSB0bXBmcyBydyxub3N1aWQsbm9kZXYscmVsYXRpbWUgMCAwDQo+IHJwY19waXBl
ZnMgL3J1bi9ycGNfcGlwZWZzIHJwY19waXBlZnMgcncscmVsYXRpbWUgMCAwDQo+IG5mc2QgL3By
b2MvZnMvbmZzZCBuZnNkIHJ3LHJlbGF0aW1lIDAgMA0KPiAvZGV2L3NkYTEgL21udC9zZGExIGV4
dDMNCj4gcncscmVsYXRpbWUsZXJyb3JzPWNvbnRpbnVlLHVzZXJfeGF0dHIsYWNsLGJhcnJpZXI9
MSxkYXRhPW9yZGVyZWQgMCAwDQo+IGRoY3AxMjIuYXNpYW51eC5uZXQ6L3RtcC9mc3gxODI1MS50
ZXN0ZGlyLw0KPiAvb3B0L2x0cC90ZXN0Y2FzZXMvYmluL2ZzeDE4MjUxIG5mcw0KPiBydyxyZWxh
dGltZSx2ZXJzPTIscnNpemU9ODE5Mix3c2l6ZT04MTkyLG5hbWxlbj0yNTUsaGFyZCxwcm90bz11
ZHAsdGltZW89MTEscmV0cmFucz0zLHNlYz1zeXMsbW91bnRhZGRyPTEwLjEuMC4xMzksbW91bnR2
ZXJzPTEsbW91bnRwb3J0PTM5OTczLG1vdW50cHJvdG89dWRwLGxvY2FsX2xvY2s9bm9uZSxhZGRy
PTEwLjEuMC4xMzkNCj4gMCAwDQo+IHJvb3RAZGhjcDE1OTovb3B0L2x0cC90ZXN0c2NyaXB0cyMN
Cg0KLi4uYW5kIHlvdSBhcmUgdXNpbmcgTkZTdjIgd2l0aCBVRFA/DQoNCi0tIA0KVHJvbmQgTXlr
bGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWts
ZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-15 12:32       ` Myklebust, Trond
  (?)
@ 2012-10-16  1:37       ` Chen Gang
  2012-10-16  2:51           ` Myklebust, Trond
  -1 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2012-10-16  1:37 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Jeff Layton, linux-nfs, linux-kernel

于 2012年10月15日 20:32, Myklebust, Trond 写道:
> RPC is not ordered. The fact that we get one RPC reply before another
> does not mean that the server sent them in that order.
> 
> This is doubly true when you use UDP as the transport protocol.

1) is it means: nfs_inode_attrs_need_update need not consider async
read_done situation ?

2) for correctness, I do not think "nfs_size_to_loff_t(fattr->size) >
i_size_read(inode)" in nfs_size_need_update is enough. (at least need
use "!=" instead of '>'), do you think so ?


3) another reference:

  A) for an old kernel version (such as 2.6.27-rc9), no such issue
(because it did not have nfs_size_need_update).

  B) the test tools which I use is from the LTP (Linux Test Project),
they use both udp and tcp to test both the nfsv2 and nfsv3.

  C) truly LTP has its limitations: "for stress test, LTP let nfs client
and server under the same machine, which will cause kernel stable
issue", but for net test, LTP use different machine (I got our issue
from LTP net test).


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16  1:37       ` Chen Gang
@ 2012-10-16  2:51           ` Myklebust, Trond
  0 siblings, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-16  2:51 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2084 bytes --]

On Tue, 2012-10-16 at 09:37 +0800, Chen Gang wrote:
> 于 2012年10月15日 20:32, Myklebust, Trond 写道:
> > RPC is not ordered. The fact that we get one RPC reply before another
> > does not mean that the server sent them in that order.
> > 
> > This is doubly true when you use UDP as the transport protocol.
> 
> 1) is it means: nfs_inode_attrs_need_update need not consider async
> read_done situation ?

I don't understand what you mean. This is mainly about the asynchronous
write situation...

> 2) for correctness, I do not think "nfs_size_to_loff_t(fattr->size) >
> i_size_read(inode)" in nfs_size_need_update is enough. (at least need
> use "!=" instead of '>'), do you think so ?

No... If I did, I would have changed this 15 years ago when I was
writing that code. Nothing here is new... 2.6.27-rc9 has the exact same
heuristics.
It boils down to the rule that if you want to ensure that data is not
_lost_, then you have to ensure that the cached file size is not less
than the true file size.

> 3) another reference:
> 
>   A) for an old kernel version (such as 2.6.27-rc9), no such issue
> (because it did not have nfs_size_need_update).
> 
>   B) the test tools which I use is from the LTP (Linux Test Project),
> they use both udp and tcp to test both the nfsv2 and nfsv3.

So what combinations are failing?

>   C) truly LTP has its limitations: "for stress test, LTP let nfs client
> and server under the same machine, which will cause kernel stable
> issue", but for net test, LTP use different machine (I got our issue
> from LTP net test).

Running the client and server on the same machine is likely to deadlock
due to memory pressure issues. The client needs to be able to _increase_
memory pressure on the server in order to reduce its own pressure. That
doesn't work well when client == server.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
@ 2012-10-16  2:51           ` Myklebust, Trond
  0 siblings, 0 replies; 15+ messages in thread
From: Myklebust, Trond @ 2012-10-16  2:51 UTC (permalink / raw)
  To: Chen Gang; +Cc: Jeff Layton, linux-nfs, linux-kernel

T24gVHVlLCAyMDEyLTEwLTE2IGF0IDA5OjM3ICswODAwLCBDaGVuIEdhbmcgd3JvdGU6DQo+IOS6
jiAyMDEy5bm0MTDmnIgxNeaXpSAyMDozMiwgTXlrbGVidXN0LCBUcm9uZCDlhpnpgZM6DQo+ID4g
UlBDIGlzIG5vdCBvcmRlcmVkLiBUaGUgZmFjdCB0aGF0IHdlIGdldCBvbmUgUlBDIHJlcGx5IGJl
Zm9yZSBhbm90aGVyDQo+ID4gZG9lcyBub3QgbWVhbiB0aGF0IHRoZSBzZXJ2ZXIgc2VudCB0aGVt
IGluIHRoYXQgb3JkZXIuDQo+ID4gDQo+ID4gVGhpcyBpcyBkb3VibHkgdHJ1ZSB3aGVuIHlvdSB1
c2UgVURQIGFzIHRoZSB0cmFuc3BvcnQgcHJvdG9jb2wuDQo+IA0KPiAxKSBpcyBpdCBtZWFuczog
bmZzX2lub2RlX2F0dHJzX25lZWRfdXBkYXRlIG5lZWQgbm90IGNvbnNpZGVyIGFzeW5jDQo+IHJl
YWRfZG9uZSBzaXR1YXRpb24gPw0KDQpJIGRvbid0IHVuZGVyc3RhbmQgd2hhdCB5b3UgbWVhbi4g
VGhpcyBpcyBtYWlubHkgYWJvdXQgdGhlIGFzeW5jaHJvbm91cw0Kd3JpdGUgc2l0dWF0aW9uLi4u
DQoNCj4gMikgZm9yIGNvcnJlY3RuZXNzLCBJIGRvIG5vdCB0aGluayAibmZzX3NpemVfdG9fbG9m
Zl90KGZhdHRyLT5zaXplKSA+DQo+IGlfc2l6ZV9yZWFkKGlub2RlKSIgaW4gbmZzX3NpemVfbmVl
ZF91cGRhdGUgaXMgZW5vdWdoLiAoYXQgbGVhc3QgbmVlZA0KPiB1c2UgIiE9IiBpbnN0ZWFkIG9m
ICc+JyksIGRvIHlvdSB0aGluayBzbyA/DQoNCk5vLi4uIElmIEkgZGlkLCBJIHdvdWxkIGhhdmUg
Y2hhbmdlZCB0aGlzIDE1IHllYXJzIGFnbyB3aGVuIEkgd2FzDQp3cml0aW5nIHRoYXQgY29kZS4g
Tm90aGluZyBoZXJlIGlzIG5ldy4uLiAyLjYuMjctcmM5IGhhcyB0aGUgZXhhY3Qgc2FtZQ0KaGV1
cmlzdGljcy4NCkl0IGJvaWxzIGRvd24gdG8gdGhlIHJ1bGUgdGhhdCBpZiB5b3Ugd2FudCB0byBl
bnN1cmUgdGhhdCBkYXRhIGlzIG5vdA0KX2xvc3RfLCB0aGVuIHlvdSBoYXZlIHRvIGVuc3VyZSB0
aGF0IHRoZSBjYWNoZWQgZmlsZSBzaXplIGlzIG5vdCBsZXNzDQp0aGFuIHRoZSB0cnVlIGZpbGUg
c2l6ZS4NCg0KPiAzKSBhbm90aGVyIHJlZmVyZW5jZToNCj4gDQo+ICAgQSkgZm9yIGFuIG9sZCBr
ZXJuZWwgdmVyc2lvbiAoc3VjaCBhcyAyLjYuMjctcmM5KSwgbm8gc3VjaCBpc3N1ZQ0KPiAoYmVj
YXVzZSBpdCBkaWQgbm90IGhhdmUgbmZzX3NpemVfbmVlZF91cGRhdGUpLg0KPiANCj4gICBCKSB0
aGUgdGVzdCB0b29scyB3aGljaCBJIHVzZSBpcyBmcm9tIHRoZSBMVFAgKExpbnV4IFRlc3QgUHJv
amVjdCksDQo+IHRoZXkgdXNlIGJvdGggdWRwIGFuZCB0Y3AgdG8gdGVzdCBib3RoIHRoZSBuZnN2
MiBhbmQgbmZzdjMuDQoNClNvIHdoYXQgY29tYmluYXRpb25zIGFyZSBmYWlsaW5nPw0KDQo+ICAg
QykgdHJ1bHkgTFRQIGhhcyBpdHMgbGltaXRhdGlvbnM6ICJmb3Igc3RyZXNzIHRlc3QsIExUUCBs
ZXQgbmZzIGNsaWVudA0KPiBhbmQgc2VydmVyIHVuZGVyIHRoZSBzYW1lIG1hY2hpbmUsIHdoaWNo
IHdpbGwgY2F1c2Uga2VybmVsIHN0YWJsZQ0KPiBpc3N1ZSIsIGJ1dCBmb3IgbmV0IHRlc3QsIExU
UCB1c2UgZGlmZmVyZW50IG1hY2hpbmUgKEkgZ290IG91ciBpc3N1ZQ0KPiBmcm9tIExUUCBuZXQg
dGVzdCkuDQoNClJ1bm5pbmcgdGhlIGNsaWVudCBhbmQgc2VydmVyIG9uIHRoZSBzYW1lIG1hY2hp
bmUgaXMgbGlrZWx5IHRvIGRlYWRsb2NrDQpkdWUgdG8gbWVtb3J5IHByZXNzdXJlIGlzc3Vlcy4g
VGhlIGNsaWVudCBuZWVkcyB0byBiZSBhYmxlIHRvIF9pbmNyZWFzZV8NCm1lbW9yeSBwcmVzc3Vy
ZSBvbiB0aGUgc2VydmVyIGluIG9yZGVyIHRvIHJlZHVjZSBpdHMgb3duIHByZXNzdXJlLiBUaGF0
DQpkb2Vzbid0IHdvcmsgd2VsbCB3aGVuIGNsaWVudCA9PSBzZXJ2ZXIuDQoNCi0tIA0KVHJvbmQg
TXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5N
eWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16  2:51           ` Myklebust, Trond
  (?)
@ 2012-10-16  4:13           ` Chen Gang
  2012-10-16 10:33             ` Jeff Layton
  -1 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2012-10-16  4:13 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Jeff Layton, linux-nfs, linux-kernel

于 2012年10月16日 10:51, Myklebust, Trond 写道:

>>
>> 1) is it means: nfs_inode_attrs_need_update need not consider async
>> read_done situation ?
> 
> I don't understand what you mean. This is mainly about the asynchronous
> write situation...

for async read done, it will call nfs_readpage_result -> nfs_read_done
-> nfs_refresh_inode -> nfs_refresh_inode_locked ->
nfs_inode_attrs_need_update -> nfs_size_need_update.

we need consider the situation that "async read_done also call
nfs_size_need_update with an old useless larger file size".

you means, it need not consider async read (only consider async write is
enough), is it correct ?

> 
> No... If I did, I would have changed this 15 years ago when I was
> writing that code. Nothing here is new... 2.6.27-rc9 has the exact same
> heuristics.

1) I have read the relative source code of 2.6.27-rc9, it is truly no
nfs_size_need_update function.

2) I have test the 2.6.27-rc9, it truly pass the LTP test of udp+nfsv2.

3) I got the 2.6.27-rc9 source code by this way (please check)
   A) get source code from (git clone)
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
   B) git archive v2.6.27-rc9 | tar -xf - -C ../2.6.27-rc9/


> It boils down to the rule that if you want to ensure that data is not
> _lost_, then you have to ensure that the cached file size is not less
> than the true file size.
> 

1) you means: in some condition, the cached file size can be bigger than
the true file size ?  can you give some example (which no negative
effect for correctness) ?

2) What I feel:
   A) I am not quite familiar with nfs (so truly need your information);
   B) I think it is truly a bug, but maybe nfs_size_need_update is not
the root cause (so need nfs maintainers' audit)
   C) if nfs_size_need_update is truly not the root cause, I shall
continue analysing it, after get enough information from nfs maintainers.


>>   B) the test tools which I use is from the LTP (Linux Test Project),
>> they use both udp and tcp to test both the nfsv2 and nfsv3.
> 
> So what combinations are failing?

for udp + nfsv2 failing (I am not test udp + nfsv3)

> 
>>   C) truly LTP has its limitations: "for stress test, LTP let nfs client
>> and server under the same machine, which will cause kernel stable
>> issue", but for net test, LTP use different machine (I got our issue
>> from LTP net test).
> 
> Running the client and server on the same machine is likely to deadlock
> due to memory pressure issues. The client needs to be able to _increase_
> memory pressure on the server in order to reduce its own pressure. That
> doesn't work well when client == server.
> 

truly got confirmation from Jeff Layton, 1-2 months ago;
also thank you for giving confirmation too.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16  4:13           ` Chen Gang
@ 2012-10-16 10:33             ` Jeff Layton
  2012-10-16 11:44               ` Chen Gang
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-10-16 10:33 UTC (permalink / raw)
  To: Chen Gang; +Cc: Myklebust, Trond, linux-nfs, linux-kernel

On Tue, 16 Oct 2012 12:13:38 +0800
Chen Gang <gang.chen@asianux.com> wrote:

> 于 2012年10月16日 10:51, Myklebust, Trond 写道:
> 
> >>
> >> 1) is it means: nfs_inode_attrs_need_update need not consider async
> >> read_done situation ?
> > 
> > I don't understand what you mean. This is mainly about the asynchronous
> > write situation...
> 
> for async read done, it will call nfs_readpage_result -> nfs_read_done
> -> nfs_refresh_inode -> nfs_refresh_inode_locked ->
> nfs_inode_attrs_need_update -> nfs_size_need_update.
> 
> we need consider the situation that "async read_done also call
> nfs_size_need_update with an old useless larger file size".
> 
> you means, it need not consider async read (only consider async write is
> enough), is it correct ?
> 
> > 
> > No... If I did, I would have changed this 15 years ago when I was
> > writing that code. Nothing here is new... 2.6.27-rc9 has the exact same
> > heuristics.
> 
> 1) I have read the relative source code of 2.6.27-rc9, it is truly no
> nfs_size_need_update function.
> 
> 2) I have test the 2.6.27-rc9, it truly pass the LTP test of udp+nfsv2.
> 
> 3) I got the 2.6.27-rc9 source code by this way (please check)
>    A) get source code from (git clone)
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
>    B) git archive v2.6.27-rc9 | tar -xf - -C ../2.6.27-rc9/
> 
> 
> > It boils down to the rule that if you want to ensure that data is not
> > _lost_, then you have to ensure that the cached file size is not less
> > than the true file size.
> > 
> 
> 1) you means: in some condition, the cached file size can be bigger than
> the true file size ?  can you give some example (which no negative
> effect for correctness) ?
> 
> 2) What I feel:
>    A) I am not quite familiar with nfs (so truly need your information);
>    B) I think it is truly a bug, but maybe nfs_size_need_update is not
> the root cause (so need nfs maintainers' audit)
>    C) if nfs_size_need_update is truly not the root cause, I shall
> continue analysing it, after get enough information from nfs maintainers.
> 
> 
> >>   B) the test tools which I use is from the LTP (Linux Test Project),
> >> they use both udp and tcp to test both the nfsv2 and nfsv3.
> > 
> > So what combinations are failing?
> 
> for udp + nfsv2 failing (I am not test udp + nfsv3)
> 


The problem is a little more fundamental than that. The attr cache
handling logic is some of the trickiest code to deal with in the NFS
client.

In any situation where we get back attributes, we have to decide
whether they are valid or stale. It's always possible for replies or
their handling to be reordered such that an older set of attributes
is processed after a newer set.

Unfortunately, the v2/v3 protocols do not have great support for
helping the client detect this situation, so we do the best we can with
what we do have. Unfortunately when things are changing very quickly we
can still get it wrong, especially with v2/3. [1]

In any case, the logic to determine this is in
nfs_inode_attrs_need_update(). Looking at the size is sort of the "last
resort" after we look at the timestamps and gencount.

The problem with doing what you suggest is that if we get it wrong, the
consequences are worse than the file appearing to be bigger than it is.
It means that written data may be silently lost.

======

[1]: v4 has a change attribute so it's slightly simpler there when the
server supports it. Unrelated Q for Trond: should we be checking the v4
change_attr in nfs_inode_attrs_need_update too? 

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16 10:33             ` Jeff Layton
@ 2012-10-16 11:44               ` Chen Gang
  2012-10-16 12:13                 ` Jeff Layton
  0 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2012-10-16 11:44 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Myklebust, Trond, linux-nfs, linux-kernel


于 2012年10月16日 18:33, Jeff Layton 写道:
> In any situation where we get back attributes, we have to decide
> whether they are valid or stale. It's always possible for replies or
> their handling to be reordered such that an older set of attributes
> is processed after a newer set.
> 
> Unfortunately, the v2/v3 protocols do not have great support for
> helping the client detect this situation, so we do the best we can with
> what we do have. Unfortunately when things are changing very quickly we
> can still get it wrong, especially with v2/3. [1]
> 
> In any case, the logic to determine this is in
> nfs_inode_attrs_need_update(). Looking at the size is sort of the "last
> resort" after we look at the timestamps and gencount.
> 

I agree with you (what I understood originally is just like what you
said above).

thank for your confirmation.

> The problem with doing what you suggest is that if we get it wrong, the
> consequences are worse than the file appearing to be bigger than it is.
> It means that written data may be silently lost.
> 

1) I understand why my suggestion is not quite suitable. I agree you.

2) but, are we truly no ways to solve this issue ?  (I do not think so).

3) I think an executable way (but maybe not a good way) is :

   A) for each client, check each task id of the client its own (such as
rpc task xid), so can know the order of tasks of the client its own.

   B) maybe also need another some synchronization code, but I think it
does not have much negative effect with performance.


> ======
> 
> [1]: v4 has a change attribute so it's slightly simpler there when the
> server supports it. Unrelated Q for Trond: should we be checking the v4
> change_attr in nfs_inode_attrs_need_update too? 

sorry for I am truly not quite familiar with nfs, I also think it is not
quite relative with current issue, so I have to skip it (although I
think these contents are valulable for Trond)

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16 11:44               ` Chen Gang
@ 2012-10-16 12:13                 ` Jeff Layton
  2012-10-17  1:37                   ` Chen Gang
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-10-16 12:13 UTC (permalink / raw)
  To: Chen Gang; +Cc: Myklebust, Trond, linux-nfs, linux-kernel

On Tue, 16 Oct 2012 19:44:38 +0800
Chen Gang <gang.chen@asianux.com> wrote:

> 
> 于 2012年10月16日 18:33, Jeff Layton 写道:
> > In any situation where we get back attributes, we have to decide
> > whether they are valid or stale. It's always possible for replies or
> > their handling to be reordered such that an older set of attributes
> > is processed after a newer set.
> > 
> > Unfortunately, the v2/v3 protocols do not have great support for
> > helping the client detect this situation, so we do the best we can with
> > what we do have. Unfortunately when things are changing very quickly we
> > can still get it wrong, especially with v2/3. [1]
> > 
> > In any case, the logic to determine this is in
> > nfs_inode_attrs_need_update(). Looking at the size is sort of the "last
> > resort" after we look at the timestamps and gencount.
> > 
> 
> I agree with you (what I understood originally is just like what you
> said above).
> 
> thank for your confirmation.
> 
> > The problem with doing what you suggest is that if we get it wrong, the
> > consequences are worse than the file appearing to be bigger than it is.
> > It means that written data may be silently lost.
> > 
> 
> 1) I understand why my suggestion is not quite suitable. I agree you.
> 
> 2) but, are we truly no ways to solve this issue ?  (I do not think so).
> 

Not that I see, but don't let me stop you from trying to find one. ;)

> 3) I think an executable way (but maybe not a good way) is :
> 
>    A) for each client, check each task id of the client its own (such as
> rpc task xid), so can know the order of tasks of the client its own.
> 

We do something like this already. That's what the gencount thing is
all about. It's still possible though to fool that check if two calls
are scheduled closely enough.

Also note that it's not just the reordering of replies that you have to
concern yourself with. The requests themselves can be reordered on the
network. The server is also under no obligation to execute calls in the
order received.

>    B) maybe also need another some synchronization code, but I think it
> does not have much negative effect with performance.
> 

Yeah, serializing things to fix this is probably a non-starter. NFSv2
and UDP transports are basically legacy code at this point, so there's
not a lot of incentive to do anything drastic here.

> 
> > ======
> > 
> > [1]: v4 has a change attribute so it's slightly simpler there when the
> > server supports it. Unrelated Q for Trond: should we be checking the v4
> > change_attr in nfs_inode_attrs_need_update too? 
> 
> sorry for I am truly not quite familiar with nfs, I also think it is not
> quite relative with current issue, so I have to skip it (although I
> think these contents are valulable for Trond)
> 

Correct. That was just an aside question for Trond or someone else who
understands the attribute revalidation code better than I do.

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size
  2012-10-16 12:13                 ` Jeff Layton
@ 2012-10-17  1:37                   ` Chen Gang
  0 siblings, 0 replies; 15+ messages in thread
From: Chen Gang @ 2012-10-17  1:37 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Myklebust, Trond, linux-nfs, linux-kernel

于 2012年10月16日 20:13, Jeff Layton 写道:
>>
>> 2) but, are we truly no ways to solve this issue ?  (I do not think so).
>>
> 
> Not that I see, but don't let me stop you from trying to find one. ;)
> 

we can divide the issue to 2 separate parts:

1) the inconsistent attribute by time delay between client and server:

   A) it is the nfs v2/v3 design issue, the "user" can understand (not
implementation mistake)
   B) we need make the time delay as shorter as we can. (this is the
reason why I call it "performance", although this "word" is still not
quit suitable)
   C) "user" can understand, not mean can bear (such as skipping writing
operation attribute changes)

2) the inconsistent attribute by a client itself:

   A) it is implementation issue, the "user" can not understand (it is
an implementation mistake)
   B) we need solve it (so I call it "correctness" issue).
   C) "user" can not understand, not mean can not bear (such as current
issue which I report)

at last, for maintainer:
   A) for "performance", we need try our best to do;
   B) for "correctness", we need fix it completely;

>> 3) I think an executable way (but maybe not a good way) is :
>>
>>    A) for each client, check each task id of the client its own (such as
>> rpc task xid), so can know the order of tasks of the client its own.
>>
> 
> We do something like this already. That's what the gencount thing is
> all about. It's still possible though to fool that check if two calls
> are scheduled closely enough.
>

1) I think gencount is not equal to sequence number, the sequence number
can mark all relative tasks of one client in order.

2) I also think, it is not quite complex to make a client itself in
consistency. (it is implementation issue, not design issue)


> Also note that it's not just the reordering of replies that you have to
> concern yourself with. The requests themselves can be reordered on the
> network. The server is also under no obligation to execute calls in the
> order received.
> 

1) I agree with you, in nfs_inode_attrs_need_update(), it need consider
this situation (the tasks from server return is not in order).

2) I do not think it can not be accomplished if the tasks of client
itself have sequence number.  (maybe, it would be enough to only judge
which task is later between the 2 tasks by sequence number).


>>    B) maybe also need another some synchronization code, but I think it
>> does not have much negative effect with performance.
>>
> 
> Yeah, serializing things to fix this is probably a non-starter. NFSv2
> and UDP transports are basically legacy code at this point, so there's
> not a lot of incentive to do anything drastic here.
> 

1) I agree with what you said above, but maybe you misunderstand of what
I said for the "item B)"

2) the "item B)" is for the completion of "item A)". when we fix this
issue, maybe have to add additional synchronization code which maybe can
cause negative effect with performance, but I think it is not much
("user" can bear).

At last, I suggest we need think of how to fix this implementation bug
in nfs-client region.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-10-17  1:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-15  2:12 [Bug fix] nfs-client: fix nfs_inode_attrs_need_update for async read_done comes during truncating to smaller size Chen Gang
2012-10-15  4:27 ` Myklebust, Trond
2012-10-15  4:27   ` Myklebust, Trond
2012-10-15  4:52   ` Chen Gang
2012-10-15  5:39     ` Chen Gang
2012-10-15 12:32     ` Myklebust, Trond
2012-10-15 12:32       ` Myklebust, Trond
2012-10-16  1:37       ` Chen Gang
2012-10-16  2:51         ` Myklebust, Trond
2012-10-16  2:51           ` Myklebust, Trond
2012-10-16  4:13           ` Chen Gang
2012-10-16 10:33             ` Jeff Layton
2012-10-16 11:44               ` Chen Gang
2012-10-16 12:13                 ` Jeff Layton
2012-10-17  1:37                   ` Chen Gang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.