lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 23 Nov 2020 11:55:29 -0800
From:   harshad shirwadkar <harshadshirwadkar@...il.com>
To:     Saranya Muruganandam <saranyamohan@...gle.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Li Xi <lixi@....com>, Wang Shilong <wshilong@....com>
Subject: Re: [RFC PATCH v3 02/61] e2fsck: copy context when using multi-thread fsck

Thanks for the patch. I noticed that this patch doesn't compile. It's
because it needs "global_ctx" to be present in the e2fsck_struct but
it gets added later in the patch series in the patch 1e1b0216
("LU-8465 e2fsck: open io-channel when copying fs"). Given that this
and also a couple of other patches need global_ctx, should we split
1e1b0216 ("LU-8465 e2fsck: open io-channel when copying fs") should we
add global_ctx in this patch or as a separate patch before this?


On Wed, Nov 18, 2020 at 7:43 AM Saranya Muruganandam
<saranyamohan@...gle.com> wrote:
>
> From: Li Xi <lixi@....com>
>
> This patch only copy the context to a new one when -m is enabled.
> It doesn't actually start any thread. When pass1 test finishes,
> the new context is copied back to the original context.
>
> Since the signal handler only changes the original context, so
> add global_ctx in "struct e2fsck_struct" and use that to check
> whether there is any signal of canceling.
>
> This patch handles the long jump properly so that all the existing
> tests can be passed even the context has been copied. Otherwise,
> test f_expisize_ea_del would fail when aborting.
>
> Signed-off-by: Li Xi <lixi@....com>
> Signed-off-by: Wang Shilong <wshilong@....com>
> Signed-off-by: Saranya Muruganandam <saranyamohan@...gle.com>
> ---
>  e2fsck/pass1.c | 114 +++++++++++++++++++++++++++++++++++++++++++++----
>  e2fsck/unix.c  |   1 +
>  2 files changed, 107 insertions(+), 8 deletions(-)
>
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 8eecd958..64d237d3 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -1144,7 +1144,22 @@ static int quota_inum_is_reserved(ext2_filsys fs, ext2_ino_t ino)
>         return 0;
>  }
>
> -void e2fsck_pass1(e2fsck_t ctx)
> +static int e2fsck_should_abort(e2fsck_t ctx)
> +{
> +       e2fsck_t global_ctx;
> +
> +       if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +               return 1;
> +
> +       if (ctx->global_ctx) {
> +               global_ctx = ctx->global_ctx;
> +               if (global_ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +                       return 1;
> +       }
> +       return 0;
> +}
> +
> +void e2fsck_pass1_thread(e2fsck_t ctx)
>  {
>         int     i;
>         __u64   max_sizes;
> @@ -1360,7 +1375,7 @@ void e2fsck_pass1(e2fsck_t ctx)
>                 if (ino > ino_threshold)
>                         pass1_readahead(ctx, &ra_group, &ino_threshold);
>                 ehandler_operation(old_op);
> -               if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +               if (e2fsck_should_abort(ctx))
>                         goto endit;
>                 if (pctx.errcode == EXT2_ET_BAD_BLOCK_IN_INODE_TABLE) {
>                         /*
> @@ -1955,7 +1970,7 @@ void e2fsck_pass1(e2fsck_t ctx)
>                 if (process_inode_count >= ctx->process_inode_size) {
>                         process_inodes(ctx, block_buf);
>
> -                       if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +                       if (e2fsck_should_abort(ctx))
>                                 goto endit;
>                 }
>         }
> @@ -2068,6 +2083,89 @@ endit:
>         else
>                 ctx->invalid_bitmaps++;
>  }
> +
> +static errcode_t e2fsck_pass1_thread_prepare(e2fsck_t global_ctx, e2fsck_t *thread_ctx)
> +{
> +       errcode_t       retval;
> +       e2fsck_t        thread_context;
> +
> +       retval = ext2fs_get_mem(sizeof(struct e2fsck_struct), &thread_context);
> +       if (retval) {
> +               com_err(global_ctx->program_name, retval, "while allocating memory");
> +               return retval;
> +       }
> +       memcpy(thread_context, global_ctx, sizeof(struct e2fsck_struct));
> +       thread_context->fs->priv_data = thread_context;
> +       thread_context->global_ctx = global_ctx;
> +
> +       *thread_ctx = thread_context;
> +       return 0;
> +}
> +
> +static int e2fsck_pass1_thread_join(e2fsck_t global_ctx, e2fsck_t thread_ctx)
> +{
> +       int     flags = global_ctx->flags;
> +#ifdef HAVE_SETJMP_H
> +       jmp_buf old_jmp;
> +
> +       memcpy(old_jmp, global_ctx->abort_loc, sizeof(jmp_buf));
> +#endif
> +       memcpy(global_ctx, thread_ctx, sizeof(struct e2fsck_struct));
> +#ifdef HAVE_SETJMP_H
> +       memcpy(global_ctx->abort_loc, old_jmp, sizeof(jmp_buf));
> +#endif
> +       /* Keep the global singal flags*/
> +       global_ctx->flags |= (flags & E2F_FLAG_SIGNAL_MASK) |
> +                            (global_ctx->flags & E2F_FLAG_SIGNAL_MASK);
> +
> +       global_ctx->fs->priv_data = global_ctx;
> +       ext2fs_free_mem(&thread_ctx);
> +       return 0;
> +}
> +
> +void e2fsck_pass1_multithread(e2fsck_t ctx)
> +{
> +       errcode_t       retval;
> +       e2fsck_t        thread_ctx;
> +
> +       retval = e2fsck_pass1_thread_prepare(ctx, &thread_ctx);
> +       if (retval) {
> +               com_err(ctx->program_name, 0,
> +                       _("while preparing pass1 thread\n"));
> +               ctx->flags |= E2F_FLAG_ABORT;
> +               return;
> +       }
> +
> +#ifdef HAVE_SETJMP_H
> +       /*
> +        * When fatal_error() happens, jump to here. The thread
> +        * context's flags will be saved, but its abort_loc will
> +        * be overwritten by original jump buffer for the later
> +        * tests.
> +        */
> +       if (setjmp(thread_ctx->abort_loc)) {
> +               thread_ctx->flags &= ~E2F_FLAG_SETJMP_OK;
> +               e2fsck_pass1_thread_join(ctx, thread_ctx);
> +               return;
> +       }
> +       thread_ctx->flags |= E2F_FLAG_SETJMP_OK;
> +#endif
> +
> +       e2fsck_pass1_thread(thread_ctx);
> +       retval = e2fsck_pass1_thread_join(ctx, thread_ctx);
> +       if (retval) {
> +               com_err(ctx->program_name, 0,
> +                       _("while joining pass1 thread\n"));
> +               ctx->flags |= E2F_FLAG_ABORT;
> +               return;
> +       }
> +}
> +
> +void e2fsck_pass1(e2fsck_t ctx)
> +{
> +       e2fsck_pass1_multithread(ctx);
> +}
> +
>  #undef FINISH_INODE_LOOP
>
>  /*
> @@ -2130,7 +2228,7 @@ static void process_inodes(e2fsck_t ctx, char *block_buf)
>                 ehandler_operation(buf);
>                 check_blocks(ctx, &pctx, block_buf,
>                              &inodes_to_process[i].ea_ibody_quota);
> -               if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +               if (e2fsck_should_abort(ctx))
>                         break;
>         }
>         ctx->stashed_inode = old_stashed_inode;
> @@ -3300,7 +3398,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
>         inlinedata_fs = ext2fs_has_feature_inline_data(ctx->fs->super);
>
>         if (check_ext_attr(ctx, pctx, block_buf, &ea_block_quota)) {
> -               if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +               if (e2fsck_should_abort(ctx))
>                         goto out;
>                 pb.num_blocks += EXT2FS_B2C(ctx->fs, ea_block_quota.blocks);
>         }
> @@ -3355,7 +3453,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
>         }
>         end_problem_latch(ctx, PR_LATCH_BLOCK);
>         end_problem_latch(ctx, PR_LATCH_TOOBIG);
> -       if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +       if (e2fsck_should_abort(ctx))
>                 goto out;
>         if (pctx->errcode)
>                 fix_problem(ctx, PR_1_BLOCK_ITERATE, pctx);
> @@ -3836,7 +3934,7 @@ static int process_bad_block(ext2_filsys fs,
>                                 *block_nr = 0;
>                                 return BLOCK_CHANGED;
>                         }
> -                       if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +                       if (e2fsck_should_abort(ctx))
>                                 return BLOCK_ABORT;
>                 } else
>                         mark_block_used(ctx, blk);
> @@ -3933,7 +4031,7 @@ static int process_bad_block(ext2_filsys fs,
>                         *block_nr = 0;
>                         return BLOCK_CHANGED;
>                 }
> -               if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> +               if (e2fsck_should_abort(ctx))
>                         return BLOCK_ABORT;
>                 return 0;
>         }
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 051b31a5..42f616e2 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -1445,6 +1445,7 @@ int main (int argc, char *argv[])
>         }
>         reserve_stdio_fds();
>
> +       ctx->global_ctx = NULL;
>         set_up_logging(ctx);
>         if (ctx->logf) {
>                 int i;
> --
> 2.29.2.299.gdc1121823c-goog
>

Powered by blists - more mailing lists