Re: [PATCH v3 2/2] 9p: use i_lock to protect update of inode->i_blocks under 32-bit

[Dominique Martinet <asmadeus@codewreck.org>]

Dominique Martinet wrote on Thu, Jan 24, 2019:
> > >  - if I create a file outside of 9p and stat it, for example starting
> > >  with 400 bytes, it starts with 8 blocks then the number of blocks
> > >  increase by 1 everytime 512 bytes are written regardless of initial
> > >  size.
> >
> > The initial i_blocks comes from the underlying filesystem, so I think 8 blocks
> > just mean the block size of the underlying filesystem is 4KB. If another 512 bytes
> > are written into the file, inode_add_bytes() will increase i_blocks by one instead of
> > fetching i_blocks from server, though i_blocks in server side will
> > still be 8.
> 
> Ah, I had only looked at v9fs_stat2inode() which creates the value from
> i_size, not v9fs_stat2inode_dotl().
> v9fs_stat2inode_dotl() does take the value from the RPC, and then 8 is
> indeed correct -- it will just be very painful to keep synchronized with
> the underlying fs; we have no ways of knowing when the remote filesystem
> decides to allocate new blocks.
> If for example the write iter only writes 0 and the underlying fs is
> optimized st_blocks might not increase at all.
> 
> > Maybe we should call v9fs_invalidate_inode_attr() in v9fs_write_end() ?
> 
> Hmm, That will probably add quite a lot of RPCs and make the cache
> option rather useless...

Actually I made an interesting point without realizing it myself -
thinking back of the option, I would keep the current behaviour with
cache=none/mmap (as the inode attr will be invalid and refreshed on
every stat anyway - this incidentally probably is why I never noticed
it, cache=none works just fine as far as st_blocks is concerned because
it always returns the underlying storage's block count) and just 100%
fake it for cache=fscache/loose.

Still going to bring this whole subject up in a separate thread to
fsdevel tomorrow, I'll put you and the v9fs-developer list in cc.


Will also add a note to run more testings with various cache
options...

-- 
Dominique