Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-02 Thread Srivatsa Vaddagiri
On Tue, Oct 02, 2007 at 06:12:39PM -0400, Eric St-Laurent wrote:
> While a sysfs interface is OK and somewhat orthogonal to the interface
> proposed the containers patches, I think maybe a new syscall should be
> considered.

We had discussed syscall vs filesystem based interface for resource
management [1] and there was a heavy bias favoring filesystem based interface,
based on which the container (now "cgroup") filesystem evolved.

Where we already have one interface defined, I would be against adding 
an equivalent syscall interface.

Note that this "fair-user" scheduling can in theory be accomplished
using the same cgroup based interface, but requires some extra setup in
userspace (either to run a daemon which moves tasks to appropriate
control groups/containers upon their uid change OR to modify initrd to mount 
cgroup filesystem at early bootup time). I expect most distros to enable
CONFIG_FAIR_CGROUP_SCHED (control group based fair group scheduler) and not 
CONFIG_FAIR_USER_SHCED (user id based fair group scheduler). The only
reason why we are providing CONFIG_FAIR_USER_SCHED and the associated
sysfs interface is to help test group scheduler w/o requiring knowledge
of cgroup filesystem.

Reference:

1. http://marc.info/?l=linux-kernel&m=116231242201300&w=2

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-02 Thread Eric St-Laurent

On Mon, 2007-10-01 at 16:44 +0200, Ingo Molnar wrote:
> > Adds tunables in sysfs to modify a user's cpu share.
> > 
> > A directory is created in sysfs for each new user in the system.
> > 
> > /sys/kernel/uids//cpu_share
> > 
> > Reading this file returns the cpu shares granted for the user.
> > Writing into this file modifies the cpu share for the user. Only an 
> > administrator is allowed to modify a user's cpu share.
> > 
> > Ex: 
> > # cd /sys/kernel/uids/
> > # cat 512/cpu_share
> > 1024
> > # echo 2048 > 512/cpu_share
> > # cat 512/cpu_share
> > 2048
> > #
> 
> looks good to me! I think this API is pretty straightforward. I've put 
> this into my tree and have updated the sched-devel git tree:
> 

While a sysfs interface is OK and somewhat orthogonal to the interface
proposed the containers patches, I think maybe a new syscall should be
considered.

Since we now have a fair share cpu scheduler, maybe an interface to
specify the cpu share directly (alternatively to priority) make sense.

For processes, it may become more intuitive (and precise) to set the
processing share directly than setting a priority which is converted to
a share.

Maybe something similar to ioprio_set() and ioprio_get() syscalls:

- per user cpu share
- per user group cpu share
- per process cpu share
- per process group cpu share


Best regards,

- Eric


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-01 Thread Srivatsa Vaddagiri
On Mon, Oct 01, 2007 at 12:12:21PM -0400, Dave Jones wrote:
> Can we start adding stuff to Documentation/ for new files created
> in sysfs ?  There's so much undocumented stuff in there now that
> it's unfunny.

Hi Dave,
We will in the next version of the patch. At this point, the patch was 
sent out to get an idea of whether sysfs is the right place for this interface 
or not.

> A great start would be 'wtf is a cpu_share and why would I want to
> change it' ?

Quick answer before we release the next version : This has to do with
the CONFIG_FAIR_USER_SCHED feature which allows dividing CPU bandwidth
equally between all users of a system. Before this patch, CPU bandwidth was 
divided equally among all non-root users. After this patch, it will
be possible to control CPU bandwidth allocation to each user separately.

For ex: by setting user "vatsa"'s cpu_share to be twice that of user
"guest", it would be possible for user "vatsa" to get 2x CPU bandwidth that of 
user "guest"


--
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-01 Thread Dave Jones
On Mon, Oct 01, 2007 at 07:34:54PM +0530, Dhaval Giani wrote:
 > Hi Ingo,
 > 
 > Adds tunables in sysfs to modify a user's cpu share.
 > 
 > A directory is created in sysfs for each new user in the system.
 > 
 >  /sys/kernel/uids//cpu_share
 > 
 > Reading this file returns the cpu shares granted for the user.
 > Writing into this file modifies the cpu share for the user. Only an 
 > administrator is allowed to modify a user's cpu share.
 > 
 > Ex: 
 >  # cd /sys/kernel/uids/
 >  # cat 512/cpu_share
 >  1024
 >  # echo 2048 > 512/cpu_share
 >  # cat 512/cpu_share
 >  2048
 >  #

