Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 15 Apr 2000 12:32:12 -0700
From:      Alfred Perlstein <bright@wintelcom.net>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        Brian Fundakowski Feldman <green@FreeBSD.ORG>, Michael Reifenberger <root@nihil.plaut.de>, FreeBSD-Current <current@FreeBSD.ORG>, alc@FreeBSD.ORG
Subject:   Re: panic: vm_object_shadow: source object has OBJ_ONEMAPPING set.
Message-ID:  <20000415123212.S4381@fw.wintelcom.net>
In-Reply-To: <200004151803.LAA79656@apollo.backplane.com>; from dillon@apollo.backplane.com on Sat, Apr 15, 2000 at 11:03:48AM -0700
References:  <Pine.BSF.4.21.0004150932460.16247-100000@green.dyndns.org> <200004151803.LAA79656@apollo.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help
* Matthew Dillon <dillon@apollo.backplane.com> [000415 11:32] wrote:
> 
> :On Thu, 13 Apr 2000, Michael Reifenberger wrote:
> :
> :> Hi,
> :> when using a linux java app (SAP PlatinGUI 46Cb2) I get the above panic.
> :> FreeBSD -current. Kernel+mods in sync.
> :> Linux from ports. Linux-Java-JDK 1.2.2 from blackdown as of yesterday.
> :> Backtrace see crash.txt. Kernelconfig see nihil.
> :> Any thoughts anyone?
> :
> :Yes, I've gotten these too.  I really believe the assumptions the code
> :there makes are wrong, and I've got a patch to correct them to what I
> :think they are supposed to be.  You've got the standard disclaimer on
> :the patch, though I assure you it has shown no ill effects to me, and I
> :noticed this bug through WINE.
> :
> :I've asked Poul-Henning Kamp, who seems to also think that the code makes
> :wrong assumptions.  I've asked Matt Dillon and gotten no reply (a month
> :now, at least).  I've asked Alan Cox, and he'd help if we could get him
> :a test case so he can watch it happen himself and debug it himself.
> :
> :Do you think you can find a specific set of steps for Alan to reproduce
> :it?
> :
> :> Bye!
> :> ----
> :> Michael Reifenberger
> :> ^.*Plaut.*$, IT, R/3 Basis, GPS
> :
> :--
> : Brian Fundakowski Feldman           \  FreeBSD: The Power to Serve!  /
> : green@FreeBSD.org                    `------------------------------'
> 
>     Oh, sorry about that.  I've run out of time and I have to take a
>     chopper to my email.  Sometimes I chop out too much.
> 
>     vm_object_shadow() is used to shadow VM objects when a write fault
>     occurs, and when forking (to avoid early copying of the vm_object's).
> 
>     Hmm.  Lets see.  Well, I'd say the original code is obviously wrong.
>     You CAN have a ref_count > 1 and still have OBJ_ONEMAPPING set.  I
>     remember Alan and I discovering that case last year as we were fixing
>     up the vm_object stuff.  Basically, the ref_count can be temporarily
>     bumped with OBJ_ONEMAPPING still set.
> 
>     Here's an associated piece of code to look at.  Line 2133 of
>     vm_map.c (in vmspace_fork()).  Here the vmspace_fork() code is
>     trying to force the creation of a shadow object which it then
>     intends to close.  It is bumping the ref_count to 'ensure' this.
>     So your change will break this piece of code.
> 
>                         /*
>                          * Add the reference before calling vm_object_shadow
>                          * to insure that a shadow object is created.
>                          */
>                         vm_object_reference(object);
>                         if (old_entry->eflags & MAP_ENTRY_NEEDS_COPY) {
>                                 vm_object_shadow(&old_entry->object.vm_object,
>                                         &old_entry->offset,
>                                         atop(old_entry->end - old_entry->start))
> ;
>                                 old_entry->eflags &= ~MAP_ENTRY_NEEDS_COPY;
>                                 object = old_entry->object.vm_object;
>                         }
>                         vm_object_clear_flag(object, OBJ_ONEMAPPING);
> 
>     I think the solution is to cear OBJ_ONEMAPPING before calling 
>     vm_object_shadow() in vm_map.c, PLUS do your patch.  I've included
>     my version below (untested).

I really don't like this, what's the point of having a "helper"
function vm_object_reference(), if it doesn't do all the required
work (vm_object_clear_flag())?  This adds unneeded complexity to the
caller.

Is there a good reason for not having the vm_object_clear_flag() in
vm_object_reference()?

> 
>     Re: The other person's comment about a possible reference count leak.
>     I do not believe there is any reference count leak, the reference count
>     was being bumped due to the forks and the shadows causing fragmentation
>     of the original object.

A reference count leak did occur, ref_count was bumped without other
sanity flags being cleared, the code makes it easy to shoot yourself
in the foot.

-Alfred

> Index: vm_map.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/vm/vm_map.c,v
> retrieving revision 1.187
> diff -u -r1.187 vm_map.c
> --- vm_map.c	2000/02/28 04:10:35	1.187
> +++ vm_map.c	2000/04/15 18:02:06
> @@ -2119,10 +2119,14 @@
>  			}
>  
>  			/*
> -			 * Add the reference before calling vm_object_shadow
> -			 * to insure that a shadow object is created.
> +			 * Clear OBJ_ONEMAPPING before calling vm_object_shadow
> +			 * to ensure that a shadow object is created.  Add a
> +			 * reference to cover the new vm_map_entry being
> +			 * associated with the object.
>  			 */
>  			vm_object_reference(object);
> +			vm_object_clear_flag(object, OBJ_ONEMAPPING);
> +
>  			if (old_entry->eflags & MAP_ENTRY_NEEDS_COPY) {
>  				vm_object_shadow(&old_entry->object.vm_object,
>  					&old_entry->offset,
> @@ -2130,7 +2134,6 @@
>  				old_entry->eflags &= ~MAP_ENTRY_NEEDS_COPY;
>  				object = old_entry->object.vm_object;
>  			}
> -			vm_object_clear_flag(object, OBJ_ONEMAPPING);
>  
>  			/*
>  			 * Clone the entry, referencing the shared object.
> Index: vm_object.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/vm/vm_object.c,v
> retrieving revision 1.171.2.1
> diff -u -r1.171.2.1 vm_object.c
> --- vm_object.c	2000/03/17 10:47:35	1.171.2.1
> +++ vm_object.c	2000/04/15 18:00:29
> @@ -900,10 +900,17 @@
>  	source = *object;
>  
>  	/*
> -	 * Don't create the new object if the old object isn't shared.
> +	 * If the old object is not shared we may be able to simply use it
> +	 * as the shadow rather then have to create a new object.  Only
> +	 * objects that we can guarentee this case can be optimized - that is,
> +	 * only objects with no handles that other processes can get a hold
> +	 * of which are otherwise unassociated, have only one mapping, and
> +	 * only one reference count.  XXX do we need the reference count check
> +	 * any more? XXX
>  	 */
>  
>  	if (source != NULL &&
> +	    (source->flags & OBJ_ONEMAPPING) != 0 &&
>  	    source->ref_count == 1 &&
>  	    source->handle == NULL &&
>  	    (source->type == OBJT_DEFAULT ||
> 


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000415123212.S4381>