ocfs2: Add quota calls for allocation and freeing of inodes and space

PATCH New Jan Kara 2 participants 5 messages Oct 20, 2008 – Oct 24, 2008 OCFS2

Share & tools

Jan Kara

author

2008-10-20 17:23 UTC #0 lore raw

PATCH 1/1 ocfs2: Add quota calls for allocation and freeing of inodes and space

Add quota calls for allocation and freeing of inodes and space, also update estimates on number of needed credits for a transaction. Move out inode allocation from ocfs2_mknod_locked() because vfs_dq_init() must be called outside of a transaction.

Signed-off-by Jan Kara <[email protected]>

---
 fs/ocfs2/alloc.c   |   18 ++++++-
 fs/ocfs2/aops.c    |   16 +++++-
 fs/ocfs2/dir.c     |   24 ++++++++-
 fs/ocfs2/file.c    |   74 +++++++++++++++++++++++--
 fs/ocfs2/inode.c   |    8 ++-
 fs/ocfs2/journal.h |   99 ++++++++++++++++++++++++----------
 fs/ocfs2/namei.c   |  151 +++++++++++++++++++++++++++++++++++----------------
 7 files changed, 298 insertions(+), 92 deletions(-)

fs/ocfs2/alloc.c +17 -1

index 10bfb46..776dd95 100644
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/highmem.h>
 #include <linux/swap.h>
+#include <linux/quotaops.h>
 
 #define MLOG_MASK_PREFIX ML_DISK_ALLOC
 #include <cluster/masklog.h>

@@ -5954,6 +5955,8 @@ static int ocfs2_do_truncate(struct ocfs2_super *osb,
 		goto bail;
 	}
 
+	vfs_dq_free_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_del));
 	spin_lock(&OCFS2_I(inode)->ip_lock);
 	OCFS2_I(inode)->ip_clusters = le32_to_cpu(fe->i_clusters) -
 				      clusters_to_del;

@@ -6262,6 +6265,7 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode,
 	struct ocfs2_alloc_context *data_ac = NULL;
 	struct page **pages = NULL;
 	loff_t end = osb->s_clustersize;
+	int did_quota = 0;
 
 	has_data = i_size_read(inode) ? 1 : 0;

@@ -6281,7 +6285,8 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode,
 		}
 	}
 
-	handle = ocfs2_start_trans(osb, OCFS2_INLINE_TO_EXTENTS_CREDITS);
+	handle = ocfs2_start_trans(osb,
+				   ocfs2_inline_to_extents_credits(osb->sb));
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
 		mlog_errno(ret);

@@ -6300,6 +6305,13 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode,
 		unsigned int page_end;
 		u64 phys;
 
+		if (vfs_dq_alloc_space_nodirty(inode,
+				       ocfs2_clusters_to_bytes(osb->sb, 1))) {
+			ret = -EDQUOT;
+			goto out_commit;
+		}
+		did_quota = 1;
+
 		ret = ocfs2_claim_clusters(osb, handle, data_ac, 1, &bit_off,
 					   &num);
 		if (ret) {

@@ -6372,6 +6384,10 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode,
 	}
 
 out_commit:
+	if (ret < 0 && did_quota)
+		vfs_dq_free_space_nodirty(inode,
+					  ocfs2_clusters_to_bytes(osb->sb, 1));
+
 	ocfs2_commit_trans(osb, handle);
 
 out_unlock:

fs/ocfs2/aops.c +13 -3

index a53da14..6f0f8c5 100644
@@ -27,6 +27,7 @@
 #include <linux/swap.h>
 #include <linux/pipe_fs_i.h>
 #include <linux/mpage.h>
+#include <linux/quotaops.h>
 
 #define MLOG_MASK_PREFIX ML_FILE_IO
 #include <cluster/masklog.h>

