On Dec 13, 2007, at 5:20 PM, Tom Tucker wrote:
quoted text
> 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  
> reserved.
> 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.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
-
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html