[PATCH 0/3] reiser4: space grabbing fixes.

[PATCH 0/3] reiser4: space grabbing fixes.

[Ivan Shapovalov]

These are mostly equivalent transformations (aside from the first patch)
I've done when I was experimenting with rwsems at zero-th iteration of
batch discard support.


Ivan Shapovalov (3):
  reiser4: block_alloc: improve error handling in reiser4_grab_reserved().
  reiser4: block_alloc: sanitize grab_enabled modifications.
  reiser4: do not mess with grab_enabled; instead, use BA_FORCE.

 fs/reiser4/block_alloc.c                 | 57 +++++++++++++++++---------------
 fs/reiser4/plugin/file/file.c            | 12 +++----
 fs/reiser4/plugin/file/file_conversion.c |  3 +-
 fs/reiser4/plugin/file/tail_conversion.c | 11 +++---
 fs/reiser4/plugin/item/extent_file_ops.c |  3 +-
 fs/reiser4/plugin/item/tail.c            |  3 +-
 fs/reiser4/safe_link.c                   |  4 +--
 7 files changed, 44 insertions(+), 49 deletions(-)

-- 
2.0.4

[PATCH 1/3] reiser4: block_alloc: improve error handling in reiser4_grab_reserved().

[Ivan Shapovalov]

In case of error in reiser4_grab_space(), return the original error code,
not -ENOSPC unconditionally.

Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
---
 fs/reiser4/block_alloc.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