Can we start adding stuff to Documentation/ for new files created
in sysfs ?  There's so much undocumented stuff in there now that
it's unfunny.

A great start would be 'wtf is a cpu_share and why would I want to
change it' ?

Dave

-- 
http://www.codemonkey.org.uk
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-01 Thread Srivatsa Vaddagiri
On Mon, Oct 01, 2007 at 04:44:02PM +0200, Ingo Molnar wrote:
> but there seems to be a bug in it, i get this:
> 
>  kobject_add failed for 500 with -EEXIST, don't try to register things with 
> the same name in the same directory.

Looks like a remove_dir/create_dir race ..

free_user() alloc_uid()

   uid_hash_remove(up);

uid_hash_insert(new, hashent);

   schedule_work()
user_kobject_create();
kobject_add();  -> Boom!

   remove_user_sysfs_dir();

/me looks up sysfs programming examples ..

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-01 Thread Ingo Molnar

hi Dhaval,

* Dhaval Giani <[EMAIL PROTECTED]> wrote:

> Hi Ingo,
> 
> Adds tunables in sysfs to modify a user's cpu share.
> 
> A directory is created in sysfs for each new user in the system.
> 
>   /sys/kernel/uids//cpu_share
> 
> Reading this file returns the cpu shares granted for the user.
> Writing into this file modifies the cpu share for the user. Only an 
> administrator is allowed to modify a user's cpu share.
> 
> Ex: 
>   # cd /sys/kernel/uids/
>   # cat 512/cpu_share
>   1024
>   # echo 2048 > 512/cpu_share
>   # cat 512/cpu_share
>   2048
>   #

looks good to me! I think this API is pretty straightforward. I've put 
this into my tree and have updated the sched-devel git tree:

 git://git.kernel.org/pub/scm/linux/kernel/git/mingo/linux-2.6-sched-devel.git

but there seems to be a bug in it, i get this:

 kobject_add failed for 500 with -EEXIST, don't try to register things with the 
same name in the same directory.
  [] kobject_shadow_add+0x13b/0x169
  [] kobject_set_name+0x28/0x91
  [] user_kobject_create+0x6a/0x90
  [] alloc_uid+0x141/0x181
  [] set_user+0x1c/0x91
  [] sys_setresuid+0xd9/0x17d
  [] sysenter_past_esp+0x5f/0x85
  ===

Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RFC/PATCH] Add sysfs control to modify a user's cpu share

2007-10-01 Thread Dhaval Giani
Hi Ingo,

Adds tunables in sysfs to modify a user's cpu share.

A directory is created in sysfs for each new user in the system.

/sys/kernel/uids//cpu_share

Reading this file returns the cpu shares granted for the user.
Writing into this file modifies the cpu share for the user. Only an 
administrator is allowed to modify a user's cpu share.

Ex: 
# cd /sys/kernel/uids/
# cat 512/cpu_share
1024
# echo 2048 > 512/cpu_share
# cat 512/cpu_share
2048
#

Signed-off-by: Srivatsa Vaddagiri <[EMAIL PROTECTED]>
Signed-off-by: Dhaval Giani <[EMAIL PROTECTED]>

---
 include/linux/sched.h |   11 +
 kernel/ksysfs.c   |4 +
 kernel/sched.c|5 ++
 kernel/user.c |  110 +-
 4 files changed, 129 insertions(+), 1 deletion(-)

