lkml.org 
[lkml]   [2007]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] RFC: have tcp_recvmsg() check kthread_should_stop() and treat it as if it were signalled
    On 06/27, Satyam Sharma wrote:
    >
    > Thanks for your comments, I'm still not convinced, however.

    An perhaps you are right. I don't have a very strong opinion on that.
    Still I can't understand why it is better if kthread_stop() sends a
    signal as well. Contrary, I believe we should avoid signals when it
    comes to kernel threads.

    One can always use force_sig() or allow_signal() + send_sig() when
    it is really needed, like cifs does.

    > On 6/26/07, Oleg Nesterov <oleg@tv-sign.ru> wrote:
    > >
    > >Personally, I don't think we should do this.
    > >
    > >kthread_stop() doesn't always mean "kill this thread asap". Suppose that
    > >CPU_DOWN does kthread_stop(workqueue->thread) but doesn't flush the queue
    > >before that (we did so before 2.6.22 and perhaps we will do again). Now
    > >work_struct->func() doing tcp_recvmsg() or wait_event_interruptible()
    > >fails,
    > >but this is probably not that we want.
    >
    > Anyway, I think _all_ usages of kthread_stop() in the kernel *do* want
    > the thread to stop *right then*. After all, kthread_stop() doesn't even
    > return (gets blocked on wait_for_completion()) till it knows the target
    > kthread *has* exited completely.

    Yes, kthread_stop(k) means that k should exit eventually, but I don't
    think that kthread_stop() should try to force the exit.

    > And if a workqueue is blocked on tcp_recvmsg() or skb_recv_datagram()
    > or some such, I don't see how that flush_workqueue (if that is what you
    > meant) would succeed anyway (unless you do send the signal too),

    timeout, but this was just a silly example. I am talking about the case
    when wait_event_interruptible() should not fail (unless something bad
    happens) inside the "while (!kthread_should_stop())" loop.

    Note also that kthread could use TASK_INTERRUPTIBLE sleep because it
    doesn't want to contribute to loadavg, and because it knows that all
    signals are ignored.

    > Note that the exact scenario you're talking about wouldn't mean the
    > kthread getting killed before it's supposed to be stopped anyway.

    Yes sure, we can't kill the kernel thread via signal. I meant we can have
    some unexpected failure.

    > >(offtopic)
    > >
    > > cifs_mount:
    > >
    > > send_sig(SIGKILL,srvTcp->tsk,1);
    > > tsk = srvTcp->tsk;
    > > if(tsk)
    > > kthread_stop(tsk);
    > >
    > >This "if(tsk)" looks wrong to me.
    >
    > I think it's bogus myself. [ Added linux-cifs-client@lists.samba.org to Cc:
    > ]
    >
    > >Can srvTcp->tsk be NULL? If yes, send_sig()
    > >is not safe. Can srvTcp->tsk become NULL after send_sig() ? If yes, this
    > >check is racy, and kthread_stop() is not safe.
    >
    > That's again something the atomicity I proposed above could avoid?

    I think this "if(tsk)" is just bogus, and should be killed.

    Oleg.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-06-27 14:27    [W:3.113 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site