<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux.git/drivers/md/dm-delay.c, branch v6.9</title>
<subtitle>Linux kernel source tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/'/>
<entry>
<title>dm-delay: avoid duplicate logic</title>
<updated>2023-11-17T19:41:14+00:00</updated>
<author>
<name>Mikulas Patocka</name>
<email>mpatocka@redhat.com</email>
</author>
<published>2023-11-17T17:24:04+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=ccadc8a21ef13eb80006ecff6a7466810e4f0dd6'/>
<id>ccadc8a21ef13eb80006ecff6a7466810e4f0dd6</id>
<content type='text'>
This is small refactoring of dm-delay - we avoid duplicate logic in
flush_delayed_bios and flush_delayed_bios_fast and join these two
functions into one.

We also add cond_resched() to flush_delayed_bios because the list may have
unbounded number of entries.

Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This is small refactoring of dm-delay - we avoid duplicate logic in
flush_delayed_bios and flush_delayed_bios_fast and join these two
functions into one.

We also add cond_resched() to flush_delayed_bios because the list may have
unbounded number of entries.

Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm-delay: fix bugs introduced by kthread mode</title>
<updated>2023-11-17T19:41:14+00:00</updated>
<author>
<name>Mikulas Patocka</name>
<email>mpatocka@redhat.com</email>
</author>
<published>2023-11-17T17:22:47+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=38cfff568169ff9f99784948f79f62ca1af5a187'/>
<id>38cfff568169ff9f99784948f79f62ca1af5a187</id>
<content type='text'>
This commit fixes the following bugs introduced by commit 70bbeb29fab0
("dm delay: for short delays, use kthread instead of timers and wq"):

* the function flush_worker_fn has no exit path - on unload, this
  function will just loop and consume 100% CPU without any progress

* the wake-up mechanism in flush_worker_fn is racy - a wake up will be
  missed if the process adds entries to the delayed_bios list just
  before set_current_state(TASK_INTERRUPTIBLE)

* flush_delayed_bios_fast submits a bio while holding a global mutex;
  this may deadlock if we have multiple stacked dm-delay devices and
  the underlying device attempts to acquire the mutex too

* if the target constructor fails, it will call delay_dtr. delay_dtr
  would attempt to free dc-&gt;timer_lock without it being initialized by
  the constructor.

* if the target constructor's kthread allocation fails, delay_dtr
  would crash trying to dereference dc-&gt;worker because it is non-NULL
  due to ERR_PTR.

Fixes: 70bbeb29fab0 ("dm delay: for short delays, use kthread instead of timers and wq")
Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This commit fixes the following bugs introduced by commit 70bbeb29fab0
("dm delay: for short delays, use kthread instead of timers and wq"):

* the function flush_worker_fn has no exit path - on unload, this
  function will just loop and consume 100% CPU without any progress

* the wake-up mechanism in flush_worker_fn is racy - a wake up will be
  missed if the process adds entries to the delayed_bios list just
  before set_current_state(TASK_INTERRUPTIBLE)

* flush_delayed_bios_fast submits a bio while holding a global mutex;
  this may deadlock if we have multiple stacked dm-delay devices and
  the underlying device attempts to acquire the mutex too

* if the target constructor fails, it will call delay_dtr. delay_dtr
  would attempt to free dc-&gt;timer_lock without it being initialized by
  the constructor.

* if the target constructor's kthread allocation fails, delay_dtr
  would crash trying to dereference dc-&gt;worker because it is non-NULL
  due to ERR_PTR.

Fixes: 70bbeb29fab0 ("dm delay: for short delays, use kthread instead of timers and wq")
Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm-delay: fix a race between delay_presuspend and delay_bio</title>
<updated>2023-11-17T19:38:46+00:00</updated>
<author>
<name>Mikulas Patocka</name>
<email>mpatocka@redhat.com</email>
</author>
<published>2023-11-17T17:21:14+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=6fc45b6ed921dc00dfb264dc08c7d67ee63d2656'/>
<id>6fc45b6ed921dc00dfb264dc08c7d67ee63d2656</id>
<content type='text'>
In delay_presuspend, we set the atomic variable may_delay and then stop
the timer and flush pending bios. The intention here is to prevent the
delay target from re-arming the timer again.

