Memory reserves or lack thereof

[Alan Cox]

On 11/15/2012 12:21, Konstantin Belousov wrote:
> On Thu, Nov 15, 2012 at 11:32:18AM -0600, Alan Cox wrote:
>> On 11/13/2012 05:54, Konstantin Belousov wrote:
>>> On Mon, Nov 12, 2012 at 05:10:01PM -0600, Alan Cox wrote:
>>>> On 11/12/2012 3:48 PM, Konstantin Belousov wrote:
>>>>> On Mon, Nov 12, 2012 at 01:28:02PM -0800, Sushanth Rai wrote:
>>>>>> This patch still doesn't address the issue of M_NOWAIT calls driving
>>>>>> the memory the all the way down to 2 pages, right ? It would be nice to
>>>>>> have M_NOWAIT just do non-sleep version of M_WAITOK and M_USE_RESERVE
>>>>>> flag to dig deep.
>>>>> This is out of scope of the change. But it is required for any further
>>>>> adjustements.
>>>> I would suggest a somewhat different response:
>>>>
>>>> The patch does make M_NOWAIT into a "non-sleep version of M_WAITOK" and 
>>>> does reintroduce M_USE_RESERVE as a way to specify "dig deep".
>>>>
>>>> Currently, both M_NOWAIT and M_WAITOK can drive the cache/free memory 
>>>> down to two pages.  The effect of the patch is to stop M_NOWAIT at two 
>>>> pages rather than allowing it to continue to zero pages.
>>>>
>>>> When you say, "This is out of scope ...", I believe that you are 
>>>> referring to changing two pages into something larger.  I agree that 
>>>> this is out of scope for the current change.
>>> I referred exactly to the difference between M_USE_RESERVE set or not.
>>> IMO this is what was asked by the question author. So yes, my mean of
>>> the 'out of scope' is about tweaking the 'two pages reserve' in some
>>> way.
>> Since M_USE_RESERVE is no longer deprecated in HEAD, here is my proposed
>> man page update to malloc(9):
>>
>> Index: share/man/man9/malloc.9
>> ===================================================================
>> --- share/man/man9/malloc.9     (revision 243091)
>> +++ share/man/man9/malloc.9     (working copy)
>> @@ -29,7 +29,7 @@
>>  .\" $NetBSD: malloc.9,v 1.3 1996/11/11 00:05:11 lukem Exp $
>>  .\" $FreeBSD$
>>  .\"
>> -.Dd January 28, 2012
>> +.Dd November 15, 2012
>>  .Dt MALLOC 9
>>  .Os
>>  .Sh NAME
>> @@ -153,13 +153,12 @@ if
>>  .Dv M_WAITOK
>>  is specified.
>>  .It Dv M_USE_RESERVE
>> -Indicates that the system can dig into its reserve in order to obtain the
>> -requested memory.
>> -This option used to be called
>> -.Dv M_KERNEL
>> -but has been renamed to something more obvious.
>> -This option has been deprecated and is slowly being removed from the
>> kernel,
>> -and so should not be used with any new programming.
>> +Indicates that the system can use its reserve of memory to satisfy the
>> +request.
>> +This option should only be used in combination with
>> +.Dv M_NOWAIT
>> +when an allocation failure cannot be tolerated by the caller without
>> +catastrophic effects on the system.
>>  .El
>>  .Pp
>>  Exactly one of either
> The text looks fine. Shouldn't the requirement for M_USE_RESERVE be also
> expressed in KASSERT, like this:
>
> diff --git a/sys/vm/vm_page.h b/sys/vm/vm_page.h
> index d9e4692..f8a4f70 100644
> --- a/sys/vm/vm_page.h
> +++ b/sys/vm/vm_page.h
> @@ -353,6 +351,9 @@ malloc2vm_flags(int malloc_flags)
>  {
>  	int pflags;
>  
> +	KASSERT((malloc_flags & M_USE_RESERVE) == 0 ||
> +	    (malloc_flags & M_NOWAIT) != 0,
> +	    ("M_USE_RESERVE requires M_NOWAIT"));
>  	pflags = (malloc_flags & M_USE_RESERVE) != 0 ? VM_ALLOC_INTERRUPT :
>  	    VM_ALLOC_SYSTEM;
>  	if ((malloc_flags & M_ZERO) != 0)
>
> I understand that this could be added to places of the allocator's entries,
> but I think that the page allocations are fine too.

Yes, please do that.

Alan