[PATCHES] fix flapping offline flag in gpfs module

[Christof Schmitt]

On Fri, Jul 18, 2014 at 11:22:28AM +0200, Volker Lendecke wrote:
> On Fri, Jul 18, 2014 at 12:19:00AM +0200, Michael Adam wrote:
> > Attached find (as a first step) a patcheset that removes
> > the use of stat_ex.vfs_private from vfs_gpfs by calling
> > SMB_VFS_IS_OFFLINE() as discussed above
> > 
> > I guess that this also fixes a few more errors regarding
> > reaction to of assumed offline status in the module.
> > 
> > Review/push appreciated.
> 
> Find the patches with my R-B and two typo fixes in the commit messages
> attached. This also contains the vfs_private removal, which is a good
> thing from my point of view. I think vfs_private is dangerous at best,
> there is no protection at all against conflicting use in multiple
> modules. If we really need such a thing, I think we would have to add
> something like the fsp extensions. But I would really like to know why
> vfs_private is really needed.
> 
> When looking at this area, I'd much rather see winattrs officially added
> to stat_ex, potentially with an additional flag that the winattrs are
> not valid here. There's at least GPFS that has special support for the
> 4 windows bits, and I could see carving out 4 bits from an inode to be
> a very worthwhile optimization for specialized file systems. An xattr
> potentially has much more overhead. And as we can then store the OFFLINE
> flag in the winattrs, we'd have a much cleaner way to do this.
> 
> When adding a validity-bit to the winattrs, this would then
> also lead to one further step: statlite, as referenced in
> http://www.mcs.anl.gov/uploads/cels/papers/TM-302-FINAL.pdf as
> an important performance optimization in the networked file system
> world. It can be very difficult to correctly query the mtime and file
> size, and in particular in unix_convert() Samba does not need that
> information at all.  Also, if a file is heavily accessed concurrently,
> Samba typically maintains its own write timestamp. We don't have to ask
> the file system for the correct mtime in a SMB2_FIND.  But that's much
> further down the road.

My main concern with patches 2 and 3 would be that this now triggers a
notification after each read or write request. On busy systems, that
will likely affect performance.

We probably need a way to cache at least the offline bit. The current
uses of the OFFLINE flag in vfs_gpfs are only in functions that have a
fsp, so we could add a FSP_EXTENSION as a workaround in vfs_gpfs.

I agree that a better way would be have the flags (at least OFFLINE) in
stat_ex. When doing this, we should also add a proper interface to get
and set the flags in the vfs modules. The current approach of vfs_gpfs
intercepting the set_xattr and get_xattr calls seems backwards. Either
we need explicit SET_ATTR and GET_ATTR calls, or require that all STAT
calls update the attributes in stat_ex and also add FSTATAT.

Moving towards statlite support would be great, especially since we
already have that in gpfs.

> > Further steps: extract the hsm-logic using is_offline()
> > from vfs_gpfs and vfs_tsmsm to one universally useful module vfs_hsm.
> > Will happily code this but might take a little time  until I
> > get to really doing it.
> 
> +1.

+1 from me too. I can also put this on my todo list, but i am not sure
when i will get to it.

Christof