@@ -1736,6 +1737,11 @@ int ocfs2_write_begin_nolock(struct address_space *mapping,
 
 	wc->w_handle = handle;
 
+	if (clusters_to_alloc && vfs_dq_alloc_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_alloc))) {
+		ret = -EDQUOT;
+		goto out_commit;
+	}
 	/*
 	 * We don't want this to fail in ocfs2_write_end(), so do it
 	 * here.

@@ -1744,7 +1750,7 @@ int ocfs2_write_begin_nolock(struct address_space *mapping,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out_quota;
 	}
 
 	/*

@@ -1757,14 +1763,14 @@ int ocfs2_write_begin_nolock(struct address_space *mapping,
 					 mmap_page);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out_quota;
 	}
 
 	ret = ocfs2_write_cluster_by_desc(mapping, data_ac, meta_ac, wc, pos,
 					  len);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out_quota;
 	}
 
 	if (data_ac)

@@ -1776,6 +1782,10 @@ success:
 	*pagep = wc->w_target_page;
 	*fsdata = wc;
 	return 0;
+out_quota:
+	if (clusters_to_alloc)
+		vfs_dq_free_space(inode,
+			  ocfs2_clusters_to_bytes(osb->sb, clusters_to_alloc));
 out_commit:
 	ocfs2_commit_trans(osb, handle);

fs/ocfs2/dir.c +22 -2

index 9cce563..c679083 100644
@@ -40,6 +40,7 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/highmem.h>
+#include <linux/quotaops.h>
 
 #define MLOG_MASK_PREFIX ML_NAMEI
 #include <cluster/masklog.h>

@@ -1182,9 +1183,9 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
 				   unsigned int blocks_wanted,
 				   struct buffer_head **first_block_bh)
 {
-	int ret, credits = OCFS2_INLINE_TO_EXTENTS_CREDITS;
 	u32 alloc, bit_off, len;
 	struct super_block *sb = dir->i_sb;
+	int ret, credits = ocfs2_inline_to_extents_credits(sb);
 	u64 blkno, bytes = blocks_wanted << sb->s_blocksize_bits;
 	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
 	struct ocfs2_inode_info *oi = OCFS2_I(dir);

@@ -1192,6 +1193,7 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
 	struct buffer_head *dirdata_bh = NULL;
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
 	handle_t *handle;
+	int did_quota = 0;
 
 	alloc = ocfs2_clusters_for_bytes(sb, bytes);

@@ -1227,6 +1229,12 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
 		goto out_sem;
 	}
 
+	if (vfs_dq_alloc_space_nodirty(dir,
+				ocfs2_clusters_to_bytes(osb->sb, alloc))) {
+		ret = -EDQUOT;
+		goto out_commit;
+	}
+	did_quota = 1;
 	/*
 	 * Try to claim as many clusters as the bitmap can give though
 	 * if we only get one now, that's enough to continue. The rest

@@ -1349,6 +1357,9 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
 	dirdata_bh = NULL;
 
 out_commit:
+	if (ret < 0 && did_quota)
+		vfs_dq_free_space_nodirty(dir,
+			ocfs2_clusters_to_bytes(osb->sb, 2));
 	ocfs2_commit_trans(osb, handle);
 
 out_sem:

@@ -1373,7 +1384,7 @@ static int ocfs2_do_extend_dir(struct super_block *sb,
 			       struct buffer_head **new_bh)
 {
 	int status;
-	int extend;
+	int extend, did_quota = 0;
 	u64 p_blkno, v_blkno;
 
 	spin_lock(&OCFS2_I(dir)->ip_lock);

@@ -1383,6 +1394,13 @@ static int ocfs2_do_extend_dir(struct super_block *sb,
 	if (extend) {
 		u32 offset = OCFS2_I(dir)->ip_clusters;
 
+		if (vfs_dq_alloc_space_nodirty(dir,
+					ocfs2_clusters_to_bytes(sb, 1))) {
+			status = -EDQUOT;
+			goto bail;
+		}
+		did_quota = 1;
+
 		status = ocfs2_do_extend_allocation(OCFS2_SB(sb), dir, &offset,
 						    1, 0, parent_fe_bh, handle,
 						    data_ac, meta_ac, NULL);

@@ -1408,6 +1426,8 @@ static int ocfs2_do_extend_dir(struct super_block *sb,
 	}
 	status = 0;
 bail:
+	if (did_quota && status < 0)
+		vfs_dq_free_space_nodirty(dir, ocfs2_clusters_to_bytes(sb, 1));
 	mlog_exit(status);
 	return status;
 }

fs/ocfs2/file.c +68 -6

128 lines not on this page · raw message

fs/ocfs2/inode.c +6 -2

index c3d76d6..4a12238 100644
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/highmem.h>
 #include <linux/pagemap.h>
+#include <linux/quotaops.h>
 
 #include <asm/byteorder.h>

@@ -601,7 +602,8 @@ static int ocfs2_remove_inode(struct inode *inode,
 		goto bail;
 	}
 
-	handle = ocfs2_start_trans(osb, OCFS2_DELETE_INODE_CREDITS);
+	handle = ocfs2_start_trans(osb, OCFS2_DELETE_INODE_CREDITS +
+					ocfs2_quota_trans_credits(inode->i_sb));
 	if (IS_ERR(handle)) {
 		status = PTR_ERR(handle);
 		mlog_errno(status);

@@ -633,6 +635,7 @@ static int ocfs2_remove_inode(struct inode *inode,
 	}
 
 	ocfs2_remove_from_cache(inode, di_bh);
+	vfs_dq_free_inode(inode);
 
 	status = ocfs2_free_dinode(handle, inode_alloc_inode,
 				   inode_alloc_bh, di);

@@ -908,7 +911,8 @@ void ocfs2_delete_inode(struct inode *inode)
 
 	mlog_entry("(inode->i_ino = %lu)\n", inode->i_ino);
 
-	if (is_bad_inode(inode)) {
+	/* Inode left after unsuccessful inode allocation? */
+	if (is_bad_inode(inode) || !OCFS2_I(inode)->ip_blkno) {
 		mlog(0, "Skipping delete of bad inode\n");
 		goto bail;
 	}

