Re: madvise(MADV_FREE) doesn't work in some cases?

On Tue, Jul 06, 2021 at 01:21:17AM +0300, Vitaliy Gusev wrote:
> Comments are below,
> 
> > commit 0392eb3c93b7dacc31dbdf8ec2fc40fa5ba67c62
> > Author: Konstantin Belousov <kib@FreeBSD.org <mailto:kib@FreeBSD.org>>
> > Date:   Mon Jul 5 21:53:22 2021 +0300
> > 
> >    madvise(MADV_FREE): try harder to handle shadow chain
> > 
> >    In particular, collapse top object and see if there is no backing object
> >    after, which means that we would not revert to older content if drop the
> >    top object.
> > 
> > diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
> > index 1ac4ccf72f11..80abac223f29 100644
> > --- a/sys/vm/vm_map.c
> > +++ b/sys/vm/vm_map.c
> > @@ -3033,6 +3033,7 @@ vm_map_madvise(
> > 			entry = vm_map_entry_succ(entry);
> > 		for (; entry->start < end;
> > 		    entry = vm_map_entry_succ(entry)) {
> > +			vm_object_t obj;
> > 			vm_offset_t useEnd, useStart;
> > 
> > 			if ((entry->eflags & MAP_ENTRY_IS_SUB_MAP) != 0)
> > @@ -3046,9 +3047,16 @@ vm_map_madvise(
> > 			 * backing object can change.
> > 			 */
> > 			if (behav == MADV_FREE &&
> > -			    entry->object.vm_object != NULL &&
> > -			    entry->object.vm_object->backing_object != NULL)
> > -				continue;
> > +			    (obj = entry->object.vm_object) != NULL &&
> > +			    obj->backing_object != NULL) {
> > +				VM_OBJECT_WLOCK(obj);
> > +				if ((obj->flags & OBJ_DEAD) != 0)
> > +					continue;
> 
> Here is object is left locked, however I didn’t met this condition.
Right.

> 
> > +				vm_object_collapse(obj);
> > +				VM_OBJECT_WUNLOCK(obj);
> > +				if (obj->backing_object != NULL)
> > +					continue;
> 
> After testing it looks that backing_object is not null here, memory is not freed and mmapfork test program is still killed. Any ideas?
> 
I finally looked at this thing again.  You do madvise(PAGE_SIZE).  With
my patch (updated version below) issue madvise() for the whole region
once.

commit b183ec9e985961bd0b450d9e7bdbbcde751c830b
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Mon Jul 5 21:53:22 2021 +0300

    madvise(MADV_FREE): try harder to handle shadow chain
    
    In particular, collapse top object and see if there is no backing object
    after, which means that we would not revert to older content if drop the
    top object.

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index 1ac4ccf72f11..188b37560e0e 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -3033,6 +3033,7 @@ vm_map_madvise(
 			entry = vm_map_entry_succ(entry);
 		for (; entry->start < end;
 		    entry = vm_map_entry_succ(entry)) {
+			vm_object_t obj;
 			vm_offset_t useEnd, useStart;
 
 			if ((entry->eflags & MAP_ENTRY_IS_SUB_MAP) != 0)
@@ -3046,9 +3047,18 @@ vm_map_madvise(
 			 * backing object can change.
 			 */
 			if (behav == MADV_FREE &&
-			    entry->object.vm_object != NULL &&
-			    entry->object.vm_object->backing_object != NULL)
-				continue;
+			    (obj = entry->object.vm_object) != NULL &&
+			    obj->backing_object != NULL) {
+				VM_OBJECT_WLOCK(obj);
+				if ((obj->flags & OBJ_DEAD) != 0) {
+					VM_OBJECT_WUNLOCK(obj);
+					continue;
+				}
+				vm_object_collapse(obj);
+				VM_OBJECT_WUNLOCK(obj);
+				if (obj->backing_object != NULL)
+					continue;
+			}
 
 			pstart = OFF_TO_IDX(entry->offset);
 			pend = pstart + atop(entry->end - entry->start);