Patchwork [1/2] writeback: sync quota after inodes writeback

login
register
mail settings
Submitter Dmitry Monakhov
Date Oct. 8, 2010, 3 p.m.
Message ID <1286550027-9684-1-git-send-email-dmonakhov@gmail.com>
Download mbox | patch
Permalink /patch/67230/
State New
Headers show

Comments

Dmitry Monakhov - Oct. 8, 2010, 3 p.m.
inode writeback usually result in quota changes especially
on filesystems with delalloc. So quota_sync() before writeback
seems pointless. Let's do the job in a natural way.

Signed-off-by: Dmitry Monakhov <dmonakhov@gmail.com>
---
 fs/sync.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Christoph Hellwig - Oct. 8, 2010, 5:08 p.m.
On Fri, Oct 08, 2010 at 07:00:26PM +0400, Dmitry Monakhov wrote:
> inode writeback usually result in quota changes especially
> on filesystems with delalloc. So quota_sync() before writeback
> seems pointless. Let's do the job in a natural way.

Yes, that's one reason why we can't use the quota sync callback for XFS,
as we need to do it after the actual sync.  Then again I'm not even
sure we need to bother with a callout from the writeback code.
Following the model of all other quota code filesystems using quota
should probably just call it from their ->sync_fs method.  Note that
this allows allows other optimizations for using the generic quota
code.  dquot_quota_sync currently calls ->sync_fs by itself which
could be optimized away by that design.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - Oct. 20, 2010, 12:25 p.m.
On Fri 08-10-10 13:08:06, Christoph Hellwig wrote:
> On Fri, Oct 08, 2010 at 07:00:26PM +0400, Dmitry Monakhov wrote:
> > inode writeback usually result in quota changes especially
> > on filesystems with delalloc. So quota_sync() before writeback
> > seems pointless. Let's do the job in a natural way.
> 
> Yes, that's one reason why we can't use the quota sync callback for XFS,
> as we need to do it after the actual sync.  Then again I'm not even
> sure we need to bother with a callout from the writeback code.
> Following the model of all other quota code filesystems using quota
> should probably just call it from their ->sync_fs method.  Note that
> this allows allows other optimizations for using the generic quota
> code.  dquot_quota_sync currently calls ->sync_fs by itself which
> could be optimized away by that design.
  Yes, that would make sense but then we have to change quotactl(Q_SYNC,
...) to result in calling ->sync_fs() as it does now. Because we have
to get quota data to disk in response to this call.

								Honza
Christoph Hellwig - Oct. 20, 2010, 3:11 p.m.
On Wed, Oct 20, 2010 at 02:25:52PM +0200, Jan Kara wrote:
>   Yes, that would make sense but then we have to change quotactl(Q_SYNC,
> ...) to result in calling ->sync_fs() as it does now. Because we have
> to get quota data to disk in response to this call.

That's a callout from the quoatactl code, which we use the quotactl ops
for, so keeping it sounds fine at least from the highlevel design
perspective.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/sync.c b/fs/sync.c
index ba76b96..b0e2c6c 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -36,14 +36,14 @@  static int __sync_filesystem(struct super_block *sb, int wait)
 	if (!sb->s_bdi || sb->s_bdi == &noop_backing_dev_info)
 		return 0;
 
-	if (sb->s_qcop && sb->s_qcop->quota_sync)
-		sb->s_qcop->quota_sync(sb, -1, wait);
-
 	if (wait)
 		sync_inodes_sb(sb);
 	else
 		writeback_inodes_sb(sb);
 
+	if (sb->s_qcop && sb->s_qcop->quota_sync)
+		sb->s_qcop->quota_sync(sb, -1, wait);
+
 	if (sb->s_op->sync_fs)
 		sb->s_op->sync_fs(sb, wait);
 	return __sync_blockdev(sb->s_bdev, wait);