fs/ocfs2/journal.h +69 ~~-30~~

159 lines not on this page · raw message

fs/ocfs2/namei.c +103 ~~-48~~

317 lines not on this page · raw message

Signature · 1 line

1.5.2.4

Joel Becker

2008-10-21 23:34 UTC #1 lore raw

PATCH 1/1 ocfs2: Add quota calls for allocation and freeing of inodes and space

Quoted 7 lines · Jan Kara

Add quota calls for allocation and freeing of inodes and space, also update estimates on number of needed credits for a transaction. Move out inode allocation from ocfs2_mknod_locked() because vfs_dq_init() must be called outside of a transaction.
Signed-off-by Jan Kara <[email protected]>

fs/ocfs2/alloc.c static int ocfs2_do_truncate(struct ocfs2_super *osb, · line 5962

Quoted 8 lines

fs/ocfs2/alloc.c +2 -0

@@ -5954,6 +5955,8 @@ static int ocfs2_do_truncate(struct ocfs2_super *osb,
 		goto bail;
 	}
 
+	vfs_dq_free_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_del));
 	spin_lock(&OCFS2_I(inode)->ip_lock);
 	OCFS2_I(inode)->ip_clusters = le32_to_cpu(fe->i_clusters) -
 				      clusters_to_del;

In this one instance, you don't clean up the quota change on
error, even though clusters_to_del might remain allocated. How come?
On that note, I don't see us a) fixing the inode up on failure
or b) writing out the new values. Now, I remember from the past that we allow a smaller i_clusters than actually in the tree, because our alloc code can handle that. So I'm sure that the fallout from us not fixing up i_clusters and/or writing it is safe.
But I wonder what the right behavior is with regards to quota.

fs/ocfs2/inode.c void ocfs2_delete_inode(struct inode *inode) · line 918

Quoted 10 lines

fs/ocfs2/inode.c +2 -1

@@ -908,7 +911,8 @@ void ocfs2_delete_inode(struct inode *inode)
 
 	mlog_entry("(inode->i_ino = %lu)\n", inode->i_ino);
 
-	if (is_bad_inode(inode)) {
+	/* Inode left after unsuccessful inode allocation? */
+	if (is_bad_inode(inode) || !OCFS2_I(inode)->ip_blkno) {
 		mlog(0, "Skipping delete of bad inode\n");
 		goto bail;
 	}

!ip_blkno is your way of saying you called
ocfs2_get_init_inode() but failed ocfs2_claim_new_inode() in ocfs2_mknod_locked(), right? Probably want to adjust your comment to point out that is_bad_inode() comes from read_inode(), !ip_blkno from mknod().
I checked into just having your failed mknod() call
make_bad_inode(), but it doesn't fit. We don't even want to get there.

Joel

Signature · 10 lines

"We will have to repent in this generation not merely for the
 vitriolic words and actions of the bad people, but for the 
 appalling silence of the good people."
	- Rev. Dr. Martin Luther King, Jr.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Jan Kara

author

2008-10-22 14:49 UTC #2 lore raw

PATCH 1/1 ocfs2: Add quota calls for allocation and freeing of inodes and space

Quoted 11 lines · Joel Becker

Quoted 10 lines · Jan Kara
Add quota calls for allocation and freeing of inodes and space, also update estimates on number of needed credits for a transaction. Move out inode allocation from ocfs2_mknod_locked() because vfs_dq_init() must be called outside of a transaction.
Signed-off-by Jan Kara <[email protected]>
fs/ocfs2/alloc.c +0 -0
@@ -5954,6 +5955,8 @@ static int ocfs2_do_truncate(struct ocfs2_super *osb,
 		goto bail;
 	}
 

Quoted 8 lines

fs/ocfs2/alloc.c static int ocfs2_do_truncate(struct ocfs2_super *osb, · line 5962
Quoted 5 lines
fs/ocfs2/alloc.c +2 -0
+	vfs_dq_free_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_del));
 	spin_lock(&OCFS2_I(inode)->ip_lock);
 	OCFS2_I(inode)->ip_clusters = le32_to_cpu(fe->i_clusters) -
 				      clusters_to_del;
In this one instance, you don't clean up the quota change on
error, even though clusters_to_del might remain allocated. How come?

Good point. Is there a way to find out how many clusters were actually
truncated before we failed?

Quoted 6 lines

On that note, I don't see us a) fixing the inode up on failure
or b) writing out the new values. Now, I remember from the past that we allow a smaller i_clusters than actually in the tree, because our alloc code can handle that. So I'm sure that the fallout from us not fixing up i_clusters and/or writing it is safe.
But I wonder what the right behavior is with regards to quota.

Well, quota usage must be always in sync with i_clusters - or better
i_blocks - because on chown or chgrp, that is what you're going to transfer from one user to another one.
As a side note, I've actually noticed a few paths (even for cases of
ENOSPC) in OCFS2 where if we fail during allocation, we actually leave behind allocated blocks not attached anywhere (i.e., only fsck can reclaim them). I discussed it with Mark and it's not completely trivial to fix. So I've worked it around so that quota allocation is done before allocating any blocks and I also tried to take care that at least quota stays correct after a failure.

Quoted 4 lines

fs/ocfs2/inode.c +0 -0

@@ -908,7 +911,8 @@ void ocfs2_delete_inode(struct inode *inode)
 
 	mlog_entry("(inode->i_ino = %lu)\n", inode->i_ino);

Quoted 10 lines

