On Fri, Apr 27, 2007 at 11:52:47AM +0400, Oleg Nesterov wrote:
> On 04/27, Jarek Poplawski wrote:
...
> > > Sorry, can't understand. done == 0 means that the queueing in progress,
> > > this work should be placed on cwq->worklist very soon, most probably
> > > right after we drop cwq->lock.
> >
>
On 04/27, Jarek Poplawski wrote:
>
> On Thu, Apr 26, 2007 at 08:34:06PM +0400, Oleg Nesterov wrote:
> > >
> > > > else if (test_and_set_bit(WORK_STRUCT_PENDING,
> > > > work_data_bits(work)))
> > > > done = del_timer(>timer)
> > >
> >
On 04/27, Jarek Poplawski wrote:
On Thu, Apr 26, 2007 at 08:34:06PM +0400, Oleg Nesterov wrote:
else if (test_and_set_bit(WORK_STRUCT_PENDING,
work_data_bits(work)))
done = del_timer(dwork-timer)
[...snip...]
On Fri, Apr 27, 2007 at 11:52:47AM +0400, Oleg Nesterov wrote:
On 04/27, Jarek Poplawski wrote:
...
Sorry, can't understand. done == 0 means that the queueing in progress,
this work should be placed on cwq-worklist very soon, most probably
right after we drop cwq-lock.
I think,
On Thu, Apr 26, 2007 at 08:44:36PM +0400, Oleg Nesterov wrote:
> On 04/26, Jarek Poplawski wrote:
> >
> > On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
> > ...
> > > > > > + spin_lock_irq(>lock);
> > > > > > + /* CPU_DEAD in progress may change cwq */
> > > > >
On Thu, Apr 26, 2007 at 08:34:06PM +0400, Oleg Nesterov wrote:
> On 04/26, Jarek Poplawski wrote:
> >
> > > void cancel_rearming_delayed_work(struct delayed_work *dwork)
> > > {
> > > struct work_struct *work = >work;
> > > struct cpu_workqueue_struct *cwq =
On 04/26, Jarek Poplawski wrote:
>
> On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
> ...
> > > > > + spin_lock_irq(>lock);
> > > > > + /* CPU_DEAD in progress may change cwq */
> > > > > + if (likely(cwq == get_wq_data(work))) {
> > > > > +
On 04/26, Jarek Poplawski wrote:
>
> > void cancel_rearming_delayed_work(struct delayed_work *dwork)
> > {
> > struct work_struct *work = >work;
> > struct cpu_workqueue_struct *cwq = get_wq_data(work);
> > int done;
>
> I don't understand, why you
On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
> On 04/25, Jarek Poplawski wrote:
...
> > > > + spin_lock_irq(>lock);
> > > > + /* CPU_DEAD in progress may change cwq */
> > > > + if (likely(cwq == get_wq_data(work))) {
> > > > +
On Wed, Apr 25, 2007 at 06:47:59PM +0400, Oleg Nesterov wrote:
> On 04/25, Oleg Nesterov wrote:
> >
> > On 04/25, Jarek Poplawski wrote:
> > >
> > > Probably this is also possible without timer i.e.
> > > with queue_work.
> >
> > Yes, thanks. While adding cpu-hotplug check I
On Wed, Apr 25, 2007 at 06:47:59PM +0400, Oleg Nesterov wrote:
On 04/25, Oleg Nesterov wrote:
On 04/25, Jarek Poplawski wrote:
Probably this is also possible without timer i.e.
with queue_work.
Yes, thanks. While adding cpu-hotplug check I forgot to add
On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
On 04/25, Jarek Poplawski wrote:
...
+ spin_lock_irq(cwq-lock);
+ /* CPU_DEAD in progress may change cwq */
+ if (likely(cwq == get_wq_data(work))) {
+
On 04/26, Jarek Poplawski wrote:
void cancel_rearming_delayed_work(struct delayed_work *dwork)
{
struct work_struct *work = dwork-work;
struct cpu_workqueue_struct *cwq = get_wq_data(work);
int done;
I don't understand, why you think cwq
On 04/26, Jarek Poplawski wrote:
On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
...
+ spin_lock_irq(cwq-lock);
+ /* CPU_DEAD in progress may change cwq */
+ if (likely(cwq == get_wq_data(work))) {
+
On Thu, Apr 26, 2007 at 08:34:06PM +0400, Oleg Nesterov wrote:
On 04/26, Jarek Poplawski wrote:
void cancel_rearming_delayed_work(struct delayed_work *dwork)
{
struct work_struct *work = dwork-work;
struct cpu_workqueue_struct *cwq = get_wq_data(work);
On Thu, Apr 26, 2007 at 08:44:36PM +0400, Oleg Nesterov wrote:
On 04/26, Jarek Poplawski wrote:
On Wed, Apr 25, 2007 at 04:47:14PM +0400, Oleg Nesterov wrote:
...
+ spin_lock_irq(cwq-lock);
+ /* CPU_DEAD in progress may change cwq */
+ if
On 04/25, Oleg Nesterov wrote:
>
> On 04/25, Jarek Poplawski wrote:
> >
> > Probably this is also possible without timer i.e.
> > with queue_work.
>
> Yes, thanks. While adding cpu-hotplug check I forgot to add ->current_work
> check, which is needed to actually implement this
On 04/25, Jarek Poplawski wrote:
>
> On Wed, Apr 25, 2007 at 02:20:38PM +0200, Jarek Poplawski wrote:
> > 2 cents more...
> ...
> > On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
> > > + do {
> > > + retry = 1;
>
> Of course this'll be shorter:
>
> retry =
On Wed, Apr 25, 2007 at 02:20:38PM +0200, Jarek Poplawski wrote:
> 2 cents more...
...
> On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
> > + do {
> > + retry = 1;
Of course this'll be shorter:
retry = 0;
> > + spin_lock_irq(>lock);
> > +
2 cents more...
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
...
> --- OLD/kernel/workqueue.c~1_CRDW 2007-04-13 17:43:23.0 +0400
> +++ OLD/kernel/workqueue.c2007-04-24 22:41:15.0 +0400
> @@ -242,11 +242,11 @@ static void run_workqueue(struct cpu_wor
...
>
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
> On 04/24, Jarek Poplawski wrote:
> >
> > This looks fine. Of course, it requires to remove some debugging
> > currently done with _PENDING flag
>
> For example?
Sorry!!! I don't know where I've seen those flags - maybe it's
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
On 04/24, Jarek Poplawski wrote:
This looks fine. Of course, it requires to remove some debugging
currently done with _PENDING flag
For example?
Sorry!!! I don't know where I've seen those flags - maybe it's
something with
2 cents more...
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
...
--- OLD/kernel/workqueue.c~1_CRDW 2007-04-13 17:43:23.0 +0400
+++ OLD/kernel/workqueue.c2007-04-24 22:41:15.0 +0400
@@ -242,11 +242,11 @@ static void run_workqueue(struct cpu_wor
...
On Wed, Apr 25, 2007 at 02:20:38PM +0200, Jarek Poplawski wrote:
2 cents more...
...
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
+ do {
+ retry = 1;
Of course this'll be shorter:
retry = 0;
+ spin_lock_irq(cwq-lock);
+
On 04/25, Jarek Poplawski wrote:
On Wed, Apr 25, 2007 at 02:20:38PM +0200, Jarek Poplawski wrote:
2 cents more...
...
On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
+ do {
+ retry = 1;
Of course this'll be shorter:
retry = 0;
No, this
On 04/25, Oleg Nesterov wrote:
On 04/25, Jarek Poplawski wrote:
Probably this is also possible without timer i.e.
with queue_work.
Yes, thanks. While adding cpu-hotplug check I forgot to add -current_work
check, which is needed to actually implement this
On 04/24, Jarek Poplawski wrote:
>
> This looks fine. Of course, it requires to remove some debugging
> currently done with _PENDING flag
For example?
>and it's hard to estimate this
> all before you do more, but it should be more foreseeable than
> current
On Mon, Apr 23, 2007 at 08:33:12PM +0400, Oleg Nesterov wrote:
> On 04/23, Jarek Poplawski wrote:
> >
> > On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
> > >
> > > First, this flag should be cleared after return from
> > > cancel_rearming_delayed_work().
> >
> > I think this
On Mon, Apr 23, 2007 at 08:33:12PM +0400, Oleg Nesterov wrote:
On 04/23, Jarek Poplawski wrote:
On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
First, this flag should be cleared after return from
cancel_rearming_delayed_work().
I think this flag, if at all,
On 04/24, Jarek Poplawski wrote:
This looks fine. Of course, it requires to remove some debugging
currently done with _PENDING flag
For example?
and it's hard to estimate this
all before you do more, but it should be more foreseeable than
current way. But
On 04/23, Jarek Poplawski wrote:
>
> On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
> >
> > First, this flag should be cleared after return from
> > cancel_rearming_delayed_work().
>
> I think this flag, if at all, probably should be cleared only
> consciously by the owner of a
On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
> On 04/20, Jarek Poplawski wrote:
> >
> > On Thu, Apr 19, 2007 at 02:21:22PM +0400, Oleg Nesterov wrote:
> > ...
> > > Yes. It would be better to use cancel_work_sync() instead of
> > > flush_workqueue()
> > > to make this less
On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
On 04/20, Jarek Poplawski wrote:
On Thu, Apr 19, 2007 at 02:21:22PM +0400, Oleg Nesterov wrote:
...
Yes. It would be better to use cancel_work_sync() instead of
flush_workqueue()
to make this less possible (because
On 04/23, Jarek Poplawski wrote:
On Fri, Apr 20, 2007 at 09:08:36PM +0400, Oleg Nesterov wrote:
First, this flag should be cleared after return from
cancel_rearming_delayed_work().
I think this flag, if at all, probably should be cleared only
consciously by the owner of a work, maybe
On 04/20, Jarek Poplawski wrote:
>
> On Thu, Apr 19, 2007 at 02:21:22PM +0400, Oleg Nesterov wrote:
> ...
> > Yes. It would be better to use cancel_work_sync() instead of
> > flush_workqueue()
> > to make this less possible (because cancel_work_sync() doesn't need to wait
> > for
> > the whole
(take 2)
I'm not sure we've agreed enough, who'll resubmit, so here
it is with WARN_ON. If it was submited already - forget it.
Jarek P.
--->
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained
On Fri, Apr 20, 2007 at 12:21:57PM +0200, Jarek Poplawski wrote:
> On Fri, Apr 20, 2007 at 06:53:54PM +1000, David Chinner wrote:
> ...
> > Yes, after spending another two hours working out why my fix was
> > then hanging in cancel_rearming_delayed_work() I was a little bit
> > annoyed at the now
On Fri, Apr 20, 2007 at 06:53:54PM +1000, David Chinner wrote:
...
> Yes, after spending another two hours working out why my fix was
> then hanging in cancel_rearming_delayed_work() I was a little bit
> annoyed at the now obviously misleading comment. Five minutes later
I agree with your
nel.org
> > Cc: Ingo Molnar <[EMAIL PROTECTED]>
> > Subject: [PATCH -mm] workqueue: debug possible endless loop in
> > cancel_rearming_delayed_work
At first I didn't spotted your thread is "independent" and
from your second message it seems you didn't receive all,
On Fri, Apr 20, 2007 at 10:13:26AM +0200, Jarek Poplawski wrote:
> On Fri, Apr 20, 2007 at 12:46:18AM +1000, David Chinner wrote:
> > On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
> > > Hi,
> > >
> > > IMHO cancel_rearming_delayed_work is dangerous place:
> >
> > Agreed - I
On Fri, Apr 20, 2007 at 12:46:18AM +1000, David Chinner wrote:
> On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
> > Hi,
> >
> > IMHO cancel_rearming_delayed_work is dangerous place:
>
> Agreed - I spent a couple of hours today learning why it
> can only be used on work
On Thu, Apr 19, 2007 at 01:07:11PM -0400, Chuck Ebbert wrote:
> Jarek Poplawski wrote:
> > Hi,
> >
> > IMHO cancel_rearming_delayed_work is dangerous place:
> >
> > - it assumes a work function always rearms (with no exception),
> > which probably isn't explained enough now (but anyway should
>
On Thu, Apr 19, 2007 at 01:07:11PM -0400, Chuck Ebbert wrote:
Jarek Poplawski wrote:
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained enough now (but anyway should
be checked in
On Fri, Apr 20, 2007 at 12:46:18AM +1000, David Chinner wrote:
On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
Agreed - I spent a couple of hours today learning why it
can only be used on work functions that
On Fri, Apr 20, 2007 at 10:13:26AM +0200, Jarek Poplawski wrote:
On Fri, Apr 20, 2007 at 12:46:18AM +1000, David Chinner wrote:
On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
Agreed - I spent a couple of
-mm] workqueue: debug possible endless loop in
cancel_rearming_delayed_work
At first I didn't spotted your thread is independent and
from your second message it seems you didn't receive all, you
expected. I wonder, if there is a need and possibility to add
somewhere (I would prefer Linus' tree
On Fri, Apr 20, 2007 at 06:53:54PM +1000, David Chinner wrote:
...
Yes, after spending another two hours working out why my fix was
then hanging in cancel_rearming_delayed_work() I was a little bit
annoyed at the now obviously misleading comment. Five minutes later
I agree with your feelings,
On Fri, Apr 20, 2007 at 12:21:57PM +0200, Jarek Poplawski wrote:
On Fri, Apr 20, 2007 at 06:53:54PM +1000, David Chinner wrote:
...
Yes, after spending another two hours working out why my fix was
then hanging in cancel_rearming_delayed_work() I was a little bit
annoyed at the now
(take 2)
I'm not sure we've agreed enough, who'll resubmit, so here
it is with WARN_ON. If it was submited already - forget it.
Jarek P.
---
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained
On 04/20, Jarek Poplawski wrote:
On Thu, Apr 19, 2007 at 02:21:22PM +0400, Oleg Nesterov wrote:
...
Yes. It would be better to use cancel_work_sync() instead of
flush_workqueue()
to make this less possible (because cancel_work_sync() doesn't need to wait
for
the whole -worklist),
Jarek Poplawski wrote:
> Hi,
>
> IMHO cancel_rearming_delayed_work is dangerous place:
>
> - it assumes a work function always rearms (with no exception),
> which probably isn't explained enough now (but anyway should
> be checked in such loops);
>
> - probably possible (theoretical) scenario:
On Thu, Apr 19, 2007 at 09:32:22AM +0200, Ingo Molnar wrote:
>
> * Jarek Poplawski <[EMAIL PROTECTED]> wrote:
>
> > + int i = 1000;
> >
> > - while (!cancel_delayed_work(dwork))
> > + while (!cancel_delayed_work(dwork)) {
> >
On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
> Hi,
>
> IMHO cancel_rearming_delayed_work is dangerous place:
Agreed - I spent a couple of hours today learning why it
can only be used on work functions that always rearm...
> - it assumes a work function always rearms (with no
On Thu, Apr 19, 2007 at 09:32:22AM +0200, Ingo Molnar wrote:
>
> * Jarek Poplawski <[EMAIL PROTECTED]> wrote:
>
> > + int i = 1000;
> >
> > - while (!cancel_delayed_work(dwork))
> > + while (!cancel_delayed_work(dwork)) {
> >
* Jarek Poplawski <[EMAIL PROTECTED]> wrote:
> + int i = 1000;
>
> - while (!cancel_delayed_work(dwork))
> + while (!cancel_delayed_work(dwork)) {
> flush_workqueue(wq);
> + BUG_ON(!i--);
> + }
if then
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained enough now (but anyway should
be checked in such loops);
- probably possible (theoretical) scenario: a few work
functions rearm themselves
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained enough now (but anyway should
be checked in such loops);
- probably possible (theoretical) scenario: a few work
functions rearm themselves
* Jarek Poplawski [EMAIL PROTECTED] wrote:
+ int i = 1000;
- while (!cancel_delayed_work(dwork))
+ while (!cancel_delayed_work(dwork)) {
flush_workqueue(wq);
+ BUG_ON(!i--);
+ }
if then make it a
On Thu, Apr 19, 2007 at 09:32:22AM +0200, Ingo Molnar wrote:
* Jarek Poplawski [EMAIL PROTECTED] wrote:
+ int i = 1000;
- while (!cancel_delayed_work(dwork))
+ while (!cancel_delayed_work(dwork)) {
flush_workqueue(wq);
+
On Thu, Apr 19, 2007 at 08:54:04AM +0200, Jarek Poplawski wrote:
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
Agreed - I spent a couple of hours today learning why it
can only be used on work functions that always rearm...
- it assumes a work function always rearms (with no
On Thu, Apr 19, 2007 at 09:32:22AM +0200, Ingo Molnar wrote:
* Jarek Poplawski [EMAIL PROTECTED] wrote:
+ int i = 1000;
- while (!cancel_delayed_work(dwork))
+ while (!cancel_delayed_work(dwork)) {
flush_workqueue(wq);
+
Jarek Poplawski wrote:
Hi,
IMHO cancel_rearming_delayed_work is dangerous place:
- it assumes a work function always rearms (with no exception),
which probably isn't explained enough now (but anyway should
be checked in such loops);
- probably possible (theoretical) scenario: a few
62 matches
Mail list logo