index 324b11c..7f9f910 100644
--- a/fs/reiser4/block_alloc.c
+++ b/fs/reiser4/block_alloc.c
@@ -360,37 +360,40 @@ int reiser4_grab_reserved(struct super_block *super,
 			  __u64 count, reiser4_ba_flags_t flags)
 {
 	reiser4_super_info_data *sbinfo = get_super_private(super);
+	int ret;
 
 	assert("nikita-3175", flags & BA_CAN_COMMIT);
 
 	/* Check the delete mutex already taken by us, we assume that
 	 * reading of machine word is atomic. */
 	if (sbinfo->delete_mutex_owner == current) {
-		if (reiser4_grab_space
-		    (count, (flags | BA_RESERVED) & ~BA_CAN_COMMIT)) {
+		ret = reiser4_grab_space (count, (flags | BA_RESERVED) & ~BA_CAN_COMMIT);
+
+		if (ret != 0) {
 			warning("zam-1003",
 				"nested call of grab_reserved fails count=(%llu)",
 				(unsigned long long)count);
 			reiser4_release_reserved(super);
-			return RETERR(-ENOSPC);
 		}
-		return 0;
-	}
+	} else {
+		ret = reiser4_grab_space(count, flags);
 
-	if (reiser4_grab_space(count, flags)) {
-		mutex_lock(&sbinfo->delete_mutex);
-		assert("nikita-2929", sbinfo->delete_mutex_owner == NULL);
-		sbinfo->delete_mutex_owner = current;
+		if (ret != 0) {
+			mutex_lock(&sbinfo->delete_mutex);
+			assert("nikita-2929", sbinfo->delete_mutex_owner == NULL);
+			sbinfo->delete_mutex_owner = current;
+			ret = reiser4_grab_space(count, flags | BA_RESERVED);
 
-		if (reiser4_grab_space(count, flags | BA_RESERVED)) {
-			warning("zam-833",
-				"reserved space is not enough (%llu)",
-				(unsigned long long)count);
-			reiser4_release_reserved(super);
-			return RETERR(-ENOSPC);
+			if (ret != 0) {
+				warning("zam-833",
+					"reserved space is not enough (%llu)",
+					(unsigned long long)count);
+				reiser4_release_reserved(super);
+			}
 		}
 	}
-	return 0;
+
+	return ret;
 }
 
 void reiser4_release_reserved(struct super_block *super)
-- 
2.0.4

[PATCH 2/3] reiser4: block_alloc: sanitize grab_enabled modifications.

[Ivan Shapovalov]

- move all grab_enabled checks and modifications into reiser4_grab_space();
- only disable grab if not BA_FORCE;
- do not re-enable grab before doing second attempt in BA_CAN_COMMIT sequence
  (feels hackish and is unneeded after the first change).

Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
---
 fs/reiser4/block_alloc.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
index 7f9f910..a30f7b9 100644
--- a/fs/reiser4/block_alloc.c
+++ b/fs/reiser4/block_alloc.c
@@ -270,12 +270,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
 
 	assert("vs-1276", ctx == get_current_context());
 
-	/* Do not grab anything on ro-mounted fs. */
-	if (rofs_super(ctx->super)) {
-		ctx->grab_enabled = 0;
-		return 0;
-	}
-
 	sbinfo = get_super_private(ctx->super);
 
 	spin_lock_reiser4_super(sbinfo);
@@ -300,9 +294,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
 
 	assert("nikita-2986", reiser4_check_block_counters(ctx->super));
 
-	/* disable grab space in current context */
-	ctx->grab_enabled = 0;
-
 unlock_and_ret:
 	spin_unlock_reiser4_super(sbinfo);
 
@@ -321,6 +312,12 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
 	if (!(flags & BA_FORCE) && !is_grab_enabled(ctx))
 		return 0;
 
+	/* Do not grab anything on ro-mounted fs. */
+	if (rofs_super(ctx->super)) {
+		ctx->grab_enabled = 0;
+		return 0;
+	}
+
 	ret = reiser4_grab(ctx, count, flags);
 	if (ret == -ENOSPC) {
 
@@ -328,10 +325,15 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
 		   present */
 		if (flags & BA_CAN_COMMIT) {
 			txnmgr_force_commit_all(ctx->super, 0);
-			ctx->grab_enabled = 1;
 			ret = reiser4_grab(ctx, count, flags);
 		}
 	}
+
+	if (!(flags & BA_FORCE) && (ret == 0)) {
+		/* disable grab space in current context */
+		ctx->grab_enabled = 0;
+	}
+
 	/*
 	 * allocation from reserved pool cannot fail. This is severe error.
 	 */
-- 
2.0.4

[PATCH 3/3] reiser4: do not mess with grab_enabled; instead, use BA_FORCE.

[Ivan Shapovalov]

Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
---
 fs/reiser4/plugin/file/file.c            | 12 ++++--------
 fs/reiser4/plugin/file/file_conversion.c |  3 +--
 fs/reiser4/plugin/file/tail_conversion.c | 11 +++++------
 fs/reiser4/plugin/item/extent_file_ops.c |  3 +--
 fs/reiser4/plugin/item/tail.c            |  3 +--
 fs/reiser4/safe_link.c                   |  4 +---
 6 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/fs/reiser4/plugin/file/file.c b/fs/reiser4/plugin/file/file.c
index 94029cd..b0bfa86 100644
--- a/fs/reiser4/plugin/file/file.c
+++ b/fs/reiser4/plugin/file/file.c
@@ -335,11 +335,10 @@ static int find_file_state(struct inode *inode, struct unix_file_info *uf_info)
  */
 static int reserve_partial_page(reiser4_tree * tree)
 {
-	grab_space_enable();
 	return reiser4_grab_reserved(reiser4_get_current_sb(),
 				     1 +
 				     2 * estimate_one_insert_into_item(tree),
-				     BA_CAN_COMMIT);
+				     BA_CAN_COMMIT | BA_FORCE);
 }
 
 /* estimate and reserve space needed to cut one item and update one stat data */
@@ -350,11 +349,10 @@ static int reserve_cut_iteration(reiser4_tree * tree)
 
 	assert("nikita-3172", lock_stack_isclean(get_current_lock_stack()));
 
-	grab_space_enable();
 	/* We need to double our estimate now that we can delete more than one
 	   node. */
 	return reiser4_grab_reserved(reiser4_get_current_sb(), estimate * 2,
-				     BA_CAN_COMMIT);
+				     BA_CAN_COMMIT | BA_FORCE);
 }
 
 int reiser4_update_file_size(struct inode *inode, loff_t new_size,
@@ -875,10 +873,9 @@ static int capture_page_and_create_extent(struct page *page)
 	/* page capture may require extent creation (if it does not exist yet)
 	   and stat data's update (number of blocks changes on extent
 	   creation) */
-	grab_space_enable();
 	result = reiser4_grab_space(2 * estimate_one_insert_into_item
 				    (reiser4_tree_by_inode(inode)),
-				    BA_CAN_COMMIT);
+				    BA_CAN_COMMIT | BA_FORCE);
 	if (likely(!result))
 		result = find_or_create_extent(page);
 
@@ -2421,9 +2418,8 @@ static int unpack(struct file *filp, struct inode *inode, int forever)
 
 		set_file_notail(inode);
 
-		grab_space_enable();
 		tograb = inode_file_plugin(inode)->estimate.update(inode);
