svn commit: r250577 - head/sys/vm

[Alan Cox]

On 05/12/2013 16:37, Andrey Chernov wrote:
> On 12.05.2013 20:50, Alan Cox wrote:
>
> GNU cc errors related to part of diff below:
> cc1: warnings being treated as errors
> ../../../vm/vm_page.c: In function 'vm_page_alloc':
> ../../../vm/vm_page.c:1209: warning: 'mpred' may be used uninitialized
> in this function
> *** [vm_page.o] Error code 1
> Formally yes, mpred here can be left unitialized.

No, it can't.  The code amounts to

    if ("x")
        mpred = ... ;
    ...
    if ("x")
        use mpred;
    ...
    if ("x")
        use mpred;

where "x" is "object != NULL".  Moreover, there are no assignments to
the variable "object" or aliases by which the variable "object" can be
modified over the lifespan of "mpred".  So, this is flawed analysis by
our antique gcc.

>> @@ -1179,7 +1206,7 @@ vm_page_alloc(vm_object_t object, vm_pin
>>  {
>>  	struct vnode *vp = NULL;
>>  	vm_object_t m_object;
>> -	vm_page_t m;
>> +	vm_page_t m, mpred;
>>  	int flags, req_class;
>>  
>>  	KASSERT((object != NULL) == ((req & VM_ALLOC_NOOBJ) == 0),
>> @@ -1195,6 +1222,11 @@ vm_page_alloc(vm_object_t object, vm_pin
>>  	if (curproc == pageproc && req_class != VM_ALLOC_INTERRUPT)
>>  		req_class = VM_ALLOC_SYSTEM;
>>  
>> +	if (object != NULL) {
>> +		mpred = vm_radix_lookup_le(&object->rtree, pindex);
>> +		KASSERT(mpred == NULL || mpred->pindex != pindex,
>> +		   ("vm_page_alloc: pindex already allocated"));
>> +	}
>>  	mtx_lock(&vm_page_queue_free_mtx);
>>  	if (cnt.v_free_count + cnt.v_cache_count > cnt.v_free_reserved ||
>>  	    (req_class == VM_ALLOC_SYSTEM &&
>> @@ -1225,8 +1257,8 @@ vm_page_alloc(vm_object_t object, vm_pin
>>  			return (NULL);
>>  #if VM_NRESERVLEVEL > 0
>>  		} else if (object == NULL || (object->flags & (OBJ_COLORED |
>> -		    OBJ_FICTITIOUS)) != OBJ_COLORED ||
>> -		    (m = vm_reserv_alloc_page(object, pindex)) == NULL) {
>> +		    OBJ_FICTITIOUS)) != OBJ_COLORED || (m =
>> +		    vm_reserv_alloc_page(object, pindex, mpred)) == NULL) {
>>  #else
>>  		} else {
>>  #endif
>> @@ -1320,7 +1352,7 @@ vm_page_alloc(vm_object_t object, vm_pin
>>  		if (object->memattr != VM_MEMATTR_DEFAULT &&
>>  		    (object->flags & OBJ_FICTITIOUS) == 0)
>>  			pmap_page_set_memattr(m, object->memattr);
>> -		vm_page_insert(m, object, pindex);
>> +		vm_page_insert_after(m, object, pindex, mpred);
>>  	} else
>>  		m->pindex = pindex;