Date: Sun, 02 Nov 2014 12:04:17 -0800 From: Xin Li <delphij@delphij.net> To: Mark R V Murray <mark@grondar.org>, Konstantin Belousov <kostikbel@gmail.com> Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org> Subject: Re: svn commit: r273958 - head/sys/dev/random Message-ID: <54568E41.8030305@delphij.net> In-Reply-To: <751CD860-95B9-4F68-AE69-976B42823AD0@grondar.org> References: <201411020201.sA221unt091493@svn.freebsd.org> <720EB74E-094A-43F3-8B1C-47BC7F6FECC3@grondar.org> <1414934579.17308.248.camel@revolution.hippie.lan> <6FB65828-6A79-4BDE-A9F7-BC472BA538CE@grondar.org> <CAJ-VmomeOwE3LOpehhJ__G=FCoBDRXrrn%2BSfjwPFODts6YYHNQ@mail.gmail.com> <20141102192057.GB53947@kib.kiev.ua> <29A795E1-19E2-49E4-9653-143D3F6F3F12@grondar.org> <20141102194625.GC53947@kib.kiev.ua> <751CD860-95B9-4F68-AE69-976B42823AD0@grondar.org>
next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
On 11/02/14 11:56, Mark R V Murray wrote:
>
>> On 2 Nov 2014, at 19:46, Konstantin Belousov
>> <kostikbel@gmail.com> wrote:
>>
>>> I don???t quite follow what you mean, but it sounds like you
>>> understand the problem. Could you please explain with a bit
>>> more detail?
>>
>> Which problem ? There are two.
>>
>> One is the Adrian' complain. tsleep(9) catches signals, and
>> return EINTR/ERESTART when catched. Typical driver code checks
>> for the errors from {t,m}sleep(9) and aborts the operation if
>> error is returned. I.e. you should do error = tsleep(...); if
>> (error != 0) { abort the loop; return to caller; } The fine
>> detail is that for the case when read has already partially
>> progressed, i.e. something was copied out to uio, the error must
>> not be returned, but short read performed instead.
>
> OK, I think I follow this.
>
> In another mail you say:
>> Yes, this is because error from tsleep() in
>> random_adaptor_read() does not abort the loop. But next loop
>> iteration calls tsleep() which returns immediately since there is
>> still pending signal. The process continues indefinitely.
>
> .. which supports this what you say further above. Thanks.
>
>> This leads to another question about the code in
>> random_adapter_read(): if ra_read method sleeps, it must handle
>> the signals as well, return error, and the second loop which
>> perorms ra_read/uiomove should be aborted as well. Again, error
>> from either ra_read or uiomove(9) must result in short read if
>> something was already copied to uio. Currently, there is no error
>> returned by ra_read (or it is ignored), and error from uiomove
>> always returned, even if something was already copied.
>
> Are you saying the same thing again, or something else? If you are
> saying something else, then I am struggling to follow you.
These are essentially the same issue. Since we do tsleep() with
PCATCH, the result should be checked and handled.
>> Second problem is that random_adaptor_lock is owned while
>> tsleep() is called (or whatever sleep primitive is used inside
>> ra_read). If platform could only provide randomness through some
>> hw, and module is loaded while thread is blocked, module cannot
>> register, while reading thread cannot make progress.
>
> I’m sorry, I don’t understand this.
There is a race condition here (practically I don't think it's a real
world problem right now, but we'd better fix it before it become one).
Consider this:
dummy adapter registered
A process requests a read(), now with
&random_adaptors_lock held (shared)
Another thread calls in and attempted
to register yarrow.
Now, the third thread can not succeed until the second thread hits an
Ctrl+C.
I think that we should probably release the lock briefly and reacquire
it after the sleep to allow a different thread to acquire it.
Cheers,
- --
Xin LI <delphij@delphij.net> https://www.delphij.net/
FreeBSD - The Power to Serve! Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0
iQIcBAEBCgAGBQJUVo5BAAoJEJW2GBstM+ns6SIP/2hhmB4//+Dc6drDF54+Yh7d
8pI16CcC58WPUrf4a3xWQFTErpwLBdaRJ0c/G6r2FNv5ZWMomlNnMEvdsj88Bn1y
8cj6NxZTfAEK4YvxvYqSFEPLIlD4hnDwLyQ8ZN78uS9gOJLmZ0uaOwAulU09zTyg
OC7rIsirUG70PKe2tC4/qYjWMWg89Pdj2XYs7+aWoKDJWNNLUetllA+NTwUI2/I1
RBgRM2rTPkNVh9jd5ZZb87BtK+r4i+kaykKTYssZFXZpn6ii7NiNN1C9s6yE8eBd
GBus2b4vCi/5g4Pdlm7geKuoi1l/UgjRiAhVPiDcdb4tuE5qmcuGuohFXM8ibrwn
AT7RVV5jcZhxOvSJlsk1yXVbrDThIfnm/rg9pbsq6+/IF96CfWTFKSSeugBKodiK
QFx9gdqi6U3AA2aQ0ydQm5zG0TUR6pQVcdiDL8FKUj/Tq2zHlKilHhYFskFL1XX4
KL83e1m8CXGjkuLSX1MpG27l30mwbESfFApaNppQjFXJHF/jHh9aS0JHKIgvCvRh
BVJFr4D8Iiioj7gtduVtX362YUIFdQxPfv2ekHmlPwN9+NaEVCAAd4PvLcuxmr9R
4aRN+JHYOlPZibmwDnlldlM2DIj5wBSB2h61+Sb6hq6USEo/IaveOnVDN8unXNnE
AXYRn8M1aNAmMG8Kb/Y3
=D/my
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54568E41.8030305>