On 21 Jul 2016, at 9:20, Trond Myklebust wrote:
On Jul 21, 2016, at 09:05, Benjamin Coddington <bcodding@xxxxxxxxxx>
wrote:
So back to Christoph's point earlier:
On 17 Jul 2016, at 23:48, Christoph Hellwig wrote:
This one breaks xfstests generic/207 on block/scsi layout for me.
The
reason for that is that we need a layoutcommit after writing out all
data for the file for the file size to be updated on the server.
You responded:
On 18 Jul 2016, at 0:32, Trond Myklebust wrote:
I’m not understanding this argument. Why do we care if the file
size is up
to date on the server if we’re not sending an actual GETATTR on
the wire
to retrieve the file size?
I guess the answer might be because we can get it back from the last
LAYOUTCOMMIT.
The patch that I followed up with should now ensure that we do not
mark the attribute cache as up to date if there is a LAYOUTCOMMIT
pending.
IOW: when the pNFS write is done, it is expected to do 2 things:
1) mark the inode for LAYOUTCOMMIT
2) mark the attribute cache as invalid (because we know the change
attribute, mtime, ctime need to be updates)
In the case of blocks pNFS write:
The call to pnfs_set_layoutcommit() in pnfs_ld_write_done() should
take care of (1)
The call to nfs_writeback_update_inode() in nfs4_write_done_cb()
should take care of (2).
Provided that these 2 calls are performed in the above order, then any
call to nfs_getattr() which has not been preceded by a call to
nfs4_proc_layoutcommit() should trigger the call to
__nfs_revalidate_inode().
OK, so maybe things are out of order here.. Thanks - this is helpful.
This test has repeated appending 4k and has this pattern on the wire:
NFS 334 V4 Call LAYOUTGET
NFS 290 V4 Reply (Call In 854) LAYOUTCOMMIT
NFS 294 V4 Call GETATTR FH: 0x4f5528b0
NFS 442 V4 Reply (Call In 858) GETATTR
NFS 374 V4 Call LAYOUTCOMMIT
NFS 314 V4 Reply (Call In 856) LAYOUTGET
NFS 334 V4 Call LAYOUTGET
NFS 290 V4 Reply (Call In 860) LAYOUTCOMMIT
NFS 294 V4 Call GETATTR FH: 0x4f5528b0
NFS 442 V4 Reply (Call In 864) GETATTR
NFS 374 V4 Call LAYOUTCOMMIT
NFS 314 V4 Reply (Call In 862) LAYOUTGET
NFS 334 V4 Call LAYOUTGET
NFS 290 V4 Reply (Call In 866) LAYOUTCOMMIT
NFS 294 V4 Call GETATTR FH: 0x4f5528b0
NFS 442 V4 Reply (Call In 870) GETATTR
NFS 314 V4 Reply (Call In 868) LAYOUTGET
NFS 374 V4 Call LAYOUTCOMMIT
NFS 290 V4 Reply (Call In 874) LAYOUTCOMMIT
NFS 314 V4 Call CLOSE StateID: 0x54d9
NFS 294 V4 Reply (Call In 876) CLOSE
That last LAYOUTCOMMIT and the CLOSE have the size we want. The
previous
GETATTR is 4k short.
When you say “4k short”, do you mean that it differs from the
value returned by the LAYOUTCOMMIT that immediately precedes it? It
looks as if there is a LAYOUTGET immediately following it, so
presumably the writes have not all completed yet.
I meant it is 4k short of the final size returned in the LAYOUTCOMMIT
immediately following. It is the same as the LAYOUTCOMMIT immediately
preceding.
FWIW at this point, here's a better look at the network:
tshark -r /tmp/pcap -T fields -e frame.number -e nfs.fh.hash -e
nfs.opcode -e nfs.fattr4.size 'frame.number > 860'
861 53,22,50
862 0x4f5528b0 53,22,50
863 53,22,49,9 409600
864 0x4f5528b0 53,22,9
865 53,22,9 409600
866 0x4f5528b0 53,22,49,9
867 53,22,50
868 0x4f5528b0 53,22,50
869 53,22,49,9 413696
870 0x4f5528b0 53,22,9
871 53,22,9 413696
872 53,22,50
873
874 0x4f5528b0 53,22,49,9
875 53,22,49,9 417792
876 0x4f5528b0 53,22,4,9
877 53,22,4,9 417792
...
880 0x4f5528b0 53,22,9
881 53,22,9 417792
Now I see there's a GETATTR after the CLOSE that returns the right size
-- but
I'll really should track down what's happening to that one to see if it
is the
same call that the test is making. Unfortunately, I'm getting pulled
away
again, so I'll dig more later. Thanks for looking.
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
