Date: Sun, 12 May 2013 16:49:11 -0500 From: Alan Cox <alc@rice.edu> To: Andrey Chernov <ache@freebsd.org> Cc: Alan Cox <alc@FreeBSD.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r250577 - head/sys/vm Message-ID: <51900E57.80608@rice.edu> In-Reply-To: <51900B97.20406@freebsd.org> References: <201305121650.r4CGoJL0087149@svn.freebsd.org> <51900B97.20406@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51900E57.80608>