linux-kernel - Re: [REV2: PATCH 1/2]: workqueue: Implement the kernel API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <509896.58523.qm@web35405.mail.mud.yahoo.com>
Date:	Tue, 30 Sep 2008 09:25:09 -0700 (PDT)
From:	Krishna Kumar <krikkku@...oo.com>
To:	linux-kernel@...r.kernel.org
Cc:	oleg@...sign.ru
Subject: Re: [REV2: PATCH 1/2]: workqueue: Implement the kernel API 

Hi Oleg,

(sending from a different email address)

> Oleg Nesterov <oleg@...sign.ru> wrote on 09/29/2008 07:57:34 PM:
>
> Yes. I must admit, I prefer the simple, non-intrusive code I suggested
> much more.
>
> Once again, the slow path is (at least, supposed to be) unlikely, and
> the difference is not that large. (I mean the slow path is when both
> queue() and update_timer() fail).
>
> Should we complicate the code to add this minor optimization (and
> btw pessimize the "normal" queue_delayed_work) ?
>
> And, once we have the correct and simple code, we can optimize it
> later.
>
> > I will go with the above
> > approach.
>
> No. Please do what _you_ think right ;)

No, you are right - I will go to the simpler (and bug-free?) interface.

> Yes. Please note that queue_delayed_work(work, 0) does not use the timer
> at all. IOW, queue_delayed_work(wq, work, 0) == queue_work(wq, &dwork->work).
> Perhaps (I don't know) update_queue_delayed_work() should do the same.
>
> From the next patch:
>
>       -       cancel_delayed_work(&afs_vlocation_reap);
>       -       schedule_delayed_work(&afs_vlocation_reap, 0);
>       +       schedule_update_delayed_work(&afs_vlocation_reap, 0);
>
> Again, I don't claim this is wrong, but please note that delay == 0.

As you stated in an earlier mail, the following code should handle all cases.
I think delay==0 is fine now, we take the costly (but rare) path.

int queue_update_delayed_work(struct workqueue_struct *wq,
                              struct delayed_work *dwork, unsigned long delay)
{
        int ret = 1;

        while (queue_delayed_work(wq, dwork, delay)) {
                unsigned long when = jiffies + delay;

                ret = 0;
                if (delay && update_timer_expiry(&dwork->timer, when))
                        break;
                cancel_work_sync(&dwork->work);
        }

        return ret;
}

I will run some tests and submit again.

Thanks once more for explaining patiently some very complicated portions :)

- KK


      Unlimited freedom, unlimited storage. Get it now, on http://help.yahoo.com/l/in/yahoo/mail/yahoomail/tools/tools-08.html/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/