Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sun, 17 Dec 2017 21:53:48 +0100
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: max_keys_per_crypt tuning

On Sun, Nov 12, 2017 at 07:19:13PM +0100, Solar Designer wrote:
> Then, OMP_SCALE - the additional scaling factor to compensate for OpenMP
> thread desynchronization (to reduce the relative wait times for getting
> the threads back in sync) - should only be applied when actually running
> more than 1 thread.  Currently we usually also apply it for the 1 thread
> case, which I now think is wrong.  We just got this fixed in
> multibit_fmt_plug.c with commit 00655ad63366.  We need changes like this:
> 
> -       self->params.min_keys_per_crypt *= omp_t;
> -       omp_t *= OMP_SCALE;
> -       self->params.max_keys_per_crypt *= omp_t;
> +       if (omp_t > 1) {
> +               self->params.min_keys_per_crypt *= omp_t;
> +               omp_t *= OMP_SCALE;
> +               self->params.max_keys_per_crypt *= omp_t;
> +       }

Dhiru has made these changes now.  Thanks!

https://github.com/magnumripper/JohnTheRipper/pull/3013

And I've just realized that we should also rename the "omp_t" variable,
since its current name is confusing (the _t suffix is commonly used on
typedef's), reserved by POSIX (all *_t are), and isn't comfortably
unlikely not to clash with whatever a future version of POSIX might
define.  In the core tree, I happen to have called a similar variable
simply "n".  That's in the *_init() functions in DES_bs.c and MD5_std.c.
If we'd like a more descriptive name (such as in one format where this
needs to be a global static variable), it can be e.g. "threads".

> We might also want to move common code like this into a function (maybe
> in formats.c) that we'd use from formats' init(), instead of keeping it
> as duplicate code like we have now.

I think the function should accept "self" and "scale" (we'll pass
OMP_SCALE) as parameters.  It could be called unconditionally (including
on non-OpenMP), and would have the "#ifdef _OPENMP" inside.

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.