However, this test is racy. Suppose that one thread goes to delay_bio,
sees that dc-&gt;may_delay is one and proceeds; now, another thread executes
delay_presuspend, it sets dc-&gt;may_delay to zero, deletes the timer and
flushes pending bios. Then, the first thread continues and adds the bio to
delayed-&gt;list despite the fact that dc-&gt;may_delay is false.

Fix this bug by changing may_delay's type from atomic_t to bool and
only access it while holding the delayed_bios_lock mutex. Note that we
don't have to grab the mutex in delay_resume because there are no bios
in flight at this point.

Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Cc: stable@vger.kernel.org
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
In delay_presuspend, we set the atomic variable may_delay and then stop
the timer and flush pending bios. The intention here is to prevent the
delay target from re-arming the timer again.

However, this test is racy. Suppose that one thread goes to delay_bio,
sees that dc-&gt;may_delay is one and proceeds; now, another thread executes
delay_presuspend, it sets dc-&gt;may_delay to zero, deletes the timer and
flushes pending bios. Then, the first thread continues and adds the bio to
delayed-&gt;list despite the fact that dc-&gt;may_delay is false.

Fix this bug by changing may_delay's type from atomic_t to bool and
only access it while holding the delayed_bios_lock mutex. Note that we
don't have to grab the mutex in delay_resume because there are no bios
in flight at this point.

Signed-off-by: Mikulas Patocka &lt;mpatocka@redhat.com&gt;
Cc: stable@vger.kernel.org
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm delay: for short delays, use kthread instead of timers and wq</title>
<updated>2023-10-31T15:06:21+00:00</updated>
<author>
<name>Christian Loehle</name>
<email>christian.loehle@arm.com</email>
</author>
<published>2023-10-20T11:46:05+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=70bbeb29fab09d6ea6cfe64109db60a97d84d739'/>
<id>70bbeb29fab09d6ea6cfe64109db60a97d84d739</id>
<content type='text'>
DM delay's current design of using timers and wq to realize the delays
is insufficient for delays below ~50ms.

This commit enhances the design to use a kthread to flush the expired
delays, trading some CPU time (in some cases) for better delay
accuracy and delays closer to what the user requested for smaller
delays. The new design is chosen as long as all the delays are below
50ms.

Since bios can't be completed in interrupt context using a kthread
is probably the most reasonable way to approach this.

Testing with
echo "0 2097152 zero" | dmsetup create dm-zeros
for i in $(seq 0 20);
do
  echo "0 2097152 delay /dev/mapper/dm-zeros 0 $i" | dmsetup create dm-delay-${i}ms;
done

Some performance numbers for comparison, on beaglebone black (single
core) CONFIG_HZ_1000=y:

fio --name=1msread --rw=randread --bs=4k --runtime=60 --time_based \
    --filename=/dev/mapper/dm-delay-1ms
Theoretical maximum: 1000 IOPS
Previous: 250 IOPS
Kthread: 500 IOPS

fio --name=10msread --rw=randread --bs=4k --runtime=60 --time_based \
    --filename=/dev/mapper/dm-delay-10ms
Theoretical maximum: 100 IOPS
Previous: 45 IOPS
Kthread: 50 IOPS

fio --name=1mswrite --rw=randwrite --direct=1 --bs=4k --runtime=60 \
    --time_based --filename=/dev/mapper/dm-delay-1ms
Theoretical maximum: 1000 IOPS
Previous: 498 IOPS
Kthread: 1000 IOPS

fio --name=10mswrite --rw=randwrite --direct=1 --bs=4k --runtime=60 \
    --time_based --filename=/dev/mapper/dm-delay-10ms
Theoretical maximum: 100 IOPS
Previous: 90 IOPS
Kthread: 100 IOPS

(This one is just to prove the new design isn't impacting throughput,
not really about delays):
fio --name=10mswriteasync --rw=randwrite --direct=1 --bs=4k \
    --runtime=60 --time_based --filename=/dev/mapper/dm-delay-10ms \
    --numjobs=32 --iodepth=64 --ioengine=libaio --group_reporting
Previous: 13.3k IOPS
Kthread: 13.3k IOPS

Signed-off-by: Christian Loehle &lt;christian.loehle@arm.com&gt;
[Harshit: kthread_create error handling fix in delay_ctr]
Signed-off-by: Harshit Mogalapalli &lt;harshit.m.mogalapalli@oracle.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
DM delay's current design of using timers and wq to realize the delays
is insufficient for delays below ~50ms.