fs/ocfs2/inode.c void ocfs2_delete_inode(struct inode *inode) · line 918
Quoted 6 lines
fs/ocfs2/inode.c +2 -1
-	if (is_bad_inode(inode)) {
+	/* Inode left after unsuccessful inode allocation? */
+	if (is_bad_inode(inode) || !OCFS2_I(inode)->ip_blkno) {
 		mlog(0, "Skipping delete of bad inode\n");
 		goto bail;
 	}
!ip_blkno is your way of saying you called
ocfs2_get_init_inode() but failed ocfs2_claim_new_inode() in ocfs2_mknod_locked(), right?

Or user is over quota...

Quoted 3 lines

Probably want to adjust your comment to
point out that is_bad_inode() comes from read_inode(), !ip_blkno from mknod().

Good point. Will do.

Quoted 2 lines

I checked into just having your failed mknod() call
make_bad_inode(), but it doesn't fit. We don't even want to get there.

Yes, I was also looking into it before introducing the test and for some
reason I didn't like it much.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Joel Becker

2008-10-22 17:31 UTC #3 lore raw

PATCH 1/1 ocfs2: Add quota calls for allocation and freeing of inodes and space

Quoted 20 lines · Jan Kara

Quoted 11 lines · Joel Becker
Quoted 10 lines · Jan Kara
Add quota calls for allocation and freeing of inodes and space, also update estimates on number of needed credits for a transaction. Move out inode allocation from ocfs2_mknod_locked() because vfs_dq_init() must be called outside of a transaction.
Signed-off-by Jan Kara <[email protected]>
fs/ocfs2/alloc.c +0 -0
@@ -5954,6 +5955,8 @@ static int ocfs2_do_truncate(struct ocfs2_super *osb,
 		goto bail;
 	}
 
Quoted 8 lines
fs/ocfs2/alloc.c static int ocfs2_do_truncate(struct ocfs2_super *osb, · line 5962
Quoted 5 lines
fs/ocfs2/alloc.c +2 -0
+	vfs_dq_free_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_del));
 	spin_lock(&OCFS2_I(inode)->ip_lock);
 	OCFS2_I(inode)->ip_clusters = le32_to_cpu(fe->i_clusters) -
 				      clusters_to_del;
In this one instance, you don't clean up the quota change on
error, even though clusters_to_del might remain allocated. How come?

Quoted 2 lines

Good point. Is there a way to find out how many clusters were actually
truncated before we failed?

Not quite sure, really. And this brings up the second question.

Quoted 6 lines

Quoted 6 lines
On that note, I don't see us a) fixing the inode up on failure
or b) writing out the new values. Now, I remember from the past that we allow a smaller i_clusters than actually in the tree, because our alloc code can handle that. So I'm sure that the fallout from us not fixing up i_clusters and/or writing it is safe.
But I wonder what the right behavior is with regards to quota.

Quoted 10 lines

Well, quota usage must be always in sync with i_clusters - or better
i_blocks - because on chown or chgrp, that is what you're going to transfer from one user to another one.
As a side note, I've actually noticed a few paths (even for cases of
ENOSPC) in OCFS2 where if we fail during allocation, we actually leave behind allocated blocks not attached anywhere (i.e., only fsck can reclaim them). I discussed it with Mark and it's not completely trivial to fix. So I've worked it around so that quota allocation is done before allocating any blocks and I also tried to take care that at least quota stays correct after a failure.

Right, there are a couple places like this, and the second
question is what matters for quota - what is supposed to be allocated (i_clusters) or what is really allocated (only different from i_clusters in these corner cases).
In that truncate case above, i_clusters on the ocfs2_dinode will
stay at the "supposed to be truncated" number, but journal_dirty() isn't called. If nothing else happens to that buffer, it won't be written. However, if it is written out due to another operation, the smaller i_clusters will appear on disk. This is a safe behavior from the view of the file (truncate happened or did not). The fact that the allocation remains in both cases is something we silently handle in the allocation paths and clean up in fsck when we spot it.