-		result = reiser4_grab_space(tograb, BA_CAN_COMMIT);
+		result = reiser4_grab_space(tograb, BA_CAN_COMMIT | BA_FORCE);
 		result = reiser4_update_sd(inode);
 	}
 
diff --git a/fs/reiser4/plugin/file/file_conversion.c b/fs/reiser4/plugin/file/file_conversion.c
index 3ce4898..e01720f 100644
--- a/fs/reiser4/plugin/file/file_conversion.c
+++ b/fs/reiser4/plugin/file/file_conversion.c
@@ -388,14 +388,13 @@ static int reserve_cryptcompress2unixfile(struct inode *inode)
 	 *     5. possible update of stat-data
 	 *
 	 */
-	grab_space_enable();
 	return reiser4_grab_space
 		(2 * tree->height +
 		 unformatted_nodes  +
 		 unformatted_nodes * estimate_one_insert_into_item(tree) +
 		 1 + estimate_one_insert_item(tree) +
 		 inode_file_plugin(inode)->estimate.update(inode),
-		 BA_CAN_COMMIT);
+		 BA_CAN_COMMIT | BA_FORCE);
 }
 
 /**
diff --git a/fs/reiser4/plugin/file/tail_conversion.c b/fs/reiser4/plugin/file/tail_conversion.c
index 1763cb1..22f30fd 100644
--- a/fs/reiser4/plugin/file/tail_conversion.c
+++ b/fs/reiser4/plugin/file/tail_conversion.c
@@ -221,13 +221,13 @@ static int reserve_tail2extent_iteration(struct inode *inode)
 	 *     5. possible update of stat-data
 	 *
 	 */
-	grab_space_enable();
 	return reiser4_grab_space
 	    (2 * tree->height +
 	     TAIL2EXTENT_PAGE_NUM +
 	     TAIL2EXTENT_PAGE_NUM * estimate_one_insert_into_item(tree) +
 	     1 + estimate_one_insert_item(tree) +
-	     inode_file_plugin(inode)->estimate.update(inode), BA_CAN_COMMIT);
+	     inode_file_plugin(inode)->estimate.update(inode),
+	     BA_CAN_COMMIT | BA_FORCE);
 }
 
 /* clear stat data's flag indicating that conversion is being converted */
@@ -235,10 +235,9 @@ static int complete_conversion(struct inode *inode)
 {
 	int result;
 
-	grab_space_enable();
 	result =
 	    reiser4_grab_space(inode_file_plugin(inode)->estimate.update(inode),
-			       BA_CAN_COMMIT);
+			       BA_CAN_COMMIT | BA_FORCE);
 	if (result == 0) {
 		reiser4_inode_clr_flag(inode, REISER4_PART_MIXED);
 		result = reiser4_update_sd(inode);
@@ -552,12 +551,12 @@ static int reserve_extent2tail_iteration(struct inode *inode)
 	 *
 	 *     4. possible update of stat-data
 	 */
-	grab_space_enable();
 	return reiser4_grab_space
 	    (estimate_one_item_removal(tree) +
 	     estimate_insert_flow(tree->height) +
 	     1 + estimate_one_insert_item(tree) +
-	     inode_file_plugin(inode)->estimate.update(inode), BA_CAN_COMMIT);
+	     inode_file_plugin(inode)->estimate.update(inode),
+	     BA_CAN_COMMIT | BA_FORCE);
 }
 
 /* for every page of file: read page, cut part of extent pointing to this page,
diff --git a/fs/reiser4/plugin/item/extent_file_ops.c b/fs/reiser4/plugin/item/extent_file_ops.c
index cb57379..8086b5f 100644
--- a/fs/reiser4/plugin/item/extent_file_ops.c
+++ b/fs/reiser4/plugin/item/extent_file_ops.c
@@ -933,8 +933,7 @@ static int write_extent_reserve_space(struct inode *inode)
 	count = estimate_one_insert_item(tree) +
 		WRITE_GRANULARITY * (1 + estimate_one_insert_into_item(tree)) +
 		estimate_one_insert_item(tree);
-	grab_space_enable();
-	return reiser4_grab_space(count, 0 /* flags */);
+	return reiser4_grab_space(count, BA_FORCE);
 }
 
 /*
diff --git a/fs/reiser4/plugin/item/tail.c b/fs/reiser4/plugin/item/tail.c
index 7f4a101..67cb4ba 100644
--- a/fs/reiser4/plugin/item/tail.c
+++ b/fs/reiser4/plugin/item/tail.c
@@ -603,8 +603,7 @@ static int write_extent_reserve_space(struct inode *inode)
 	count = estimate_one_insert_item(tree) +
 		estimate_insert_flow(tree->height) +
 		estimate_one_insert_item(tree);
-	grab_space_enable();
-	return reiser4_grab_space(count, 0 /* flags */);
+	return reiser4_grab_space(count, BA_FORCE);
 }
 
 #define PAGE_PER_FLOW 4
