[uml-devel] Fwd: Re: SMP skas requirements

[uml-devel] Fwd: Re: SMP skas requirements

[Blaisorblade]

----------  Forwarded Message  ----------

Subject: Re: SMP skas requirements
Date: Tuesday 28 March 2006 00:32
From: Blaisorblade <blaisorblade@yahoo.it>
To: Jeff Dike <jdike@addtoit.com>
Cc: user-mode-linux-devel@projects.sourceforge.net

On Tuesday 21 March 2006 00:44, Blaisorblade wrote:
> On Monday 20 March 2006 22:06, Jeff Dike wrote:
> > There is some interest from a group inside Intel in helping with skas
> > SMP.  I wrote up the following, and I'd appreciate some review to see
> > if I missed anything.
> >
> > 				Jeff
> >
> > Summary, details below:
> >
> > Locking between threads of skas0 data page
> > Locking between threads of tlb flushing
> > Create one host thread per UML process when SMP is enabled,
> > 	propagating CLONE_VM to host
> > Use userspace_pids array properly
> > Make an SMP cleaning pass over UML arch

There are tons of places where spinlocks are used improperly. We need
 stricter locking, however that is complicated by the addition of atomic
 sections where UML can't sleep.

For instance, sigio_lock needs to be irqsave (I'll test the patch for this
shortly). On the other side, uml_console_write shouldn't take any lock, it
seems (see Documentation/tty.txt).

> Especially maybe_map must pin the pages (see implementation of 4G/4G
> copy_*_user from Ingo Molnar).
>
> > Add locking to filehandle
>
> I'll work on this, I had come up with a nice design time ago, if you merge
> a lockless filehandle I'll be glad to add locking to it. If you have a
> somehow stable code base on it, let me know and I'll work on the patches.
>
> > Details:
> >
> > Of particular concern is the filehandle abstraction, which is used for
> > file descriptor management.  With hostfs and humfs, it is easy to hit
> > the file descriptor limit.  filehandles transparently deal with this
> > by detecting -EMFILE from the host, closing descriptors in order to
> > free up slots, and reopening closed files when they become active
> > again.  The problem is that there is currently nothing preventing a
> > descriptor from being reclaimed while it is being used.  A descriptor
> > is first checked to see if it's open (and reopened if not), and then
> > used.  The descriptor reclaiming code running on a different processor
> > could close the descriptor after it's checked for validity and before
> > it's used.  We need something to prevent this, like a
> > get_fh()/put_fh() around all such operations.
>
> My design was that you:
> a) take the list lock
> b) remove the fd from the list
> c) drop the list lock
> instead of simply bringing the fd upfront
> d) call read (or whatever) on the interested fd
> e) redo a)-c) but now to _add_ the fd on list, on the front
>
> This may also need a reference count if in step b) the fd may already be on
> the list.
>
> *) when _any_ fd API of the existing set (os_*) is called, it detects
> -EMFILE/-ENFILE and closes fd's on the back of the list (i.e. we do LRU on
> the list)
> *) using new APIs is only needed to register an fd as "reclaimable" - this
> is optional since it can be used only for file-based fds (not for pipes,
> probably not for sockets)

--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

-------------------------------------------------------

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

		
___________________________________ 
Yahoo! Messenger with Voice: chiama da PC a telefono a tariffe esclusive 
http://it.messenger.yahoo.com



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Fwd: Re: SMP skas requirements

[Blaisorblade]

----------  Forwarded Message  ----------

Subject: Re: SMP skas requirements
Date: Wednesday 29 March 2006 02:41
From: Jeff Dike <jdike@addtoit.com>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: user-mode-linux-devel@projects.sourceforge.net

On Wed, Mar 29, 2006 at 12:40:00AM +0200, Blaisorblade wrote:
> Since you've now split out delete-hostfs, why don't you merge the new
> hostfs, possibly labeling it as "EXPERIMENTAL"?

My two main gripes right now are
	there are three filesystems (or one framework and two
filesystems) in one directory
	it depends on filehandle, which has aspects that neither of us likes

> Btw, there's a ton of other patches which I don't see reasons for not
> merging like Al Viro's cleanups (I've given a look to them and they
> seem safe).

Just sent in today.

> And is punctuaction_fixes likely to cause instability?

No, but I recall that you didn't really like it.

> Finally, I'd like to get devshm merged if there aren't problems with the
> code.

That one needs to check if /dev/shm is present, and fall back to /tmp
if not.

> Don't know how do you implemented it this time, but since we had special
> read/write functions for filehandles, the get_fh() and put_fh() could be
> simply put in their body. Has this changed?

No, but you didn't like those (and I agree) because of the extra
layering they added.

