From: Jan Kara <jack@suse.cz>
To: "dbasehore ." <dbasehore@chromium.org>
Cc: Tejun Heo <tj@kernel.org>, Jan Kara <jack@suse.cz>,
Alexander Viro <viro@zento.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
Kees Cook <keescook@chromium.org>,
linux-fsdevel@vger.kernel.org,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-mm@kvack.org, bleung@chromium.org, sonnyrao@chromium.org,
Luigi Semenzato <semenzato@chromium.org>
Subject: Re: [PATCH] backing_dev: Fix hung task on sync
Date: Mon, 17 Mar 2014 10:53:17 +0100 [thread overview]
Message-ID: <20140317095317.GD2210@quack.suse.cz> (raw)
In-Reply-To: <CAGAzgspTZnUh_qi=FeQ4hS4LRiexPccTyALMg3Gt1K0ZZq_MuQ@mail.gmail.com>
On Sat 15-03-14 13:22:53, dbasehore . wrote:
> Resurrecting this for further discussion about the root of the problem.
>
> mod_delayed_work_if_later addresses the problem one way, but the
> problem is still there for mod_delayed_work.
But flusher works care about only that one way, don't they? We always
want the flushing work to execute at min(timer so far, new time target). So
for that mod_delayed_work_if_later() works just fine.
> I think we could take
> another approach that doesn't modify the API, but still addresses
> (most of) the problem.
>
> mod_delayed_work currently removes a work item from a workqueue if it
> is on it. Correct me if I'm wrong, but I don't think that this is
> necessarily required for mod_delayed_work to have the current
> behavior. We should be able to set the timer while a delayed_work is
> currently on a workqueue. If the delayed_work is still on the
> workqueue when the timer goes off, everything is fine. If it has left
> the workqueue, we can queue it again.
But here you are relying on the fact that flusher works always want the
work to be executed immediately (i.e., they will be queued), or after some
fixed time T. So I agree what you suggest will work but changing the API as
Tejun described seems cleaner to me.
Honza
> On Wed, Feb 19, 2014 at 11:01 AM, Tejun Heo <tj@kernel.org> wrote:
> > Hello, Jan.
> >
> > On Wed, Feb 19, 2014 at 10:27:31AM +0100, Jan Kara wrote:
> >> You are the workqueue expert so you may know better ;) But the way I
> >> understand it is that queue_delayed_work() does nothing if the timer is
> >> already running. Since we queue flusher work to run either immediately or
> >> after dirty_writeback_interval we are safe to run queue_delayed_work()
> >> whenever we want it to run after dirty_writeback_interval and
> >> mod_delayed_work() whenever we want to run it immediately.
> >
> > Ah, okay, so it's always mod on immediate and queue on delayed. Yeah,
> > that should work.
> >
> >> But it's subtle and some interface where we could say queue delayed work
> >> after no later than X would be easier to grasp.
> >
> > Yeah, I think it'd be better if we had something like
> > mod_delayed_work_if_later(). Hmm...
> >
> > Thanks.
> >
> > --
> > tejun
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-03-17 9:53 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-15 4:12 [PATCH] backing_dev: Fix hung task on sync Derek Basehore
2014-02-17 9:20 ` Jan Kara
2014-02-18 22:55 ` Tejun Heo
2014-02-19 9:27 ` Jan Kara
2014-02-19 19:01 ` Tejun Heo
2014-03-11 18:23 ` Andrew Morton
2014-03-11 20:19 ` Jan Kara
2014-03-15 20:22 ` dbasehore .
2014-03-16 14:59 ` Tejun Heo
2014-03-16 19:13 ` dbasehore .
2014-03-16 20:20 ` dbasehore .
2014-03-17 14:40 ` Tejun Heo
2014-03-17 20:53 ` dbasehore .
2014-03-17 20:59 ` Tejun Heo
2014-03-17 9:53 ` Jan Kara [this message]
2014-02-19 15:31 ` dbasehore .
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=20140317095317.GD2210@quack.suse.cz \
--to=jack@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=bleung@chromium.org \
--cc=darrick.wong@oracle.com \
--cc=dbasehore@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=semenzato@chromium.org \
--cc=sonnyrao@chromium.org \
--cc=tj@kernel.org \
--cc=viro@zento.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).