2007-12-14 14:28:39[permalink] [raw]
On Dec 13, 2007, at 5:20 PM, Tom Tucker wrote:
> On Thu, 2007-12-13 at 16:45 -0500, Chuck Lever wrote:
>> On Dec 13, 2007, at 4:33 PM, J. Bruce Fields wrote:
>>> On Wed, Dec 12, 2007 at 01:10:17PM -0500, Chuck Lever wrote:
>>>> On Dec 11, 2007, at 6:32 PM, Tom Tucker wrote:
>>>>> + required = atomic_read(&svsk->sk_reserved) + serv->sv_max_mesg;
>>>>> + wspace = sk_stream_wspace(svsk->sk_sk);
>>>>> + if (wspace < sk_stream_min_wspace(svsk->sk_sk))
>>>>> + return 0;
>>>>> + if (required * 2 > wspace)
>>>>> + return 0;
>>>> Since "required" is an int, this test can behave differently than
>>>> the one
>>>> it replaces.
>>> If sk_reserved can approach half of 2^31, for example, then
>>> surely we
>>> have bigger problems?
>> What stops sk_reserved from going negative?
> Nothing actively _stops_ it from going negative. Indirectly it is
> prevented by the "fact" that sv_max_mesg is always greater than the
> amount returned by any read on the socket. If this is not true, then
> sk_reserved can go negative. That would occur in svc_recv when the
> difference between the amount currently reserved (worst case
> sv_max_mesg) and the amount read is subtracted from the amount
> If the amount read were greater than sv_max_mesg then the result would
> be negative (both meanings intended).
> I could add a BUG_ON and run some tests.
My (limited) understanding is that the amount of socket output buffer
space the server thinks it needs for sending a reply is subtracted
from sk_reserved. So it seems like svc_reserve() would be the place
to assert that sk_reserved does not go negative.
As this is used mostly by svc_reserve_auth(), that suggests that a
reasonable test case would be running a read/write intensive workload
with some security flavor that uses large authenticators.