Index: linux-2.6.23-rc8-sched-devel/include/linux/sched.h
===
--- linux-2.6.23-rc8-sched-devel.orig/include/linux/sched.h
+++ linux-2.6.23-rc8-sched-devel/include/linux/sched.h
@@ -86,6 +86,7 @@ struct sched_param {
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -598,9 +599,18 @@ struct user_struct {
 
 #ifdef CONFIG_FAIR_USER_SCHED
struct task_group *tg;
+   struct kset kset;
+   struct subsys_attribute user_attr;
+   struct work_struct work;
 #endif
 };
 
+#ifdef CONFIG_FAIR_USER_SCHED
+extern int uids_kobject_init(void);
+#else
+static inline int uids_kobject_init(void) { return 0; }
+#endif
+
 extern struct user_struct *find_user(uid_t);
 
 extern struct user_struct root_user;
@@ -1846,6 +1856,7 @@ extern struct task_group *sched_create_g
 extern void sched_destroy_group(struct task_group *tg);
 extern void sched_move_task(struct task_struct *tsk);
 extern int sched_group_set_shares(struct task_group *tg, unsigned long shares);
+extern unsigned long sched_group_shares(struct task_group *tg);
 
 #endif
 
Index: linux-2.6.23-rc8-sched-devel/kernel/ksysfs.c
===
--- linux-2.6.23-rc8-sched-devel.orig/kernel/ksysfs.c
+++ linux-2.6.23-rc8-sched-devel/kernel/ksysfs.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define KERNEL_ATTR_RO(_name) \
 static struct subsys_attribute _name##_attr = __ATTR_RO(_name)
@@ -116,6 +117,9 @@ static int __init ksysfs_init(void)
  ¬es_attr);
}
 
+   if (!error)
+   error = uids_kobject_init();
+
return error;
 }
 
Index: linux-2.6.23-rc8-sched-devel/kernel/sched.c
===
--- linux-2.6.23-rc8-sched-devel.orig/kernel/sched.c
+++ linux-2.6.23-rc8-sched-devel/kernel/sched.c
@@ -6928,4 +6928,9 @@ int sched_group_set_shares(struct task_g
return 0;
 }
 
+unsigned long sched_group_shares(struct task_group *tg)
+{
+   return tg->shares;
+}
+
 #endif /* CONFIG_FAIR_GROUP_SCHED */
Index: linux-2.6.23-rc8-sched-devel/kernel/user.c
===
--- linux-2.6.23-rc8-sched-devel.orig/kernel/user.c
+++ linux-2.6.23-rc8-sched-devel/kernel/user.c
@@ -56,6 +56,62 @@ struct user_struct root_user = {
 };
 
 #ifdef CONFIG_FAIR_USER_SCHED
+
+static struct kobject uids_kobject;
+
+ssize_t cpu_shares_show(struct kset *kset, char *buffer)
+{
+   struct user_struct *up = container_of(kset, struct user_struct, kset);
+
+   return sprintf(buffer, "%lu\n", sched_group_shares(up->tg));
+}
+
+ssize_t cpu_shares_store(struct kset *kset, const char *buffer, size_t size)
+{
+   struct user_struct *up = container_of(kset, struct user_struct, kset);
+   unsigned long shares;
+   int rc;
+
+   sscanf(buffer, "%lu", &shares);
+
+   rc = sched_group_set_shares(up->tg, shares);
+
+   return (rc ? rc : size);
+}
+
+static void user_attr_init(struct subsys_attribute *sa, char *name, int mode)
+{
+   sa->attr.name = name;
+   sa->attr.mode = mode;
+   sa->show = cpu_shares_show;
+   sa->store = cpu_shares_store;
+}
+
+static int user_kobject_create(struct user_struct *up)
+{
+   struct kset *kset = &up->kset;
+   struct kobject *kobj = &kset->kobj;
+   int error;
+
+   memset(kset, 0, sizeof(struct kset));
+   kobj->parent = &uids_kobject;
+   kobject_set_name(kobj, "%d", up->uid);
+   kset_init(kset);
+   user_attr_init(&up->user_attr, "cpu_share", 0644);
+
+   error = kobject_add(kobj);
+
+   if (error)
+   goto done;
+
+   error = sysfs_create_file(kobj, &up->user_attr.attr);
+   if (error)
+   kobject_del(kobj);
+
+done:
+   return error;
+}
+
 static void sched_destroy_user(struct user_struct *up)
 {
sched_destroy_group(up->tg);
@@ -77,11 +133,53 @@ static void sched_switch_user(struct tas
sched_move_task(p);
 }
 
+int __init uids_kobject_init(void)
+{
+