lkml.org 
[lkml]   [2013]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] ipc: convert to idr_alloc()
From
On Thu, Feb 7, 2013 at 5:14 PM, Tejun Heo <tj@kernel.org> wrote:
> Convert to the much saner new idr interface.
>
> The new interface doesn't directly translate to the way idr_pre_get()
> was used around ipc_addid() as preloading disables preemption. From
> my cursory reading, it seems like we should be able to do all
> allocation from ipc_addid(), so I moved it there. Can you please
> check whether this would be okay? If this is wrong and ipc_addid()
> should be allowed to be called from non-sleepable context, I'd suggest
> allocating id itself in the outer functions and later install the
> pointer using idr_replace().
>
> Only compile tested.
>
> v2: The v1 conversion of ipcget_public() was incorrect. As
> idr_pre_get() returns 0 for -ENOMEM failure, @err should have been
> initialized to 1 not 0. As the function doesn't do preloading
> itself anymore, there's no point in the error handling path.
> Simply remove the -ENOMEM path.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: James Morris <jmorris@namei.org>
> ---
> Yeah, I messed up the ipcget_public() conversion. Can you please test
> this one?
>

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ The idr-i2c (v2) fix as well! ]

See also file attachments...

- Sedat -

> Thanks.
>
> ipc/util.c | 30 +++++++++---------------------
> 1 file changed, 9 insertions(+), 21 deletions(-)
>
> --- a/ipc/util.c
> +++ b/ipc/util.c
> @@ -252,7 +252,7 @@ int ipc_addid(struct ipc_ids* ids, struc
> {
> kuid_t euid;
> kgid_t egid;
> - int id, err;
> + int id;
> int next_id = ids->next_id;
>
> if (size > IPCMNI)
> @@ -261,17 +261,21 @@ int ipc_addid(struct ipc_ids* ids, struc
> if (ids->in_use >= size)
> return -ENOSPC;
>
> + idr_preload(GFP_KERNEL);
> +
> spin_lock_init(&new->lock);
> new->deleted = 0;
> rcu_read_lock();
> spin_lock(&new->lock);
>
> - err = idr_get_new_above(&ids->ipcs_idr, new,
> - (next_id < 0) ? 0 : ipcid_to_idx(next_id), &id);
> - if (err) {
> + id = idr_alloc(&ids->ipcs_idr, new,
> + (next_id < 0) ? 0 : ipcid_to_idx(next_id), 0,
> + GFP_NOWAIT);
> + idr_preload_end();
> + if (id < 0) {
> spin_unlock(&new->lock);
> rcu_read_unlock();
> - return err;
> + return id;
> }
>
> ids->in_use++;
> @@ -307,19 +311,10 @@ static int ipcget_new(struct ipc_namespa
> struct ipc_ops *ops, struct ipc_params *params)
> {
> int err;
> -retry:
> - err = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL);
> -
> - if (!err)
> - return -ENOMEM;
>
> down_write(&ids->rw_mutex);
> err = ops->getnew(ns, params);
> up_write(&ids->rw_mutex);
> -
> - if (err == -EAGAIN)
> - goto retry;
> -
> return err;
> }
>
> @@ -376,8 +371,6 @@ static int ipcget_public(struct ipc_name
> struct kern_ipc_perm *ipcp;
> int flg = params->flg;
> int err;
> -retry:
> - err = idr_pre_get(&ids->ipcs_idr, GFP_KERNEL);
>
> /*
> * Take the lock as a writer since we are potentially going to add
> @@ -389,8 +382,6 @@ retry:
> /* key not used */
> if (!(flg & IPC_CREAT))
> err = -ENOENT;
> - else if (!err)
> - err = -ENOMEM;
> else
> err = ops->getnew(ns, params);
> } else {
> @@ -413,9 +404,6 @@ retry:
> }
> up_write(&ids->rw_mutex);
>
> - if (err == -EAGAIN)
> - goto retry;
> -
> return err;
> }
>
execve("./scripts/build_linux-next.sh", ["./scripts/build_linux-next.sh"], [/* 48 vars */]) = 0
brk(0) = 0x17a0000
access("/etc/ld.so.nohwcap", F_OK) = -1 ENOENT (No such file or directory)
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fce1c170000
access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=93838, ...}) = 0
mmap(NULL, 93838, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fce1c159000
close(3) = 0
access("/etc/ld.so.nohwcap", F_OK) = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\200\30\2\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1811128, ...}) = 0
mmap(NULL, 3925208, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7fce1bb91000
mprotect(0x7fce1bd46000, 2093056, PROT_NONE) = 0
mmap(0x7fce1bf45000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1b4000) = 0x7fce1bf45000
mmap(0x7fce1bf4b000, 17624, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fce1bf4b000
close(3) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fce1c158000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fce1c157000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fce1c156000
arch_prctl(ARCH_SET_FS, 0x7fce1c157700) = 0
mprotect(0x7fce1bf45000, 16384, PROT_READ) = 0
mprotect(0x619000, 4096, PROT_READ) = 0
mprotect(0x7fce1c172000, 4096, PROT_READ) = 0
munmap(0x7fce1c159000, 93838) = 0
getpid() = 2422
rt_sigaction(SIGCHLD, {0x40f100, ~[RTMIN RT_1], SA_RESTORER, 0x7fce1bbc74a0}, NULL, 8) = 0
geteuid() = 1000
brk(0) = 0x17a0000
brk(0x17c1000) = 0x17c1000
getppid() = 2419
stat("/home/wearefam/src/linux-kernel", {st_mode=S_IFDIR|0775, st_size=12288, ...}) = 0
stat(".", {st_mode=S_IFDIR|0775, st_size=12288, ...}) = 0
open("./scripts/build_linux-next.sh", O_RDONLY) = 3
fcntl(3, F_DUPFD, 10) = 10
close(3) = 0
fcntl(10, F_SETFD, FD_CLOEXEC) = 0
rt_sigaction(SIGINT, NULL, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGINT, {0x40f100, ~[RTMIN RT_1], SA_RESTORER, 0x7fce1bbc74a0}, NULL, 8) = 0
rt_sigaction(SIGQUIT, NULL, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGQUIT, {SIG_DFL, ~[RTMIN RT_1], SA_RESTORER, 0x7fce1bbc74a0}, NULL, 8) = 0
rt_sigaction(SIGTERM, NULL, {SIG_DFL, [], 0}, 8) = 0
rt_sigaction(SIGTERM, {SIG_DFL, ~[RTMIN RT_1], SA_RESTORER, 0x7fce1bbc74a0}, NULL, 8) = 0
read(10, "#!/bin/sh\n\n### HELP\n# 1. make de"..., 8192) = 3852
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2423
close(4) = 0
read(3, "/home/wearefam/src/linux-kernel\n", 128) = 32
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2423
chdir("/home/wearefam/src/linux-kernel/linux-next") = 0
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2424
close(4) = 0
read(3, "4\n", 128) = 2
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 2
read(3, "", 128) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2424
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2425
close(4) = 0
read(3, "3\n", 128) = 2
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 2
read(3, "", 128) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2425
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2426
close(4) = 0
read(3, "8\n", 128) = 2
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2426
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2427
close(4) = 0
read(3, "0\n", 128) = 2
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2427
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2428
close(4) = 0
read(3, "-rc6\n", 128) = 5
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2428
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2429
close(4) = 0
read(3, "next20130207\n", 128) = 13
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2429
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2434
close(4) = 0
read(3, "3.8.0~rc6\n", 128) = 10
read(3, "", 128) = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 0
close(3) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2434
write(1, "make options ...... CC=gcc-4.6 -"..., 116make options ...... CC=gcc-4.6 -j4 KBUILD_BUILD_USER=sedat.dilek@gmail.com LOCALVERSION=-next20130207-4-iniza-small
) = 116
write(1, "dep-pkg options ... KDEB_PKGVERS"..., 75dep-pkg options ... KDEB_PKGVERSION=3.8.0~rc6~next20130207-4~iniza+dileks1
) = 75
openat(AT_FDCWD, ".", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 56 entries */, 32768) = 1800
getdents(3, /* 0 entries */, 32768) = 0
close(3) = 0
stat("/opt/llvm/bin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/usr/lib/lightdm/lightdm/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/usr/local/sbin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/usr/local/bin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/usr/sbin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/usr/bin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/sbin/rm", 0x7fffdcbdb450) = -1 ENOENT (No such file or directory)
stat("/bin/rm", {st_mode=S_IFREG|0755, st_size=55888, ...}) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2437
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 2437
--- SIGCHLD (Child exited) @ 0 (0) ---
rt_sigreturn(0x11) = 2437
stat("/opt/llvm/bin/fakeroot", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/lib/lightdm/lightdm/fakeroot", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/local/sbin/fakeroot", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/local/bin/fakeroot", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/sbin/fakeroot", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/bin/fakeroot", {st_mode=S_IFREG|0755, st_size=3895, ...}) = 0
pipe([3, 4]) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2438
close(4) = 0
stat("/opt/llvm/bin/tee", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/lib/lightdm/lightdm/tee", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/local/sbin/tee", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/local/bin/tee", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/sbin/tee", 0x7fffdcbdb4b0) = -1 ENOENT (No such file or directory)
stat("/usr/bin/tee", {st_mode=S_IFREG|0755, st_size=27072, ...}) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fce1c1579d0) = 2439
close(3) = 0
close(4294967295) = -1 EBADF (Bad file descriptor)
wait4(-1, make KBUILD_SRC=
make[3]: Nothing to be done for `all'.
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CC scripts/mod/devicetable-offsets.s
GEN scripts/mod/devicetable-offsets.h
HOSTCC scripts/mod/file2alias.o
HOSTLD scripts/mod/modpost
make[3]: Nothing to be done for `relocs'.
[unhandled content-type:application/octet-stream]
\
 
 \ /
  Last update: 2013-02-07 22:42    [W:0.068 / U:1.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site