Date: Wed, 8 Sep 2004 21:40:29 -0400 From: Brian Fundakowski Feldman <green@freebsd.org> To: David Xu <davidxu@freebsd.org> Cc: threads@freebsd.org Subject: Re: [mistry.7@osu.edu: Re: FreeBSD and wine mmap] Message-ID: <20040909014029.GL928@green.homeunix.org> In-Reply-To: <413FB291.7060501@freebsd.org> References: <20040908174941.GF928@green.homeunix.org> <413FB291.7060501@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 09, 2004 at 09:32:01AM +0800, David Xu wrote:
> Brian Fundakowski Feldman wrote:
>
> >I don't think that any allocated "red zones" should be left empty --
> >they should be explicitly allocated with no protection so that actually
> >free memory space can be explicitly searched for. Comments?
> >
> >----- Forwarded message from Anish Mistry <mistry.7@osu.edu> -----
> >
> >From: Anish Mistry <mistry.7@osu.edu>
> >To: Brian Fundakowski Feldman <green@freebsd.org>
> >Subject: Re: FreeBSD and wine mmap
> >Date: Sun, 5 Sep 2004 13:15:15 -0400
> >Cc: freebsd-current@freebsd.org
> >
> >On Sunday 29 August 2004 12:59 am, you wrote:
> >
> >>On Tue, Aug 10, 2004 at 01:53:14PM -0400, Anish Mistry wrote:
> >>
> >>>-----BEGIN PGP SIGNED MESSAGE-----
> >>>Hash: SHA1
> >>>
> >>>On Wednesday 04 August 2004 06:39 pm, you wrote:
> >>>
> >>>>On Wed, Aug 04, 2004 at 06:28:02PM -0400, Anish Mistry wrote:
> >>>>
> >>>>>Ok, so we need something like vm_map_findspace(), but for
> >
> >process
> >
> >>>>>address mapping? ie. pmap_findspace() that will return an
> >
> >address to
> >
> >>>>>a large enough free chunk?
> >>>>
> >>>>That's a good start, just to get something to work with. How this
> >
> >fits
> >
> >>>>in with the vm code and whether it is ultimately suitable in the
> >
> >long
> >
> >>>>run is probably up to Alan Cox. For now, just get something that
> >
> >(a)
> >
> >>>>doesn't break anything else; and (b) lets Wine behave the way it
> >
> >needs
> >
> >>>>to.
> >>>>
> >>>>AFAIK, there are still pthread issues with Wine, but those can't
> >
> >be
> >
> >>>>addressed until the mmap issue has a work-around.
> >>>
> >>>I've got a small patch that gets by the initial problem about not
> >
> >being
> >
> >>>to mmap the memory for the libraries, but the addresses that are
> >
> >mmap'ed
> >
> >>>seem to seem to overlap with memory that the current pthread
> >>>implementation want to mmap for the "red zone" when wine tries to
> >
> >create
> >
> >>>a thread. It can't mmap the "red zone" addresses since all those
> >
> >address
> >
> >>>mapping where gobbled up before the thread launched.
> >>>I'll try to figure out a way to maybe leave a space for the "red
> >
> >zone"
> >
> >>>and see if that works.
> >>>Someone who actually knows what they are doing should probably take
> >
> >a
> >
> >>>look.
> >>
> >>The red pages are implemented by leaving the memory space unallocated;
> >>I don't like that one bit -- this will cause those spaces to be
> >
> >allocated
> >
> >>but given no protection, which should provide the crash feature that
> >
> >the
> >
> >>guard pages are there for, but be less bogus (and it doesn't use more
> >>"memory," but it will use a few more vm_map_entrys.
> >>
> >>Index: lib/libpthread/thread/thr_stack.c
> >>===================================================================
> >>RCS file: /usr/ncvs/src/lib/libpthread/thread/thr_stack.c,v
> >>retrieving revision 1.8
> >>diff -u -r1.8 thr_stack.c
> >>--- lib/libpthread/thread/thr_stack.c 14 Sep 2003 22:39:44 -0000 1.8
> >>+++ lib/libpthread/thread/thr_stack.c 29 Aug 2004 04:50:28 -0000
> >>@@ -214,6 +214,17 @@
> >> stacksize, PROT_READ | PROT_WRITE, MAP_STACK,
> >> -1, 0)) == MAP_FAILED)
> >> attr->stackaddr_attr = NULL;
> >>+ if (attr->stackaddr_attr != NULL) {
> >>+ void *red;
> >>+
> >>+ red = mmap((char *)attr->stackaddr_attr + stacksize,
> >>+ _thr_guard_default, PROT_NONE,
> >>+ MAP_ANON | MAP_FIXED | MAP_PRIVATE, -1, 0);
> >>+ if (red == MAP_FAILED) {
> >>+ (void)munmap(attr->stackaddr_attr, stacksize);
> >>+ attr->stackaddr_attr = NULL;
> >>+ }
> >>+ }
> >> }
> >> if (attr->stackaddr_attr != NULL)
> >> return (0);
> >
> >This is good. Can this be committed?
>
> So can newest thread still overflow its stack with this code ?
> Also how about if another thread allocated the red zone(but not used
> as red zone) before your thread can execute this mmap with MAP_FIXED ?
> This introduces a failure case we didn't have.
>
> I think you'd map stacksize + guard_size, and use mprotect() to set
> red zone page.
That's a very good point indeed. Would you like to modify the code to
do that and commit it?
--
Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\
<> green@FreeBSD.org \ The Power to Serve! \
Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040909014029.GL928>