Generally looks fine, but as Dave already mentioned I'd rather keep
i_state manipulation outside the filesystems. This could be done with
two wrappers like the following, which should also keep the churn
inside fsync implementations downs:
int fsync_begin(struct inode *inode, int datasync)
{
int ret = 0;
unsigned mask = I_DIRTY_DATASYNC;
if (!datasync)
mask |= I_DIRTY_SYNC;
spin_lock(&inode_lock);
if (!inode_writeback_begin(inode, 1))
goto out;
if (!(inode->i_state & mask))
goto out;
inode->i_state &= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC);
ret = 1;
out:
spin_unlock(&inode_lock);
return ret;
}
static void fsync_end(struct inode *inode, int fail)
{
spin_lock(&inode_lock);
if (fail)
inode->i_state |= I_DIRTY_SYNC | I_DIRTY_DATASYNC;
inode_writeback_end(inode);
spin_unlock(&inode_lock);
}
note that this one marks the inode fully dirty in case of a failure,
which is a bit overkill but keeps the interface simpler. Given that
failure is fsync is catastrophic anyway (filesystem corruption, etc)
that seems fine to me.
Alternatively we could add a fsync_helper that gets a function
pointer with the ->write_inode signature and contains the above
code before and after it. generic_file_fsync would pass the real
->write_inode while other filesystems could pass specific routines.
--
