lkml.org 
[lkml]   [2011]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 0/5] Signal scalability series
    On 09/30, Matt Fleming wrote:
    >
    > However, it's a good first step and
    > hopefully by keeping it relatively simple it'll make it easier to
    > review.

    Cough. I'll try to read this series next week, but currently I feel
    I will never able to understand this code. It surely compliacates
    things a lot.

    But. All I can do is to _try_ to check this series from the correctness
    pov. I can't believe (at least at first glance) this worth the trouble,
    but otoh I won't argue unless I'll find the bugs.

    > arch/ia64/kernel/signal.c | 4 +-
    > drivers/block/nbd.c | 2 +-
    > drivers/usb/gadget/f_mass_storage.c | 2 +-
    > drivers/usb/gadget/file_storage.c | 2 +-
    > fs/autofs4/waitq.c | 5 +-
    > fs/exec.c | 17 +-
    > fs/jffs2/background.c | 2 +-
    > fs/ncpfs/sock.c | 2 +
    > fs/proc/array.c | 2 +
    > fs/signalfd.c | 11 +-
    > include/linux/init_task.h | 4 +
    > include/linux/sched.h | 23 +-
    > kernel/exit.c | 29 +-
    > kernel/fork.c | 4 +
    > kernel/freezer.c | 10 +-
    > kernel/kmod.c | 8 +-
    > kernel/posix-timers.c | 5 +-
    > kernel/ptrace.c | 68 ++--
    > kernel/signal.c | 737 +++++++++++++++++++++++++++--------
    > net/9p/client.c | 6 +-
    > net/sunrpc/svc.c | 3 -
    > security/selinux/hooks.c | 11 +-
    > 22 files changed, 677 insertions(+), 280 deletions(-)

    And, this patch adds 4 new locks:

    sighand_struct->action_lock

    signal_struct->ctrl_lock
    signal_struct->shared_siglock

    task_struct->siglock

    Nice ;) For what? This should be justified, imho.


    Hmm. Just out of curiosity, I blindly applied the whole series and poke
    the _random_ function to look at, dequeue_signal(). And it looks wrong.

    spin_lock_irqsave(&current->signal->ctrl_lock, flags);
    current->jobctl |= JOBCTL_STOP_DEQUEUED;
    This signal->ctrl_lock can't help. A sig_kernel_stop() should be
    dequeued under the same lock, and we shouldn't release it unless we
    set JOBCTL_STOP_DEQUEUED. Otherwise we race with SIGCONT.

    May be do_signal_stop() does something special? At first flance it doesn't.
    But wait, it does while_each_thread() under ->ctrl_lock, why this is safe?

    May be I was just lucky ;)

    Oleg.



    \
     
     \ /
      Last update: 2011-09-30 18:59    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean