On 11/16/2014 10:53 AM, Edward Shishkin wrote:
On 11/16/2014 06:38 AM, Ivan Shapovalov wrote:
On Thursday 23 October 2014 at 11:18:04, Ivan Shapovalov wrote:
- check and modify ctx->grab_enabled in reiser4_grab_space(), not
reiser4_grab()
- do not re-enable grab before doing second attempt in BA_CAN_COMMIT
sequence
(it is unneeded given the first change)
...testing has revealed that this is wrong. Please see below.
Signed-off-by: Ivan Shapovalov <intelfx100@xxxxxxxxx>
---
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..9339de7 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);
}
Here, txnmgr_force_commit_all() performs space grabbing (with
BA_FORCE flag)
and as such indirectly sets ctx->grab_enabled to 0.
Initial problem.
Situation: ctx->grab_enabled == 0
Call: reiser4_grab_space(toomany, BA_FORCE | BA_CAN_COMMIT)
* first reiser4_grab() returns -ENOSPC
* txnmgr_force_commit_all() is called
* ctx->grab_enabled is set to 1 by us
* second reiser4_grab() returns -ENOSPC as well
Result: ctx->grab_enabled == 1
Problem with this patch.
Situation: ctx->grab_enabled == 1
Call: reiser4_grab_space(toomany, BA_CAN_COMMIT)
* first reiser4_grab() returns -ENOSPC
* txnmgr_force_commit_all() is called and sets ctx->grab_enabled to 0
* second reiser4_grab() returns -ENOSPC as well
Result: no grab performed, but ctx->grab_enabled == 0.
Solution 1:
save and restore ctx->grab_enabled across call to
txnmgr_force_commit_all();
Solution 2:
make txnmgr_force_commit_all() not touch ctx->grab_enabled,
i. e. set ctx->grab_enabled to 0 only if !BA_FORCE.
What would you suggest?
Could you please remind, what problem
this patch series (sanitize grab_enabled) fixes?
To be clear, I forgot why I included this patch series.
What is the system crash, or disk space leak, or something else..?
Edward.
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
