Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

kernel/sys.c: get rid of expensive divides in groups_sort()

groups_sort() can be quite long if user loads a large gid table.

This is because GROUP_AT(group_info, some_integer) uses an integer divide.
So having to do XXX thousand divides during one syscall can lead to very
high latencies. (NGROUPS_MAX=65536)

In the past (25 Mar 2006), an analog problem was found in groups_search()
(commit d74beb9f33a5f16d2965f11b275e401f225c949d ) and at that time I
changed some variables to unsigned int.

I believe that a more generic fix is to make sure NGROUPS_PER_BLOCK is
unsigned.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Eric Dumazet and committed by
Linus Torvalds
1bf47346 6b2fb3c6

+11 -11
+1 -1
include/linux/sched.h
··· 810 810 811 811 struct io_context; /* See blkdev.h */ 812 812 #define NGROUPS_SMALL 32 813 - #define NGROUPS_PER_BLOCK ((int)(PAGE_SIZE / sizeof(gid_t))) 813 + #define NGROUPS_PER_BLOCK ((unsigned int)(PAGE_SIZE / sizeof(gid_t))) 814 814 struct group_info { 815 815 int ngroups; 816 816 atomic_t usage;
+10 -10
kernel/sys.c
··· 1145 1145 struct group_info *group_info) 1146 1146 { 1147 1147 int i; 1148 - int count = group_info->ngroups; 1148 + unsigned int count = group_info->ngroups; 1149 1149 1150 1150 for (i = 0; i < group_info->nblocks; i++) { 1151 - int cp_count = min(NGROUPS_PER_BLOCK, count); 1152 - int off = i * NGROUPS_PER_BLOCK; 1153 - int len = cp_count * sizeof(*grouplist); 1151 + unsigned int cp_count = min(NGROUPS_PER_BLOCK, count); 1152 + unsigned int len = cp_count * sizeof(*grouplist); 1154 1153 1155 - if (copy_to_user(grouplist+off, group_info->blocks[i], len)) 1154 + if (copy_to_user(grouplist, group_info->blocks[i], len)) 1156 1155 return -EFAULT; 1157 1156 1157 + grouplist += NGROUPS_PER_BLOCK; 1158 1158 count -= cp_count; 1159 1159 } 1160 1160 return 0; ··· 1165 1165 gid_t __user *grouplist) 1166 1166 { 1167 1167 int i; 1168 - int count = group_info->ngroups; 1168 + unsigned int count = group_info->ngroups; 1169 1169 1170 1170 for (i = 0; i < group_info->nblocks; i++) { 1171 - int cp_count = min(NGROUPS_PER_BLOCK, count); 1172 - int off = i * NGROUPS_PER_BLOCK; 1173 - int len = cp_count * sizeof(*grouplist); 1171 + unsigned int cp_count = min(NGROUPS_PER_BLOCK, count); 1172 + unsigned int len = cp_count * sizeof(*grouplist); 1174 1173 1175 - if (copy_from_user(group_info->blocks[i], grouplist+off, len)) 1174 + if (copy_from_user(group_info->blocks[i], grouplist, len)) 1176 1175 return -EFAULT; 1177 1176 1177 + grouplist += NGROUPS_PER_BLOCK; 1178 1178 count -= cp_count; 1179 1179 } 1180 1180 return 0;