linux-kernel - Re: [dm-devel] A review of dm-writeboost

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 08 Oct 2013 22:17:36 +0900
From:	Akira Hayakawa <ruby.wktk@...il.com>
To:	dm-devel@...hat.com
CC:	mpatocka@...hat.com, devel@...verdev.osuosl.org,
	thornber@...hat.com, snitzer@...hat.com, cesarb@...arb.net,
	gregkh@...uxfoundation.org, david@...morbit.com,
	linux-kernel@...r.kernel.org, tj@...nel.org, agk@...hat.com,
	joe@...ches.com, akpm@...ux-foundation.org, ejt@...hat.com,
	dan.carpenter@...cle.com, m.chehab@...sung.com, ruby.wktk@...il.com
Subject: Re: [dm-devel] A review of dm-writeboost

Mikulas,

Let me ask you about this comment
of choosing the best API.
For the rest, I will reply later.

> BTW. You should use wait_event_interruptible_lock_irq instead of 
> wait_event_interruptible and wait_event_interruptible_lock_irq_timeout 
> instead of wait_event_interruptible_timeout. The functions with "lock_irq" 
> automatically drop and re-acquire the spinlock, so that the condition is 
> tested while the lock is held - so that there are no asynchronous accesses 
> to !list_empty(&cache->flush_queue).

wait_event_interruptible_lock_irq_timeout is
added to the kernel since 3.12 by this patch.
https://lkml.org/lkml/2013/8/21/362
However, it is only used by zfcp itself.

I am afraid I want to refrain from this new API
and keep the code as it is now.
Later if this API is widely accepted
it is time to use it in writeboost
is my opinion.
The fact this API is not added for a long time
makes me feel it should not be used at least at this moment of time.
I want to use only those truly stable.
writeboost as a storage kernel module should be stable.
I believe depending only on the stable APIs is a
good way of making a software stable.

All said, I tried to fix this.
Is this change what you meant?

@@ -24,10 +24,10 @@ int flush_proc(void *data)

                spin_lock_irqsave(&cache->flush_queue_lock, flags);
                while (list_empty(&cache->flush_queue)) {
-                       spin_unlock_irqrestore(&cache->flush_queue_lock, flags);
-                       wait_event_interruptible_timeout(
+                       wait_event_interruptible_lock_irq_timeout(
                                cache->flush_wait_queue,
-                               (!list_empty(&cache->flush_queue)),
+                               !list_empty(&cache->flush_queue),
+                               cache->flush_queue_lock,
                                msecs_to_jiffies(100));

                        /*
@@ -36,8 +36,6 @@ int flush_proc(void *data)
                         */
                        if (kthread_should_stop())
                                return 0;
-                       else
-                               spin_lock_irqsave(&cache->flush_queue_lock, flags);
                }

Akira
--
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/