linux-kernel - Re: [PATCH] iio: dht11: Read bit stream from IRQ on falling edges only

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: <ac9ab2224ad19ac606de38fa474dce0c@ccbib.org>
Date:   Tue, 07 Feb 2023 11:33:10 +0100
From:   harald@...ib.org
To:     pelzi@...ing-snail.de
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: dht11: Read bit stream from IRQ on falling edges
 only

On 2023-02-05 21:41, pelzi@...ing-snail.de wrote:
> Following up on Harald's remark, I can provide some first comparison
> data indeed:
> 
> Am 31.01.23 um 10:44 schrieb harald@...ib.org:
>> This seems like a really small benefit. And we would lose the
>> low state timings in debug output, which I personally find quite
>> convenient. Unless there is data, that this change actually improves
>> something for somebody, I'd reject it.
> 
> Running test script against the original kernel module (see below where 
> on):
> 
> #     real [s]    user [s]  sys [s]  success fails  err per succ
> 1     222,068     0,515     0,506     83     96     115,66 %
> 2     223,152     0,603     0,493     86     99     115,12 %
> *3*   223,502     0,563     0,411     91     68     74,73 %
> *4*   209,626     0,431     0,189     100    15     15,00 %
> *5*   209,689     0,46      0,193     100    19     19,00 %
> *6*   220,35      0,413     0,315     100    35     35,00 %
> 
> 
> Running the patched module:
> 
> # 	Real 	User 	Sys 	Successes 	Failures 	Error rate
> 1 	223,061 	0,459 	0,258 	88 	25 	28,41 %
> 2 	222,431 	0,561 	0,367 	75 	57 	76,00 %
> 3 	225,675 	0,436 	0,178 	92 	19 	20,65 %
> 4 	222,746 	0,444 	0,194 	98 	23 	23,47 %
> 5 	222,668 	0,416 	0,205 	97 	20 	20,62 %
> *6* 	204,126 	0,34 	0,138 	100 	0 	0,00 %
> *7* 	210,495 	0,393 	0,199 	100 	16 	16,00 %
> *8* 	212,563 	0,447 	0,139 	100 	19 	19,00 %
> 
> All tests run on the same board, Allwinner H3 sold as BananaPi M2 Zero,
> under kernel 6.2.0-rc5+. The devicetree overlay is setting the
> input-debounce property of &pio to 5µs, or, because of the excessive
> error rates of the original driver in this configuration, to 1µs (lines
> marked with an asterisk).
> 
> The test simply tries to read temperature and humidity from the 
> IIO/dht11
> exposed input files every 2 seconds, immediately repeating after an 
> error.
> 
> Real/User/Sys is determined by good ol' time command, successes and
> failures are counted by the test script.
> 
> Two aspects strike:
> 
> 1) the patched version of the driver is working satisfactory even with
> 5µs input-debounce filter, where the original driver shows more failed
> than successful reads in this configuration.
> 
> 2) The error rate is consistently lower with the patched driver
> (67,9% to 33,8% average)
> 
> I believe to see similar results, i.e. a noticable improvement on the 
> error
> rate, on my old trusted RaspberryPi 2B (without any devicetree 
> fiddling, of
> course), however without quantitative comparison and based on some 
> Raspbian
> patch level rather than based on kernel 6.2.0-rc5+.
> 
> Of course I have only access to a handful of DHT22 devices, most 
> probably
> from the same production batch. But I think at least I'd like to stick
> with the patched version, tbh.

Aside from different chips (and mostly the old DHT11) there are many 
things,
that influence timing: cable lenght, input characteristic etc.

It looks good, but we should still be careful.

> Hope this helps, let me know if it'd pay to work on another version of
> the patch!

Thanks, these are indeed interresting results. If you want to move this
forward, the next steps would be:
1) Sharing your test script - yes it's trivial, but still ...
2) A theoretical analysis about possible regressions depending on timer
resolution as mentioned in an earlier message.
3) Ideally figuring out, why your version performs better then what we
currently have. I have some suspicions, but better understanding might
lead to a better approach. E.g. maybe recording the other edges isn't
the problem so long as we ignore them during decoding?

As I see it, the main thing we are losing with your current proposal is
some diagnostic features. If we keep them as much as possible and have
regressions understood and covered, I see no reason to reject your idea.

best regards,
Harald

> Best wishes
> 
> Andreas