[9fans] segattach in telnetd

[9fans] segattach in telnetd

[arisawa]

Hello,

we have an example for segattach() in /sys/src/cmd/ip/telnetd.c

void*
share(ulong len)
{
	uchar *vastart;

	vastart = sbrk(0);
	if(vastart == (void*)-1)
		return 0;
	vastart += 2*1024*1024;

	if(segattach(0, "shared", vastart, len) == (void*)-1)
		return 0;

	return vastart;
}

I think it should be
-	if(segattach(0, "shared", vastart, len) == (void*)-1)
+	if((vastart = segattach(0, "shared", vastart, len)) == (void*)-1)

Kenji Arisawa

Re: [9fans] segattach in telnetd

[cinap_lenrek]

why? segattach() gets an explicit address here and
it should round va down and va+len up as neccesary
to make them page aligned. but the initial passed
range should be covered by the created segment
in any case.

do you have any trouble with telnet that is fixed
by your change?

--
cinap

Re: [9fans] segattach in telnetd

[arisawa]

Hello,

I think 
	vastart += 2*1024*1024; 
is weird.
if we look /proc/$ip/segment in share(), we will have better solution.
but I am not convinced we should apply the solution.
because we seldom needs shared segment.

I haven’t had any trouble with telnetd.
I needed shared segment for special purpose in my program (Pegasus).

thanks

2014/09/14 12:17、cinap_lenrek@felloff.net のメール：

> why? segattach() gets an explicit address here and
> it should round va down and va+len up as neccesary
> to make them page aligned. but the initial passed
> range should be covered by the created segment
> in any case.
> 
> do you have any trouble with telnet that is fixed
> by your change?
> 
> --
> cinap
> 

Re: [9fans] segattach in telnetd

[cinap_lenrek]

the + 2MB is to keep some distance to the bss (heap).
the assumption might'v been that telnet will not
dynamically allocate more than 2MB from the time
when the segment is created.

a better solution would be to just pass 0 as va. the
kernel will then put the segment downwards from
the stack base and finds a hole in the address
space for us returning the base address from
segattach().

segments cannot expand down. even the stack segment
has a fixed size and uses demand paging for downwards
expansion, thats why this works :)

--
cinap

Re: [9fans] segattach in telnetd

[arisawa]

Hello cinap,

My experiment here.

void
main(int argc, char *argv[])
{	char *ap;
	char buf[1];
	ARGBEGIN{
	default: usage();
	}ARGEND
	
	ap = segattach(0,"share",0,1024);
	print("%p\n",ap);	// ffffffff  error

	/* compare */
	ap = sbrk(0);
	print("sbrk %p\n",ap);	// 6cb0
	ap += 2*1024*1024;
	print("%p\n",ap);	// 206cb0
	ap = segattach(0, "shared", ap, 1024);
	print("%p\n",ap);	// 206000

	read(0,buf,1);
}

we can find the allocated share memory:

term% cat /proc/612768/segment
Stack     defff000 dffff000    1
Text   R      1000     6000    1
Data          6000     7000    1
Bss           7000     7000    1
Shared      206000   207000    1
term% 

note that 0x206cb0 + 0x400 > 0x20700.
if we believe that we have a memory
of range [mp,mp+1024) by executing
mp = share(1024)
then we will fail.

Kenji Arisawa

2014/09/14 12:17、cinap_lenrek@felloff.net のメール：

> it should round va down and va+len up as neccesary
> to make them page aligned. but the initial passed
> range should be covered by the created segment
> in any case.

Re: [9fans] segattach in telnetd

[cinap_lenrek]

it works, you just mistyped "shared" in the first call.

--
cinap

Re: [9fans] segattach in telnetd

[arisawa]

ooh
that is my halt.
egattach(0,"shared",0,1024);
works fine!

my test

test1(void)
{
	char *ap;
	ap = segattach(0,"shared",0,1024);
	print("%p\n",ap);	// deffe000
}

term% cat /proc/917256/segment
Stack     defff000 dffff000    1
Text   R      1000     6000    1
Data          6000     7000    1
Bss           7000     7000    1
Shared    deffe000 defff000    1
term%

that nice.

by the way, is this behavior special to 9front?

Kenji Arisawa

2014/09/14 13:51、cinap_lenrek@felloff.net のメール：

> it works, you just mistyped "shared" in the first call.
> 
> --
> cinap
> 

Re: [9fans] segattach in telnetd

[cinap_lenrek]

no, it is not. labs also seems to just round up the length
not adding the remainder of the unaligned va address. it
would be a one line fix in the kernel.

the manpage sais this:

          Va and len specify the position of the segment in the
          process's address space.  Va is rounded down to the nearest
          page boundary and va+len is rounded up.  The system does not
          permit segments to overlap.  If va is zero, the system will
          choose a suitable address.

the question is, does the va in va+len mean before or after
the rounding down. i'd say *before* because otherwise, theres no
point in define it as va+len.

--
cinap

Re: [9fans] segattach in telnetd

[erik quanstrom]

> by the way, is this ehavior special to 9front?

nope.  9atom does this.  i had to deal with this for 64 bit
quite a while ago, since the add to mb to the brk trick doesn't
work when your page size is 2mb.  be aware if you're attaching segments
that your idea of what the page size might be may be inaccurate.
i
am not sure i read cinap's email correctly, but i'd like to
phrase the man page as [va, va+len) will be valid unless the
return value is (void*)-1.  there are no promises about other
addresses just before or after this range.

- erik

Re: [9fans] segattach in telnetd

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 311 bytes --]

On 14 September 2014 15:43, erik quanstrom <quanstro@quanstro.net> wrote:

> but i'd like to
> phrase the man page as [va, va+len) will be valid unless the
> return value is (void*)-1.  there are no promises about other
> addresses just before or after this range.
>

I think that's correct behaviour.

[-- Attachment #2: Type: text/html, Size: 668 bytes --]

Re: [9fans] segattach in telnetd

[erik quanstrom]

> nope.  9atom does this.  i had to deal with this for 64 bit
> quite a while ago, since the add to mb to the brk trick doesn't
> work when your page size is 2mb.  be aware if you're attaching segments
> that your idea of what the page size might be may be inaccurate.

i should be more clear on this, the magic numbers don't work.  this exposes
the fact that this code is making assumptions that aren't called out.  it's best
to avoid those by letting the kernel place the segment.  the kernel indeed has
the information to place this segment correctly.  ... or should have---there's a bug
in the labs kernel where an automaticly placed segment can collide with the
temporary stack.  i solved this by putting the temporary stack above the normal
stack, but i think there was some thought that there were better ideas.  i felt that
running a placement algorithm on the temporary stack for every exec was
excessive.

- erik

Re: [9fans] segattach in telnetd

[cinap_lenrek]

on the other point, you are indeed correct. the manpage
or the kernel is wrong. will sort this out tomorrow.

--
cinap