> > In this case, get_fh should just increment a count, put_fh should
> > decrement it, and the only list operation should be to move it to the
> > end of the list so it's last to be reclaimed.  The reclaimer would not
> > reclaim filehandles with non-zero counts.  I see no point in removing
> > it from the list and adding it back, as that seems not to protect
> > against anything.
>
> You need a spinlock on the list. With the non-reference-counted approach,
> removing that from the list allowed dropping the spinlock over the I/O call
> on the host. That's not needed with the refcount.
>
> With the refcount, likely you don't need to move it off-list, but that
> could maybe be useful to avoid looping on unused fd; however, this requires
> taking the spinlock when reinserting the element on the list.

I think you're agreeing with me, but I'm missing why we want to move
things on and off the list.

> Instead, with the refcount, if you decrease to 0 the refcount of FH_1 while
> a reclaim loop is iterating over FH_1, you only risk that FH_1 is missed on
> that reclaim pass, which isn't a race.

Yup.

> Note: testing that a fd has refcount 0 must be done atomically with freeing
> it; i.e. even with an atomic_t refcount, it must be incremented only while
> you have a lock on the list; and while freeing it, you must use
> atomic_dec_and_lock() so you get a lock on the list if the refcount goes to
> 0 (atomic_dec_and_lock() is equivalent to taking the lock, doing dec and
> test, and releasing it, but is faster).
>
> get_fh() {
> 	spin_lock(&list_lock);
> 	<iterate on list>
> 	atomic_inc(fd->count); //while still holding the lock!!
> 	if (atomic_read(fd->count) <= 0)
> 		BUG();
> 	spin_unlock(&list_lock);
> }
>
> put_fh(pointer to fd on list "fd") {
> 	if(atomic_dec_and_lock(fd->count, &list_lock)) {
> 		//Here we have the lock!
> 		//Remove the thing from list and free it
> 	}
> }

Yes, this looks reasonable.

> > You're envisioning the os_* interfaces calling back into filehandle.c
> > to get a descriptor if needed?
>
> To cause fd reclaim, that's what I mean (not sure if you meant the
> same).

Yes.

> I suggested last time to rename this as "make_reclaimable" (or
> set_reclaimable), is_reclaimable() sounds as an interrogation
> method.

Fine by me.

				Jeff

-------------------------------------------------------

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Fwd: Re: SMP skas requirements

[Blaisorblade]

Forwarding this again...

Jeff, user-mode-linux-devel@projects.sourceforge.net address is wrong! It's 
@_lists_, not @_projects_.

----------  Forwarded Message  ----------

Subject: Re: SMP skas requirements
Date: Wednesday 29 March 2006 00:40
From: Blaisorblade <blaisorblade@yahoo.it>
To: Jeff Dike <jdike@addtoit.com>
Cc: user-mode-linux-devel@projects.sourceforge.net

On Tuesday 28 March 2006 21:38, Jeff Dike wrote:
> On Mon, Mar 27, 2006 at 11:33:37PM +0100, Blaisorblade wrote:
> > Especially maybe_map must pin the pages (see implementation of 4G/4G
> > copy_*_user from Ingo Molnar).
>
> Yup, looks like we can follow Ingo's implementation on this one.
>
> > I'll work on this, I had come up with a nice design time ago, if you
> > merge a lockless filehandle I'll be glad to add locking to it. If you
> > have a somehow stable code base on it, let me know and I'll work on the
> > patches.
>
> I believe that the filehandle that's in my patchset is stable.  I've
> beat on it with hostfs with no problems.  humfs is less healthy, but
> it looks like the problems don't have anything to do with filehandle.

Since you've now split out delete-hostfs, why don't you merge the new hostfs,
possibly labeling it as "EXPERIMENTAL"?

Btw, there's a ton of other patches which I don't see reasons for not
 merging, like Al Viro's cleanups (I've given a look to them and they seem
 safe). And is punctuaction_fixes likely to cause instability? IMHO it causes
 much more problems to leave it (and other patches) undefinitely hanging in
 the tree.

Finally, I'd like to get devshm merged if there aren't problems with the
 code.

> > My design was that you:
> > a) take the list lock
> > b) remove the fd from the list
> > c) drop the list lock
> > instead of simply bringing the fd upfront
> > d) call read (or whatever) on the interested fd
> > e) redo a)-c) but now to _add_ the fd on list, on the front
>
> Which was pretty much the get_fh() and put_fh() I was considering.
> It's just a pain to have to wrap that around every file access.

Don't know how do you implemented it this time, but since we had special
read/write functions for filehandles, the get_fh() and put_fh() could be
simply put in their body. Has this changed?

> > This may also need a reference count if in step b) the fd may already be
> > on the list.

I meant "off the list", but you understood me equally.

> The scenario I think you have in mind is
> 	one process gets an fd for file I/O
> 	so does another one
> 	the first process finishes and sticks the thing back on the
> list
> 	it is reclaimed, screwing the second process, which hasn't
> made the system call yet

Exactly.

> In this case, get_fh should just increment a count, put_fh should
> decrement it, and the only list operation should be to move it to the
> end of the list so it's last to be reclaimed.  The reclaimer would not
> reclaim filehandles with non-zero counts.  I see no point in removing
> it from the list and adding it back, as that seems not to protect
> against anything.