diff --git a/fs/reiser4/safe_link.c b/fs/reiser4/safe_link.c
index d59f6f0..f4cff69 100644
--- a/fs/reiser4/safe_link.c
+++ b/fs/reiser4/safe_link.c
@@ -130,13 +130,11 @@ int safe_link_grab(reiser4_tree * tree, reiser4_ba_flags_t flags)
 {
 	int result;
 
-	grab_space_enable();
 	/* The sbinfo->delete_mutex can be taken here.
 	 * safe_link_release() should be called before leaving reiser4
 	 * context. */
 	result =
-	    reiser4_grab_reserved(tree->super, safe_link_tograb(tree), flags);
-	grab_space_enable();
+	    reiser4_grab_reserved(tree->super, safe_link_tograb(tree), flags | BA_FORCE);
 	return result;
 }
 
-- 
2.0.4

Re: [PATCH 1/3] reiser4: block_alloc: improve error handling in reiser4_grab_reserved().

[Edward Shishkin]

On 08/18/2014 04:14 PM, Ivan Shapovalov wrote:
> In case of error in reiser4_grab_space(), return the original error code,
> not -ENOSPC unconditionally.
>
> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>   fs/reiser4/block_alloc.c | 35 +++++++++++++++++++----------------
>   1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
> index 324b11c..7f9f910 100644
> --- a/fs/reiser4/block_alloc.c
> +++ b/fs/reiser4/block_alloc.c
> @@ -360,37 +360,40 @@ int reiser4_grab_reserved(struct super_block *super,
>   			  __u64 count, reiser4_ba_flags_t flags)
>   {
>   	reiser4_super_info_data *sbinfo = get_super_private(super);
> +	int ret;
>   
>   	assert("nikita-3175", flags & BA_CAN_COMMIT);
>   
>   	/* Check the delete mutex already taken by us, we assume that
>   	 * reading of machine word is atomic. */
>   	if (sbinfo->delete_mutex_owner == current) {
> -		if (reiser4_grab_space
> -		    (count, (flags | BA_RESERVED) & ~BA_CAN_COMMIT)) {
> +		ret = reiser4_grab_space (count, (flags | BA_RESERVED) & ~BA_CAN_COMMIT);
> +
> +		if (ret != 0) {
>   			warning("zam-1003",
>   				"nested call of grab_reserved fails count=(%llu)",
>   				(unsigned long long)count);
>   			reiser4_release_reserved(super);
> -			return RETERR(-ENOSPC);
>   		}
> -		return 0;
> -	}
> +	} else {
> +		ret = reiser4_grab_space(count, flags);
>   
> -	if (reiser4_grab_space(count, flags)) {
> -		mutex_lock(&sbinfo->delete_mutex);
> -		assert("nikita-2929", sbinfo->delete_mutex_owner == NULL);
> -		sbinfo->delete_mutex_owner = current;
> +		if (ret != 0) {
> +			mutex_lock(&sbinfo->delete_mutex);
> +			assert("nikita-2929", sbinfo->delete_mutex_owner == NULL);
> +			sbinfo->delete_mutex_owner = current;
> +			ret = reiser4_grab_space(count, flags | BA_RESERVED);
>   
> -		if (reiser4_grab_space(count, flags | BA_RESERVED)) {
> -			warning("zam-833",
> -				"reserved space is not enough (%llu)",
> -				(unsigned long long)count);
> -			reiser4_release_reserved(super);
> -			return RETERR(-ENOSPC);
> +			if (ret != 0) {
> +				warning("zam-833",
> +					"reserved space is not enough (%llu)",
> +					(unsigned long long)count);
> +				reiser4_release_reserved(super);
> +			}
>   		}
>   	}
> -	return 0;
> +
> +	return ret;
>   }
>   
>   void reiser4_release_reserved(struct super_block *super)

OK

Re: [PATCH 2/3] reiser4: block_alloc: sanitize grab_enabled modifications.

[Edward Shishkin]

On 08/18/2014 04:14 PM, Ivan Shapovalov wrote:
> - move all grab_enabled checks and modifications into reiser4_grab_space();
> - only disable grab if not BA_FORCE;
> - do not re-enable grab before doing second attempt in BA_CAN_COMMIT sequence
>    (feels hackish and is unneeded after the first change).
>
> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>   fs/reiser4/block_alloc.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
> index 7f9f910..a30f7b9 100644
> --- a/fs/reiser4/block_alloc.c
> +++ b/fs/reiser4/block_alloc.c
> @@ -270,12 +270,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
>   
>   	assert("vs-1276", ctx == get_current_context());
>   
> -	/* Do not grab anything on ro-mounted fs. */
> -	if (rofs_super(ctx->super)) {
> -		ctx->grab_enabled = 0;
> -		return 0;
> -	}
> -
>   	sbinfo = get_super_private(ctx->super);
>   
>   	spin_lock_reiser4_super(sbinfo);
> @@ -300,9 +294,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
>   
>   	assert("nikita-2986", reiser4_check_block_counters(ctx->super));
>   
> -	/* disable grab space in current context */
> -	ctx->grab_enabled = 0;
> -
>   unlock_and_ret:
>   	spin_unlock_reiser4_super(sbinfo);
>   
> @@ -321,6 +312,12 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
>   	if (!(flags & BA_FORCE) && !is_grab_enabled(ctx))
>   		return 0;
>   
> +	/* Do not grab anything on ro-mounted fs. */
> +	if (rofs_super(ctx->super)) {
> +		ctx->grab_enabled = 0;
> +		return 0;
> +	}
> +
>   	ret = reiser4_grab(ctx, count, flags);
>   	if (ret == -ENOSPC) {
>   
> @@ -328,10 +325,15 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
>   		   present */
>   		if (flags & BA_CAN_COMMIT) {
>   			txnmgr_force_commit_all(ctx->super, 0);
> -			ctx->grab_enabled = 1;
>   			ret = reiser4_grab(ctx, count, flags);
>   		}
>   	}
> +
> +	if (!(flags & BA_FORCE) && (ret == 0)) {


Hmm, but we disabled it unconditionally in reiser4_grab().
Not sure, if this is equivalent...


> +		/* disable grab space in current context */
> +		ctx->grab_enabled = 0;
> +	}

Re: [PATCH 3/3] reiser4: do not mess with grab_enabled; instead, use BA_FORCE.

[Edward Shishkin]

On 08/18/2014 04:14 PM, Ivan Shapovalov wrote:
> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>   fs/reiser4/plugin/file/file.c            | 12 ++++--------
>   fs/reiser4/plugin/file/file_conversion.c |  3 +--
>   fs/reiser4/plugin/file/tail_conversion.c | 11 +++++------
>   fs/reiser4/plugin/item/extent_file_ops.c |  3 +--
>   fs/reiser4/plugin/item/tail.c            |  3 +--
>   fs/reiser4/safe_link.c                   |  4 +---
>   6 files changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/fs/reiser4/plugin/file/file.c b/fs/reiser4/plugin/file/file.c
> index 94029cd..b0bfa86 100644
> --- a/fs/reiser4/plugin/file/file.c
> +++ b/fs/reiser4/plugin/file/file.c
> @@ -335,11 +335,10 @@ static int find_file_state(struct inode *inode, struct unix_file_info *uf_info)
>    */
>   static int reserve_partial_page(reiser4_tree * tree)
>   {
> -	grab_space_enable();
>   	return reiser4_grab_reserved(reiser4_get_current_sb(),
>   				     1 +
>   				     2 * estimate_one_insert_into_item(tree),
> -				     BA_CAN_COMMIT);
> +				     BA_CAN_COMMIT | BA_FORCE);
>   }
>   
>   /* estimate and reserve space needed to cut one item and update one stat data */
> @@ -350,11 +349,10 @@ static int reserve_cut_iteration(reiser4_tree * tree)
>   
>   	assert("nikita-3172", lock_stack_isclean(get_current_lock_stack()));
>   
> -	grab_space_enable();
>   	/* We need to double our estimate now that we can delete more than one
>   	   node. */
>   	return reiser4_grab_reserved(reiser4_get_current_sb(), estimate * 2,
> -				     BA_CAN_COMMIT);
> +				     BA_CAN_COMMIT | BA_FORCE);
>   }
>   
>   int reiser4_update_file_size(struct inode *inode, loff_t new_size,
> @@ -875,10 +873,9 @@ static int capture_page_and_create_extent(struct page *page)
>   	/* page capture may require extent creation (if it does not exist yet)
>   	   and stat data's update (number of blocks changes on extent
>   	   creation) */
> -	grab_space_enable();
>   	result = reiser4_grab_space(2 * estimate_one_insert_into_item
>   				    (reiser4_tree_by_inode(inode)),
> -				    BA_CAN_COMMIT);
> +				    BA_CAN_COMMIT | BA_FORCE);
>   	if (likely(!result))
>   		result = find_or_create_extent(page);
>   
> @@ -2421,9 +2418,8 @@ static int unpack(struct file *filp, struct inode *inode, int forever)
>   
>   		set_file_notail(inode);
>   
> -		grab_space_enable();
>   		tograb = inode_file_plugin(inode)->estimate.update(inode);
> -		result = reiser4_grab_space(tograb, BA_CAN_COMMIT);
> +		result = reiser4_grab_space(tograb, BA_CAN_COMMIT | BA_FORCE);
>   		result = reiser4_update_sd(inode);
>   	}
>   
> diff --git a/fs/reiser4/plugin/file/file_conversion.c b/fs/reiser4/plugin/file/file_conversion.c
> index 3ce4898..e01720f 100644
> --- a/fs/reiser4/plugin/file/file_conversion.c
> +++ b/fs/reiser4/plugin/file/file_conversion.c
> @@ -388,14 +388,13 @@ static int reserve_cryptcompress2unixfile(struct inode *inode)
>   	 *     5. possible update of stat-data
>   	 *
>   	 */
> -	grab_space_enable();
>   	return reiser4_grab_space
>   		(2 * tree->height +
>   		 unformatted_nodes  +
>   		 unformatted_nodes * estimate_one_insert_into_item(tree) +
>   		 1 + estimate_one_insert_item(tree) +
>   		 inode_file_plugin(inode)->estimate.update(inode),
> -		 BA_CAN_COMMIT);
> +		 BA_CAN_COMMIT | BA_FORCE);
>   }
>   
>   /**
> diff --git a/fs/reiser4/plugin/file/tail_conversion.c b/fs/reiser4/plugin/file/tail_conversion.c
> index 1763cb1..22f30fd 100644
> --- a/fs/reiser4/plugin/file/tail_conversion.c
> +++ b/fs/reiser4/plugin/file/tail_conversion.c
> @@ -221,13 +221,13 @@ static int reserve_tail2extent_iteration(struct inode *inode)
>   	 *     5. possible update of stat-data
>   	 *
>   	 */
> -	grab_space_enable();
>   	return reiser4_grab_space
>   	    (2 * tree->height +
>   	     TAIL2EXTENT_PAGE_NUM +
>   	     TAIL2EXTENT_PAGE_NUM * estimate_one_insert_into_item(tree) +
>   	     1 + estimate_one_insert_item(tree) +
> -	     inode_file_plugin(inode)->estimate.update(inode), BA_CAN_COMMIT);
> +	     inode_file_plugin(inode)->estimate.update(inode),
> +	     BA_CAN_COMMIT | BA_FORCE);
>   }
>   
>   /* clear stat data's flag indicating that conversion is being converted */
> @@ -235,10 +235,9 @@ static int complete_conversion(struct inode *inode)
>   {
>   	int result;
>   
> -	grab_space_enable();
>   	result =
>   	    reiser4_grab_space(inode_file_plugin(inode)->estimate.update(inode),
> -			       BA_CAN_COMMIT);
> +			       BA_CAN_COMMIT | BA_FORCE);
>   	if (result == 0) {
>   		reiser4_inode_clr_flag(inode, REISER4_PART_MIXED);
>   		result = reiser4_update_sd(inode);
> @@ -552,12 +551,12 @@ static int reserve_extent2tail_iteration(struct inode *inode)
>   	 *
>   	 *     4. possible update of stat-data
>   	 */
> -	grab_space_enable();
>   	return reiser4_grab_space
>   	    (estimate_one_item_removal(tree) +
>   	     estimate_insert_flow(tree->height) +
>   	     1 + estimate_one_insert_item(tree) +
> -	     inode_file_plugin(inode)->estimate.update(inode), BA_CAN_COMMIT);
> +	     inode_file_plugin(inode)->estimate.update(inode),
> +	     BA_CAN_COMMIT | BA_FORCE);
>   }
>   
>   /* for every page of file: read page, cut part of extent pointing to this page,
> diff --git a/fs/reiser4/plugin/item/extent_file_ops.c b/fs/reiser4/plugin/item/extent_file_ops.c
> index cb57379..8086b5f 100644
> --- a/fs/reiser4/plugin/item/extent_file_ops.c
> +++ b/fs/reiser4/plugin/item/extent_file_ops.c
> @@ -933,8 +933,7 @@ static int write_extent_reserve_space(struct inode *inode)
>   	count = estimate_one_insert_item(tree) +
>   		WRITE_GRANULARITY * (1 + estimate_one_insert_into_item(tree)) +
>   		estimate_one_insert_item(tree);
> -	grab_space_enable();
> -	return reiser4_grab_space(count, 0 /* flags */);
> +	return reiser4_grab_space(count, BA_FORCE);
>   }
>   
>   /*
> diff --git a/fs/reiser4/plugin/item/tail.c b/fs/reiser4/plugin/item/tail.c
> index 7f4a101..67cb4ba 100644
> --- a/fs/reiser4/plugin/item/tail.c
> +++ b/fs/reiser4/plugin/item/tail.c
> @@ -603,8 +603,7 @@ static int write_extent_reserve_space(struct inode *inode)
>   	count = estimate_one_insert_item(tree) +
>   		estimate_insert_flow(tree->height) +
>   		estimate_one_insert_item(tree);
> -	grab_space_enable();
> -	return reiser4_grab_space(count, 0 /* flags */);
> +	return reiser4_grab_space(count, BA_FORCE);
>   }
>   
>   #define PAGE_PER_FLOW 4
> diff --git a/fs/reiser4/safe_link.c b/fs/reiser4/safe_link.c
> index d59f6f0..f4cff69 100644
> --- a/fs/reiser4/safe_link.c
> +++ b/fs/reiser4/safe_link.c
> @@ -130,13 +130,11 @@ int safe_link_grab(reiser4_tree * tree, reiser4_ba_flags_t flags)
>   {
>   	int result;
>   
> -	grab_space_enable();
>   	/* The sbinfo->delete_mutex can be taken here.
>   	 * safe_link_release() should be called before leaving reiser4
>   	 * context. */
>   	result =
> -	    reiser4_grab_reserved(tree->super, safe_link_tograb(tree), flags);
> -	grab_space_enable();
> +	    reiser4_grab_reserved(tree->super, safe_link_tograb(tree), flags | BA_FORCE);
>   	return result;
>   }
>   

OK

Re: [PATCH 2/3] reiser4: block_alloc: sanitize grab_enabled modifications.

[Ivan Shapovalov]

[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]

On Saturday 18 October 2014 at 19:47:29, Edward Shishkin wrote:	
> On 08/18/2014 04:14 PM, Ivan Shapovalov wrote:
> > - move all grab_enabled checks and modifications into reiser4_grab_space();
> > - only disable grab if not BA_FORCE;
> > - do not re-enable grab before doing second attempt in BA_CAN_COMMIT sequence
> >    (feels hackish and is unneeded after the first change).
> >
> > Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> > ---
> >   fs/reiser4/block_alloc.c | 22 ++++++++++++----------
> >   1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
> > index 7f9f910..a30f7b9 100644
> > --- a/fs/reiser4/block_alloc.c
> > +++ b/fs/reiser4/block_alloc.c
> > @@ -270,12 +270,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
> >   
> >   	assert("vs-1276", ctx == get_current_context());
> >   
> > -	/* Do not grab anything on ro-mounted fs. */
> > -	if (rofs_super(ctx->super)) {
> > -		ctx->grab_enabled = 0;
> > -		return 0;
> > -	}
> > -
> >   	sbinfo = get_super_private(ctx->super);
> >   
> >   	spin_lock_reiser4_super(sbinfo);
> > @@ -300,9 +294,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
> >   
> >   	assert("nikita-2986", reiser4_check_block_counters(ctx->super));
> >   
> > -	/* disable grab space in current context */
> > -	ctx->grab_enabled = 0;
> > -
> >   unlock_and_ret:
> >   	spin_unlock_reiser4_super(sbinfo);
> >   
> > @@ -321,6 +312,12 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
> >   	if (!(flags & BA_FORCE) && !is_grab_enabled(ctx))
> >   		return 0;
> >   
> > +	/* Do not grab anything on ro-mounted fs. */
> > +	if (rofs_super(ctx->super)) {
> > +		ctx->grab_enabled = 0;
> > +		return 0;
> > +	}
> > +
> >   	ret = reiser4_grab(ctx, count, flags);
> >   	if (ret == -ENOSPC) {
> >   
> > @@ -328,10 +325,15 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
> >   		   present */
> >   		if (flags & BA_CAN_COMMIT) {
> >   			txnmgr_force_commit_all(ctx->super, 0);
> > -			ctx->grab_enabled = 1;
> >   			ret = reiser4_grab(ctx, count, flags);
> >   		}
> >   	}
> > +
> > +	if (!(flags & BA_FORCE) && (ret == 0)) {
> 
> 
> Hmm, but we disabled it unconditionally in reiser4_grab().
> Not sure, if this is equivalent...

Yes, you are right. I can't recall any particular justification for this
change... so it should probably say

    if (ret == 0) {

-- 
Ivan Shapovalov / intelfx /

> > +		/* disable grab space in current context */
> > +		ctx->grab_enabled = 0;
> > +	}
> 

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [PATCH 2/3] reiser4: block_alloc: sanitize grab_enabled modifications.

[Ivan Shapovalov]

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

On Monday 20 October 2014 at 13:43:50, Ivan Shapovalov wrote:	
> On Saturday 18 October 2014 at 19:47:29, Edward Shishkin wrote:	
> > On 08/18/2014 04:14 PM, Ivan Shapovalov wrote:
> > > - move all grab_enabled checks and modifications into reiser4_grab_space();
> > > - only disable grab if not BA_FORCE;
> > > - do not re-enable grab before doing second attempt in BA_CAN_COMMIT sequence
> > >    (feels hackish and is unneeded after the first change).
> > >
> > > Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> > > ---
> > >   fs/reiser4/block_alloc.c | 22 ++++++++++++----------
> > >   1 file changed, 12 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
> > > index 7f9f910..a30f7b9 100644
> > > --- a/fs/reiser4/block_alloc.c
> > > +++ b/fs/reiser4/block_alloc.c
> > > @@ -270,12 +270,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
> > >   
> > >   	assert("vs-1276", ctx == get_current_context());
> > >   
> > > -	/* Do not grab anything on ro-mounted fs. */
> > > -	if (rofs_super(ctx->super)) {
> > > -		ctx->grab_enabled = 0;
> > > -		return 0;
> > > -	}
> > > -
> > >   	sbinfo = get_super_private(ctx->super);
> > >   
> > >   	spin_lock_reiser4_super(sbinfo);
> > > @@ -300,9 +294,6 @@ reiser4_grab(reiser4_context * ctx, __u64 count, reiser4_ba_flags_t flags)
> > >   
> > >   	assert("nikita-2986", reiser4_check_block_counters(ctx->super));
> > >   
> > > -	/* disable grab space in current context */
> > > -	ctx->grab_enabled = 0;
> > > -
> > >   unlock_and_ret:
> > >   	spin_unlock_reiser4_super(sbinfo);
> > >   
> > > @@ -321,6 +312,12 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
> > >   	if (!(flags & BA_FORCE) && !is_grab_enabled(ctx))
> > >   		return 0;
> > >   
> > > +	/* Do not grab anything on ro-mounted fs. */
> > > +	if (rofs_super(ctx->super)) {
> > > +		ctx->grab_enabled = 0;
> > > +		return 0;
> > > +	}
> > > +
> > >   	ret = reiser4_grab(ctx, count, flags);
> > >   	if (ret == -ENOSPC) {
> > >   
> > > @@ -328,10 +325,15 @@ int reiser4_grab_space(__u64 count, reiser4_ba_flags_t flags)
> > >   		   present */
> > >   		if (flags & BA_CAN_COMMIT) {
> > >   			txnmgr_force_commit_all(ctx->super, 0);
> > > -			ctx->grab_enabled = 1;
> > >   			ret = reiser4_grab(ctx, count, flags);
> > >   		}
> > >   	}
> > > +
> > > +	if (!(flags & BA_FORCE) && (ret == 0)) {
> > 
> > 
> > Hmm, but we disabled it unconditionally in reiser4_grab().
> > Not sure, if this is equivalent...
> 
> Yes, you are right. I can't recall any particular justification for this
> change...

I've apparently "rediscovered" the justification. See my response to
PATCHv2 2/3...

-- 
Ivan Shapovalov / intelfx /

> so it should probably say
> 
>     if (ret == 0) {
> 
> > > +		/* disable grab space in current context */
> > > +		ctx->grab_enabled = 0;
> > > +	}
> > 

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 213 bytes --]