--- Log opened Wed Feb 18 00:00:52 2026
15:09 -!- jelly is now known as Guest3038
17:48 < dhowells> Asmadeus: okay, the problem is that netfs_perform_write() moves i_size after netfs_write_folio() has sampled it
17:49 < dhowells> but I'm not sure how because both functions access i_size with the folio locked
18:04 < Asmadeus> Are you sure it's not as he said in the commit? e.g. ls triggering a stat fetch from server, and outdated server value overriding the local one
18:05 < dhowells> Asmadeus: weird - I'm seeing i_size revert somehow
18:06 < Asmadeus> v9fs_vfs_getattr() calls v9fs_stat2inode() which in turn sets i_size (v9fs_i_size_write) if not told not to
18:07 < dhowells> 9p_repro.sh-1580 : netfs_write_iter: WRITE-ITER i=d306b44 s=1e l=3 f=10
18:07 < dhowells> 9p_repro.sh-1580 : netfs_modify: i=d306b44 ix=00000 o=1e l=3
18:07 < dhowells> 9p_repro.sh-1580 : netfs_folio: i=d306b44 ix=00000-00000 mod-streamw
18:07 < dhowells> 9p_repro.sh-1580 : netfs_i_size: i=d306b44 iz=21
18:07 < dhowells> 9p_repro.sh-1582 : netfs_write_iter: WRITE-ITER i=d306b44 s=1e l=3 f=10
18:07 < dhowells> 9p_repro.sh-1582 : netfs_finfo: i=d306b44 ix=00000 dirty=1e-21
18:07 < dhowells> 9p_repro.sh-1582 : netfs_flush: i=d306b44 ix=00000 o=1e l=3
18:07 < dhowells> 9p_repro.sh-1582 : netfs_folio: i=d306b44 ix=00000-00000 flush
18:07 < dhowells> 9p_repro.sh-1582 : netfs_write: R=000001fd WRITEBACK c=00000000 i=d306b44 by=0-ffffffffffffffff
18:07 < dhowells> 9p_repro.sh-1582 : netfs_i_size: i=d306b44 iz=1e
18:07 < dhowells> 9p_repro.sh-1582 : netfs_wback: i=d306b44 ix=00000 o=1e l=3 iz=1e
18:07 < dhowells> 9p_repro.sh-1582 : netfs_folio: i=d306b44 ix=00000-00000 store
18:07 < dhowells> hmmm
18:08 < dhowells> maybe 9p is reverting the inode size?
18:08 < Asmadeus> Yes, that's what his patch prevents (if cache is enabled)
18:08 < Asmadeus> Well, v2
18:09 < dhowells> ah - there's a patch at the beginning of the message sequence - I started in the middle of it
18:10 < dhowells> I have to go out for a bit, will try that patch when I get back
18:10 < Asmadeus> I'm not sure why v1 wanted to wait (and worked), so I'm tempted to just go with v2: if cache is enabled then the client view is authoritative... But 9p doesn't have any mechanism like nfs to lease or just re-check a file after 60s, so it's pretty permanent
18:11 < Asmadeus> Ah, sorry if you hadn't seen the patch. Christian says it works so I trust his test here, mostly wanted your opinion on this -- how is netfs supposed to handle remote file changes
20:56 -!- jelly is now known as Guest3067
21:30 < dhowells> Asmadeus: the 9P write RPC call doesn't return the new file attributes, does it?
21:41 -!- Guest3067 is now known as jelly
21:51 < dhowells> Asmadeus: I think your options are a bit limited with 9P as it doesn't have any equivalent of AFS's data version or NFS's change attribute
21:51 < dhowells> if a third party change happens, you're kind of stuffed
21:53 < dhowells> I think what we need to do is, check to see whether the size differs from that which we expect (ie. ->remote_i_size)
22:03 < Asmadeus> right, write returns the amount of data written, so we can keep track of what we wrote but it doesn't tell more
22:06 < Asmadeus> I didn't remember remote_i_size, we actually set it in a couple of places... but I don't see where it's actually used?
22:10 < Asmadeus> oh, in fs/nfs/inode.c they have an interesting check: if the new_isize obtained from server is different from the cur_isize, they only update the size if !nfs_have_writebacks(inode)
22:11 < Asmadeus> Is there a way to tell if there are writebacks in progress with netfs?
22:12 < Asmadeus> It apparently just checks if there are outstanding requests (atomic_long_read(&NFS_I(inode)->nrequests) != 0)
22:13 < Asmadeus> It seems to me that if there's no outstanding request/unflushed data and size from the server doesn't match we could invalidate the cache and set the new size -- it's not perfect (won't catch in-place writes) but better than just ignoring the size
22:14 < Asmadeus> If there's unflushed data or pending writes then keeping the local size is the sane thing to do
22:38 < dhowells> Asmadeus: see the reply I just sent
22:41 < Asmadeus> We can't check mtime, it's set by the server when it processes the IO but we don't know exactly when (won't exactly match whenever we update our's)
22:42 < Asmadeus> likewise remote_i_size is the last size we got, so if we just wrote to the file it's expected to have changed?
22:43 < dhowells> by "we don't know exactly when" - do you mean you don't know at what point the server will update it or do you mean that you don't know to what the server will update it?
22:43 < dhowells> "... to what the server will set it?" I should say
22:43 < Asmadeus> we don't know to what the server will set it, right
22:44 < Asmadeus> It should set it somewhere between a write call and it's reply, but the value will be any timestamp in between
22:44 < dhowells> wrt to i_size, I sent a follow up to my reply - we'd have to update remote_i_size on completion of the write
22:44 < dhowells> of the write RPC
22:45 < dhowells> hmmm
22:45 < dhowells> so we can predict the new i_size, but not the new mtime
22:45 < Asmadeus> Well I guess we could bound it, but then there's also the can of worms of the server and client not being synchronized properly.. it'll be set at the *server time* between these two, not our's
22:45 < dhowells> however, we can note that we expect the mtime to have changed
22:46 < dhowells> and forego the check
22:46 < Asmadeus> Right we could note that we expect it to not have changed if we didn't do any IO
22:46 < Asmadeus> (and thus any change means remote got updated)
22:47 < dhowells> I presume there's no way to "compound" a status fetch with a write?
22:47 < dhowells> (like you can do in cifs and nfs3+)
22:47 < Asmadeus> For size, just updating the size when write is done might not be enough for the same reason... Picture this: write starts -> sent to server, server updates size, meanwhile we send a stat() in parallel and get updated size.. but write is not complete so remote i_size isn't updated yet
22:48 < Asmadeus> Correct, we have no compound/chained IO either
22:48 < dhowells> actually, stat() is less of a problem
22:49 < Asmadeus> I think there's no way around either serializing stat() with other IOs, or ignoring the size if there are IOs in flight like nfs does
22:49 < dhowells> unless userspace states AT_STATX_DONT_SYNC, you're going to have to flush
22:49 < iokill> 
22:51 < Asmadeus> Would a flush make any further IO wait for the stat to complete as well?
22:51 < dhowells> I suppose not
22:51 < Asmadeus> (I don't think we're flushing at all right now.. that might look like the v1)
22:52 < dhowells> no, you call filemap_fdatawrite(), but that doesn't wait
22:52 < Asmadeus>        AT_STATX_SYNC_AS_STAT
22:52 < Asmadeus>               Do whatever stat(2) does.  This is the default and is very much filesystem-specific.
22:53 < dhowells> yes - because this is a flag for statx(), not stat()
22:53 < Asmadeus> Yes, but I meant according to this there is not standard about whether we need to sync or not?
22:53 < dhowells> for stat(), correct
22:53 < Asmadeus> (although we shouldn't ignore AT_STATX_FORCE_SYNC..)
22:54 < dhowells> I think you need to call filemap_write_and_wait()
22:54 < dhowells> unless AT_STATX_DONT_SYNC is specified
22:54 < dhowells> see nfs_getattr()
22:55 < Asmadeus> So that'd be his v1 - https://lore.kernel.org/all/20251227083751.715152-1-pierre@barre.sh/T/#u
22:55 < Asmadeus> (well, it actually does both waiting and ignores the value in cached mode)
22:56 < dhowells> yeah - it needs some work, I think
22:56 < dhowells> I can have a play with it
22:57 < dhowells> give me a little while to poke at it
22:59 < Asmadeus> Sure, thanks -- I'll reply to the mail anyway with what we discussed (perhaps just quote it all as well...) for others
23:00 < Asmadeus> And I still think something like nfs update-i_size-unless-nfs_have_writebacks() might be interesting if we can do it reasonably