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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Linux File System Development]     [Linux BTRFS]     [Linux NFS]     [Linux Filesystems]     [Ext4 Filesystem]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Resources]

  Powered by Linux