Joel

Signature · 9 lines

Life's Little Instruction Book #267

	"Lie on your back and look at the stars."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Jan Kara

author

2008-10-24 22:28 UTC #4 lore raw

PATCH 1/1 ocfs2: Add quota calls for allocation and freeing of inodes and space

Quoted 43 lines · Joel Becker

Quoted 20 lines · Jan Kara
Quoted 11 lines · Joel Becker
Quoted 10 lines · Jan Kara
Add quota calls for allocation and freeing of inodes and space, also update estimates on number of needed credits for a transaction. Move out inode allocation from ocfs2_mknod_locked() because vfs_dq_init() must be called outside of a transaction.
Signed-off-by Jan Kara <[email protected]>
fs/ocfs2/alloc.c +0 -0
@@ -5954,6 +5955,8 @@ static int ocfs2_do_truncate(struct ocfs2_super *osb,
 		goto bail;
 	}
 
Quoted 8 lines
fs/ocfs2/alloc.c static int ocfs2_do_truncate(struct ocfs2_super *osb, · line 5962
Quoted 5 lines
fs/ocfs2/alloc.c +2 -0
+	vfs_dq_free_space_nodirty(inode,
+			ocfs2_clusters_to_bytes(osb->sb, clusters_to_del));
 	spin_lock(&OCFS2_I(inode)->ip_lock);
 	OCFS2_I(inode)->ip_clusters = le32_to_cpu(fe->i_clusters) -
 				      clusters_to_del;
In this one instance, you don't clean up the quota change on
error, even though clusters_to_del might remain allocated. How come?
Quoted 2 lines
Good point. Is there a way to find out how many clusters were actually
truncated before we failed?
Not quite sure, really. And this brings up the second question.
Quoted 6 lines
Quoted 6 lines
On that note, I don't see us a) fixing the inode up on failure
or b) writing out the new values. Now, I remember from the past that we allow a smaller i_clusters than actually in the tree, because our alloc code can handle that. So I'm sure that the fallout from us not fixing up i_clusters and/or writing it is safe.
But I wonder what the right behavior is with regards to quota.
Quoted 10 lines
Well, quota usage must be always in sync with i_clusters - or better
i_blocks - because on chown or chgrp, that is what you're going to transfer from one user to another one.
As a side note, I've actually noticed a few paths (even for cases of
ENOSPC) in OCFS2 where if we fail during allocation, we actually leave behind allocated blocks not attached anywhere (i.e., only fsck can reclaim them). I discussed it with Mark and it's not completely trivial to fix. So I've worked it around so that quota allocation is done before allocating any blocks and I also tried to take care that at least quota stays correct after a failure.

Quoted 12 lines

Right, there are a couple places like this, and the second
question is what matters for quota - what is supposed to be allocated (i_clusters) or what is really allocated (only different from i_clusters in these corner cases).
In that truncate case above, i_clusters on the ocfs2_dinode will
stay at the "supposed to be truncated" number, but journal_dirty() isn't called. If nothing else happens to that buffer, it won't be written. However, if it is written out due to another operation, the smaller i_clusters will appear on disk. This is a safe behavior from the view of the file (truncate happened or did not). The fact that the allocation remains in both cases is something we silently handle in the allocation paths and clean up in fsck when we spot it.

Well, this is a bit difficult, but let's say that while the filesystem
is consistent with what is says to quota, quota is happy. Probably i_clusters are more important to quota because they're used on chown etc. So fs should make sure it tells quota about space changes exactly as it changes i_clusters.
Anyway, the situation when inode buffer with i_clusters change may or
may not be written to disk seems icky. I think that should be somehow fixed to deterministic behavior, preferably so that i_clusters always match real usage ;) I agree that error handling of these cases is nasty though and in cases of IO errors or so we cannot guarantee anything.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR