[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [patch 3/7] fs: introduce inode writeback helpers
On Wed, Nov 24, 2010 at 01:06:13AM +1100, wrote:
> Inode dirty state cannot be securely tested without participating properly
> in the inode writeback protocol. Some filesystems need to check this state,
> so break out the code into helpers and make them available.
> This could also be used to reduce strange interactions between background
> writeback and fsync. Currently if we fsync a single page in a file, the
> entire file gets requeued to the back of the background IO list, even if
> it is due for writeout and has a large number of pages. That's left for
> a later time.

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;

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;
return ret;

static void fsync_end(struct inode *inode, int fail)
if (fail)
inode->i_state |= I_DIRTY_SYNC | I_DIRTY_DATASYNC;

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.

 \ /
  Last update: 2010-11-29 16:15    [W:0.078 / U:2.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site