Re: [PATCH v2] ipc: convert to idr_alloc()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 7, 2013 at 5:14 PM, Tejun Heo <tj@xxxxxxxxxx> 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@xxxxxxxxxx>
> Reported-by: Sedat Dilek <sedat.dilek@xxxxxxxxx>
> Cc: Stanislav Kinsbursky <skinsbursky@xxxxxxxxxxxxx>
> Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> Cc: James Morris <jmorris@xxxxxxxxx>
> ---
> Yeah, I messed up the ipcget_public() conversion.  Can you please test
> this one?
>

Tested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx> [ 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@xxxxxxxxx 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'.

Attachment: 3.8.0-rc6-next20130207-4-iniza-small.patch
Description: Binary data


[Index of Archives]     [Linux Kernel]     [Linux USB Development]     [Yosemite News]     [Linux SCSI]

  Powered by Linux