From: Roland Dreier <roland@topspin.com>
To: Andrew Morton <akpm@digeo.com>
Cc: "Shawn Starr" <spstarr@sh0n.net>,
rml@tech9.net, rmk@arm.linux.org.uk,
linux-kernel@vger.kernel.org
Subject: Re: [BUG][2.5.66bk9+] - tty hangings - patches, dmesg & sysctl+T info
Date: 08 Apr 2003 21:47:55 -0700 [thread overview]
Message-ID: <52znn0mg3o.fsf@topspin.com> (raw)
In-Reply-To: <20030408211216.71022d84.akpm@digeo.com>
Andrew> Well it does look like you've hit the flush_workqueue
Andrew> livelock.
I don't think it's really livelock. I think it's just the fact that
his kernel (with your tty-shutdown-race-fix patch) does
del_timer_sync() without decrementing nr_queued() and so
flush_workqueue() never returns.
Still, I like the idea of this patch, since it resolves the livelock.
But I don't think the implementation is quite right. insert_sequence
doesn't get incremented until delayed_work_timer_fn(). That means
that a driver (tty_io.c, for example) could call
schedule_delayed_work(), then call flush_scheduled_work() before
delayed_work_timer_fn() has run for that work.
In that case schedule_delayed_work() could return immediately because
insert_sequence and remove_sequence are (probably) equal. Then
delayed_work_timer_fn() runs after the driver exits, and we're back
with the original problem (running a freed timer).
It should be pretty easy to rejigger the patch so that it works
correctly, just by moving the cwq->insert_sequence++ from
delayed_work_timer_fn() into queue_delayed_work() (right before the
add_timer(), say). I'm still not positive that this covers
everything; I need to think a little harder.
In any case, I think we still have to do something to fix
release_dev() in tty_io.c. It seems we should at least add the
clear_bit(TTY_DONT_FLIP, &tty->flags); however, I'm not familiar
enough with how the tty driver works to know whether TTY_DONT_FLIP
could get set again (while we're waiting for flush_scheduled_work()).
If so we would also need something along the lines of
cancel_delayed_work(&tty->flip.work).
Shawn, I think the patch I just posted a little while ago (with a
fixed cancel_delayed_work() implementation) is more likely to cure
your tty hanging right now. However, I think something along the
lines of this patch from Andrew is a better solution in the long run.
- Roland
next prev parent reply other threads:[~2003-04-09 4:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-04-06 20:09 [BUG][2.5.66bk9+] - changes to timers still broken - we don't oops anymore Shawn Starr
2003-04-06 20:20 ` Roland Dreier
2003-04-06 20:38 ` Andrew Morton
2003-04-06 21:00 ` Shawn Starr
2003-04-09 2:12 ` [BUG][2.5.66bk9+] - tty hangings - patches, dmesg & sysctl+T info Shawn Starr
2003-04-09 4:12 ` Andrew Morton
2003-04-09 4:47 ` Roland Dreier [this message]
2003-04-09 4:56 ` Andrew Morton
2003-04-09 4:16 ` Roland Dreier
2003-04-09 4:27 ` Andrew Morton
2003-04-09 4:52 ` Roland Dreier
2003-04-09 5:01 ` Shawn Starr
-- strict thread matches above, loose matches on Subject: below --
2003-04-12 16:08 Shawn Starr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52znn0mg3o.fsf@topspin.com \
--to=roland@topspin.com \
--cc=akpm@digeo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rmk@arm.linux.org.uk \
--cc=rml@tech9.net \
--cc=spstarr@sh0n.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.