This commit enhances the design to use a kthread to flush the expired
delays, trading some CPU time (in some cases) for better delay
accuracy and delays closer to what the user requested for smaller
delays. The new design is chosen as long as all the delays are below
50ms.

Since bios can't be completed in interrupt context using a kthread
is probably the most reasonable way to approach this.

Testing with
echo "0 2097152 zero" | dmsetup create dm-zeros
for i in $(seq 0 20);
do
  echo "0 2097152 delay /dev/mapper/dm-zeros 0 $i" | dmsetup create dm-delay-${i}ms;
done

Some performance numbers for comparison, on beaglebone black (single
core) CONFIG_HZ_1000=y:

fio --name=1msread --rw=randread --bs=4k --runtime=60 --time_based \
    --filename=/dev/mapper/dm-delay-1ms
Theoretical maximum: 1000 IOPS
Previous: 250 IOPS
Kthread: 500 IOPS

fio --name=10msread --rw=randread --bs=4k --runtime=60 --time_based \
    --filename=/dev/mapper/dm-delay-10ms
Theoretical maximum: 100 IOPS
Previous: 45 IOPS
Kthread: 50 IOPS

fio --name=1mswrite --rw=randwrite --direct=1 --bs=4k --runtime=60 \
    --time_based --filename=/dev/mapper/dm-delay-1ms
Theoretical maximum: 1000 IOPS
Previous: 498 IOPS
Kthread: 1000 IOPS

fio --name=10mswrite --rw=randwrite --direct=1 --bs=4k --runtime=60 \
    --time_based --filename=/dev/mapper/dm-delay-10ms
Theoretical maximum: 100 IOPS
Previous: 90 IOPS
Kthread: 100 IOPS

(This one is just to prove the new design isn't impacting throughput,
not really about delays):
fio --name=10mswriteasync --rw=randwrite --direct=1 --bs=4k \
    --runtime=60 --time_based --filename=/dev/mapper/dm-delay-10ms \
    --numjobs=32 --iodepth=64 --ioengine=libaio --group_reporting
Previous: 13.3k IOPS
Kthread: 13.3k IOPS

Signed-off-by: Christian Loehle &lt;christian.loehle@arm.com&gt;
[Harshit: kthread_create error handling fix in delay_ctr]
Signed-off-by: Harshit Mogalapalli &lt;harshit.m.mogalapalli@oracle.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: add helper macro for simple DM target module init and exit</title>
<updated>2023-04-11T16:09:08+00:00</updated>
<author>
<name>Yangtao Li</name>
<email>frank.li@vivo.com</email>
</author>
<published>2023-04-09T16:43:37+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=3664ff82dae1ef9f14f7763d3dd30565e7ef9e14'/>
<id>3664ff82dae1ef9f14f7763d3dd30565e7ef9e14</id>
<content type='text'>
Eliminate duplicate boilerplate code for simple modules that contain
a single DM target driver without any additional setup code.

Add a new module_dm() macro, which replaces the module_init() and
module_exit() with template functions that call dm_register_target()
and dm_unregister_target() respectively.

Signed-off-by: Yangtao Li &lt;frank.li@vivo.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Eliminate duplicate boilerplate code for simple modules that contain
a single DM target driver without any additional setup code.

Add a new module_dm() macro, which replaces the module_init() and
module_exit() with template functions that call dm_register_target()
and dm_unregister_target() respectively.

Signed-off-by: Yangtao Li &lt;frank.li@vivo.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: push error reporting down to dm_register_target()</title>
<updated>2023-04-11T16:01:01+00:00</updated>
<author>
<name>Yangtao Li</name>
<email>frank.li@vivo.com</email>
</author>
<published>2023-03-18T13:16:33+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=b362c733ed7bf312ed729847bc26ba89febc556e'/>
<id>b362c733ed7bf312ed729847bc26ba89febc556e</id>
<content type='text'>
Simplifies each DM target's init method by making dm_register_target()
responsible for its error reporting (on behalf of targets).

Signed-off-by: Yangtao Li &lt;frank.li@vivo.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Simplifies each DM target's init method by making dm_register_target()
responsible for its error reporting (on behalf of targets).

Signed-off-by: Yangtao Li &lt;frank.li@vivo.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: change "unsigned" to "unsigned int"</title>
<updated>2023-02-14T19:23:06+00:00</updated>
<author>
<name>Heinz Mauelshagen</name>
<email>heinzm@redhat.com</email>
</author>
<published>2023-01-25T20:14:58+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=86a3238c7b9b759cb864f4f768ab2e24687dc0e6'/>
<id>86a3238c7b9b759cb864f4f768ab2e24687dc0e6</id>
<content type='text'>
Signed-off-by: Heinz Mauelshagen &lt;heinzm@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Signed-off-by: Heinz Mauelshagen &lt;heinzm@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: add missing SPDX-License-Indentifiers</title>
<updated>2023-02-14T19:23:06+00:00</updated>
<author>
<name>Heinz Mauelshagen</name>
<email>heinzm@redhat.com</email>
</author>
<published>2023-01-25T20:00:44+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=3bd940030752a33ff665eefdd74a1cdb74a4f9b0'/>
<id>3bd940030752a33ff665eefdd74a1cdb74a4f9b0</id>
<content type='text'>
'GPL-2.0-only' is used instead of 'GPL-2.0' because SPDX has
deprecated its use.

Suggested-by: John Wiele &lt;jwiele@redhat.com&gt;
Signed-off-by: Heinz Mauelshagen &lt;heinzm@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
'GPL-2.0-only' is used instead of 'GPL-2.0' because SPDX has
deprecated its use.

Suggested-by: John Wiele &lt;jwiele@redhat.com&gt;
Signed-off-by: Heinz Mauelshagen &lt;heinzm@redhat.com&gt;
Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: simplify basic targets</title>
<updated>2022-05-05T21:31:35+00:00</updated>
<author>
<name>Mike Snitzer</name>
<email>snitzer@kernel.org</email>
</author>
<published>2022-03-30T17:52:10+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=e86f2b005a51437d2887eec5ee659d0287d370ad'/>
<id>e86f2b005a51437d2887eec5ee659d0287d370ad</id>
<content type='text'>
Remove needless factoring and remap bi_sector regardless of
bio_sectors() being non-zero.

Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Remove needless factoring and remap bi_sector regardless of
bio_sectors() being non-zero.

Signed-off-by: Mike Snitzer &lt;snitzer@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>dm: simplify dm_sumbit_bio_remap interface</title>
<updated>2022-03-10T18:44:56+00:00</updated>
<author>
<name>Mike Snitzer</name>
<email>snitzer@redhat.com</email>
</author>
<published>2022-03-10T16:45:58+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=b7f8dff09827c96032c34a945ee7757e394b5952'/>
<id>b7f8dff09827c96032c34a945ee7757e394b5952</id>
<content type='text'>
Remove the from_wq argument from dm_sumbit_bio_remap(). Eliminates the
need for dm_sumbit_bio_remap() callers to know whether they are
calling for a workqueue or from the original dm_submit_bio().

Add map_task to dm_io struct, record the map_task in alloc_io and
clear it after all target -&gt;map() calls have completed. Update
dm_sumbit_bio_remap to check if 'current' matches io-&gt;map_task rather
than rely on passed 'from_rq' argument.

This change really simplifies the chore of porting each DM target to
using dm_sumbit_bio_remap() because there is no longer the risk of
programming error by not completely knowing all the different contexts
a particular method that calls dm_sumbit_bio_remap() might be used in.

Signed-off-by: Mike Snitzer &lt;snitzer@redhat.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Remove the from_wq argument from dm_sumbit_bio_remap(). Eliminates the
need for dm_sumbit_bio_remap() callers to know whether they are
calling for a workqueue or from the original dm_submit_bio().

Add map_task to dm_io struct, record the map_task in alloc_io and
clear it after all target -&gt;map() calls have completed. Update
dm_sumbit_bio_remap to check if 'current' matches io-&gt;map_task rather
than rely on passed 'from_rq' argument.

This change really simplifies the chore of porting each DM target to
using dm_sumbit_bio_remap() because there is no longer the risk of
programming error by not completely knowing all the different contexts
a particular method that calls dm_sumbit_bio_remap() might be used in.

Signed-off-by: Mike Snitzer &lt;snitzer@redhat.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
