linux-kernel - Re: [PATCH v2 10/15] NTB: ntb_test: Update ntb

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: <3fe534b5-cf62-4e7b-eb33-f19b4274156b@deltatee.com>
Date:   Tue, 5 Dec 2017 12:38:28 -0700
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Jon Mason <jdmason@...zu.us>, Serge Semin <fancer.lancer@...il.com>
Cc:     Dave Jiang <dave.jiang@...el.com>,
        "Hubbe, Allen" <Allen.Hubbe@....com>,
        "S-k, Shyam-sundar" <Shyam-sundar.S-k@....com>,
        "Yu, Xiangliang" <Xiangliang.Yu@....com>,
        Gary R Hook <gary.hook@....com>, Sergey.Semin@...latforms.ru,
        linux-ntb <linux-ntb@...glegroups.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 10/15] NTB: ntb_test: Update ntb_tool DB tests



On 05/12/17 11:27 AM, Jon Mason wrote:
>>          echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
>>
>> -       write_file "c $DB_BITMASK" "$REM/db"
>> +       DB_VALID_MASK=$(read_file "$LOC/db_valid_mask")
>>
>> -       for ((i=1; i <= 8; i++)); do
>> -               let DB=$(read_file "$REM/db") || true
>> -               if [[ "$DB" != "$EXP" ]]; then
>> +       write_file "c $DB_VALID_MASK" "$REM/db"
>> +
>> +       for ((i = 0; i < 64; i++)); do
> 
> I'm guessing this should be a tunable variable, for those systems with
> a different number of doorbells.

That's actually the point of this patch. Serge is using db_valid_mask to 
test all possible doorbells instead of just the first 8. 64 is just the 
maximum number.

Looks like a nice improvement.

The only thing that's not clear to me is what this does:

> +	write_file "c $DB_VALID_MASK" "$REM/db_mask"
> +	write_file $DB_VALID_MASK "$REM/db_event"
> +	write_file "s $DB_VALID_MASK" "$REM/db_mask"
> +
> +	write_file "c $DB_VALID_MASK" "$REM/db"

It would be good to mention it in the commit message (then, as a rule of 
thumb, commits that do "A" and "B" should actually be two commits). 
Especially seeing db_event appears to be new and hasn't been discussed 
in any commit message.

Logan