On Tue 05-06-12 14:15:46, Dave Chinner wrote: > On Sat, Jun 02, 2012 at 12:30:32AM +0200, Jan Kara wrote: > > Generic code now blocks all writers from standard write paths. So we add > > blocking of all writers coming from ioctl (we get a protection of ioctl against > > racing remount read-only as a bonus) and convert xfs_file_aio_write() to a > > non-racy freeze protection. We also keep freeze protection on transaction > > start to block internal filesystem writes such as removal of preallocated > > blocks. > > I don't think this will apply to a current TOT XFS - the end_io > context hunks look wrong. Perhaps your rebased this before the XFS > tree was merged? Umm, doesn't look like that. I've based my patches on top of 51eab603f5c86dd1eae4c525df3e7f7eeab401d6 which is after XFS merge. > > CC: Ben Myers <bpm@xxxxxxx> > > CC: Alex Elder <elder@xxxxxxxxxx> > > CC: xfs@xxxxxxxxxxx > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > --- > > fs/xfs/xfs_aops.c | 18 ++++++++++++++++ > > fs/xfs/xfs_file.c | 10 ++++++-- > > fs/xfs/xfs_ioctl.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++-- > > fs/xfs/xfs_ioctl32.c | 12 ++++++++++ > > fs/xfs/xfs_iomap.c | 4 +- > > fs/xfs/xfs_mount.c | 2 +- > > fs/xfs/xfs_mount.h | 3 -- > > fs/xfs/xfs_sync.c | 2 +- > > fs/xfs/xfs_trans.c | 17 ++++++++++++-- > > fs/xfs/xfs_trans.h | 2 + > > 10 files changed, 109 insertions(+), 16 deletions(-) > > > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > > index ae31c31..4a001b8 100644 > > --- a/fs/xfs/xfs_aops.c > > +++ b/fs/xfs/xfs_aops.c > > @@ -124,6 +124,12 @@ xfs_setfilesize_trans_alloc( > > ioend->io_append_trans = tp; > > > > /* > > + * We will pass freeze protection with a transaction. So tell lockdep > > + * we released it. > > + */ > > + rwsem_release(&ioend->io_inode->i_sb->s_writers.lock_map[SB_FREEZE_FS-1], > > + 1, _THIS_IP_); > > + /* > > Oh, that's rather ugly. If this is necessary where a transaction > handle is passed to another thread and completed there, then this > really needs to be wrapped in helper functions so it is always done > correctly when the PF_TRANS flag is also transferred. That can be > done later, though. It will also need to be done to the allocation > code which passes allocation off to a workqueue, but that is > currently synchronous so won't be a problem for this change right > now... This lockdep magic is necessary because lockdep freaks out if you acquire lock in one process and release it in another one. But wrapping that inside a function is a good idea. > > @@ -631,7 +638,11 @@ xfs_ioc_space( > > if (ioflags & IO_INVIS) > > attr_flags |= XFS_ATTR_DMI; > > > > + error = mnt_want_write_file(filp); > > + if (error) > > + return error; > > error = xfs_change_file_space(ip, cmd, bf, filp->f_pos, attr_flags); > > + mnt_drop_write_file(filp); > > return -error; > > Those positive/negative error conversions are starting to get > confusing and difficult to get right. I'm going to have to convert > XFS at some point to return negative errors everywhere so we can get > rid of that problem once and for all... Yeah, it's a bit messy. > Otherwise, this looks OK. I'll need to pull this in and test it, but > the I was using the previous version of the patch series for almost > the entire 3.4-rc cycle and didn't come across any problems with > it.... Thanks! Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
