Re: [PATCH] sysctl-controlled number of groups.

[David Mosberger <davidm@napali.hpl.hp.com>]

>From the ia64-side, it looks mostly fine to me.  Some minor things:

 - Typo in first sentence of patch comment (can be -> to be ?)
 - If I'm reading the patch right, there will be identical sys32_getgroups16()
   definitions in .../ia32/sys_ia32.c and and compat_linux.c; did you mean
   to name the latter compat_getgroups16()?  (ditto for setgroups16 and s390
   and sparc64, i think)
 - I suspect removing NGROUPS from param.h will break glibc and/or user-level
   apps.  param.h is one of those kernel files that are directly exposed
   to user-level; may want to keep NGROUPS inside an #ifndef __KERNEL__.

	--david

>>>>> On Thu, 25 Sep 2003 13:21:01 +1000, Rusty Russell <rusty@rustcorp.com.au> said:

  Rusty> We have a client (using SAMBA) who has people in 190 groups.  Since NT
  Rusty> has hierarchical groups, this is not all that rare.

  Rusty> What do people think of this patch?

  Rusty> Name: Dynamic Allocation of Groups Array When Required: With Refcounting
  Rusty> Author: Rusty Russell
  Rusty> Status: Tested on 2.6.0-test5-bk5
  Rusty> Depends: Misc/qemu-page-offset.patch.gz

  Rusty> D: This patch allows the maximum number of groups can be varied using
  Rusty> D: sysctl.  To do this, we use a separate group allocation if >
  Rusty> D: NGROUPS_INTERNAL, which we reference count (sharing being
  Rusty> D: v. v. common).
  Rusty> D: 
  Rusty> D: Changes:
  Rusty> D: 1) Remove the NGROUPS define from archs.
  Rusty> D: 2) Fixup the few places which declare [NGROUPS] arrays on the stack.
  Rusty> D: 3) The ia64, s390 and sparc64 ports have their own setgroups/getgroups
  Rusty> D:    implementations: unify them on the ia64 one, which calls the core
  Rusty> D:    functions.
  Rusty> D: 4) Change the task_struct to have an [NGROUPS_INTERNAL] array and
  Rusty> D:    a pointer (not convinced this is worth it at all, might skip it
  Rusty> D:    and always use external).
  Rusty> D: 5) Use a reference counted external array, fix up fork() to deal.
  Rusty> D: 6) Introduce max_groups and use it instead of NGROUPS.
  Rusty> D: 7) Add a sysctl to vary max_groups.
  Rusty> D: 
  Rusty> D: This patch scars nfs: artificially restrict the groups there to 
  Rusty> D: SVC_CRED_NGROUPS (32), which is probably wrong, but won't break if
  Rusty> D: they don't change the default.
  Rusty> D: 
  Rusty> D: This patch breaks intermezzo: I'm not sure how they want to deal
  Rusty> D: with it.