linux-kernel - Re: [PATCH 06/13] async_tx: add support for asynchronous GF multiplication

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 linux-cve-announce 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]

Message-ID: <e9c3a7c20903191020k4d0169e3oc9749103a711f3e3@mail.gmail.com>
Date:	Thu, 19 Mar 2009 10:20:19 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	"H. Peter Anvin" <h.peter.anvin@...el.com>
Cc:	linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
	neilb@...e.de, maciej.sosnowski@...el.com,
	Ilya Yanok <yanok@...raft.com>, Yuri Tikhonov <yur@...raft.com>
Subject: Re: [PATCH 06/13] async_tx: add support for asynchronous GF 
	multiplication

On Thu, Mar 19, 2009 at 9:06 AM, H. Peter Anvin <h.peter.anvin@...el.com> wrote:
> Dan Williams wrote:
>> + * @scfs: array of source coefficients used in GF-multiplication
>
> Array of source coefficients?  Are you doing a vector-vector
> multiplication here?
>
> Given this code:
>
>>       for (d = 0; d < len; d++) {
>> +             wq = wp = ptrs[0][d];
>> +             for (z = 1; z < src_cnt; z++) {
>> +                     wd = ptrs[z][d];
>> +                     wp ^= wd;
>> +                     wq ^= raid6_gfmul[scfs[z]][wd];
>> +             }
>
> ... it kinds of looks like that.
>
> This is really quite expensive!  The whole point of the restore code
> that exists is that we never do a two-dimensional lookup, instead
> caching a pointer to the multiplication table that we intend to use,
> because the RAID-6 code only ever contains scalar-vector multiplications.
>
> I really don't get this, and I think it's broken.

Something is broken if we take this path.  This routine could stand to
have a WARN_ONCE(), because if it is ever called there is something is
wrong with the raid6 offload driver.  The intent is that the
asynchronous recovery routines will perform an early check to see if
the recovery can be offloaded (with a series of calls to
async_xor/async_pq).  If not we fall back to the optimized synchronous
recovery routines (raid6_2data_recov, raid6_datap_recov).  The only
time this path will be taken is if we have decided to perform
asynchronous recovery but at a later call to async_pq the offload
driver reports it has run out of descriptors.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/