You need a spinlock on the list. With the non-reference-counted approach,
removing that from the list allowed dropping the spinlock over the I/O call
on the host. That's not needed with the refcount.

With the refcount, likely you don't need to move it off-list, but that could
maybe be useful to avoid looping on unused fd; however, this requires taking
the spinlock when reinserting the element on the list.

Instead, with the refcount, if you decrease to 0 the refcount of FH_1 while a
reclaim loop is iterating over FH_1, you only risk that FH_1 is missed on
that reclaim pass, which isn't a race.

Note: testing that a fd has refcount 0 must be done atomically with freeing
it; i.e. even with an atomic_t refcount, it must be incremented only while
you have a lock on the list; and while freeing it, you must use
atomic_dec_and_lock() so you get a lock on the list if the refcount goes to 0
(atomic_dec_and_lock() is equivalent to taking the lock, doing dec and test,
and releasing it, but is faster).

get_fh() {
	spin_lock(&list_lock);
	<iterate on list>
	atomic_inc(fd->count); //while still holding the lock!!
	if (atomic_read(fd->count) <= 0)
		BUG();
	spin_unlock(&list_lock);
}

put_fh(pointer to fd on list "fd") {
	if(atomic_dec_and_lock(fd->count, &list_lock)) {
		//Here we have the lock!
		//Remove the thing from list and free it
	}
}

> > *) when _any_ fd API of the existing set (os_*) is called, it detects
> > -EMFILE/-ENFILE and closes fd's on the back of the list (i.e. we do LRU
> > on the list)
>
> You're envisioning the os_* interfaces calling back into filehandle.c
> to get a descriptor if needed?

To cause fd reclaim, that's what I mean (not sure if you meant the same).

> That would make this whole thing less
> "layery", which I can see.

We discussed and agreed on this last time, in the end, and I'd like not to
argue again on this.

Transparence to callers is more important than layerization. The point of
having an os_{file I/O op. name} layer is to allow the callers not to change
when the implementation changes.

> > *) using new APIs is only needed to register an fd as "reclaimable" -
> > this is optional since it can be used only for file-based fds (not for
> > pipes, probably not for sockets)
>
> Which is how things work now, as is_reclaimable is called only when it
> is known that a file has been opened.

I suggested last time to rename this as "make_reclaimable" (or
set_reclaimable), is_reclaimable() sounds as an interrogation method.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

-------------------------------------------------------

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Fwd: Re: SMP skas requirements

[Blaisorblade]

----------  Forwarded Message  ----------

Subject: Re: SMP skas requirements
Date: Tuesday 28 March 2006 00:33
From: Blaisorblade <blaisorblade@yahoo.it>
To: Jeff Dike <jdike@addtoit.com>
Cc: user-mode-linux-devel@projects.sourceforge.net

On Monday 20 March 2006 22:06, Jeff Dike wrote:
> There is some interest from a group inside Intel in helping with skas
> SMP.  I wrote up the following, and I'd appreciate some review to see
> if I missed anything.
>
> 				Jeff
>
>
> Summary, details below:
>
> Locking between threads of skas0 data page
> Locking between threads of tlb flushing
> Create one host thread per UML process when SMP is enabled,
> 	propagating CLONE_VM to host
> Use userspace_pids array properly
> Make an SMP cleaning pass over UML arch

Especially maybe_map must pin the pages (see implementation of 4G/4G
copy_*_user from Ingo Molnar).

> Add locking to filehandle

I'll work on this, I had come up with a nice design time ago, if you merge a
lockless filehandle I'll be glad to add locking to it. If you have a somehow
stable code base on it, let me know and I'll work on the patches.

> Details:
>
> Of particular concern is the filehandle abstraction, which is used for
> file descriptor management.  With hostfs and humfs, it is easy to hit
> the file descriptor limit.  filehandles transparently deal with this
> by detecting -EMFILE from the host, closing descriptors in order to
> free up slots, and reopening closed files when they become active
> again.  The problem is that there is currently nothing preventing a
> descriptor from being reclaimed while it is being used.  A descriptor
> is first checked to see if it's open (and reopened if not), and then
> used.  The descriptor reclaiming code running on a different processor
> could close the descriptor after it's checked for validity and before
> it's used.  We need something to prevent this, like a
> get_fh()/put_fh() around all such operations.

My design was that you:
a) take the list lock
b) remove the fd from the list
c) drop the list lock
instead of simply bringing the fd upfront
d) call read (or whatever) on the interested fd
e) redo a)-c) but now to _add_ the fd on list, on the front

This may also need a reference count if in step b) the fd may already be on
the list.

*) when _any_ fd API of the existing set (os_*) is called, it detects
-EMFILE/-ENFILE and closes fd's on the back of the list (i.e. we do LRU on
the list)
*) using new APIs is only needed to register an fd as "reclaimable" - this is
optional since it can be used only for file-based fds (not for pipes,
probably not for sockets)
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

-------------------------------------------------------

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel