* Re: dm stripe: add DAX support
From: Mike Snitzer @ 2016-07-13 2:01 UTC (permalink / raw)
To: Kani, Toshimitsu, axboe-b10kYP2dOMg
Cc: linux-nvdimm-y27Ovi1pjclAfugRpC6u6w@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <1468362104.8908.43.camel-ZPxbGqLxI0U@public.gmane.org>
On Tue, Jul 12 2016 at 6:22pm -0400,
Kani, Toshimitsu <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
> On Fri, 2016-06-24 at 14:29 -0400, Mike Snitzer wrote:
> >
> > BTW, if in your testing you could evaluate/quantify any extra overhead
> > from DM that'd be useful to share. It could be there are bottlenecks
> > that need to be fixed, etc.
>
> Here are some results from fio benchmark. The test is single-threaded and is
> bound to one CPU.
>
> DAX LVM IOPS NOTE
> ---------------------------------------
> Y N 790K
> Y Y 754K 5% overhead with LVM
> N N 567K
> N Y 457K 20% overhead with LVM
>
> DAX: Y: mount -o dax,noatime, N: mount -o noatime
> LVM: Y: dm-linear on pmem0 device, N: pmem0 device
> fio: bs=4k, size=2G, direct=1, rw=randread, numjobs=1
>
> Among the 5% overhead with DAX/LVM, the new DM direct_access interfaces
> account for less than 0.5%.
>
> dm_blk_direct_access 0.28%
> linear_direct_access 0.17%
>
> The average latency increases slightly from 0.93us to 0.95us. I think most of
> the overhead comes from the submit_bio() path, which is used only for
> accessing metadata with DAX. I believe this is due to cloning bio for each
> request in DM. There is 12% more L2 miss in total.
>
> Without DAX, 20% overhead is observed with LVM. Average latency increases
> from 1.39us to 1.82us. Without DAX, bio is cloned for both data and metadata.
Thanks for putting this summary together. Unfortunately none of the DM
changes can be queued for 4.8 until Jens takes the 2 block core patches:
https://patchwork.kernel.org/patch/9196021/
https://patchwork.kernel.org/patch/9196019/
Not sure what the hold up and/or issue is with them. But I've asked
twice (and implicilty a 3rd time here). Hopefully they land in time for
4.8.
Mike
^ permalink raw reply
* Help with assembling a stopped array
From: Vegard Haugland @ 2016-07-13 1:23 UTC (permalink / raw)
To: linux-raid
Hi all,
I have a raid6 setup (called /dev/md4) with 10 harddrives, ranged from
sda to sdj.
One of my drives fell out recently, /dev/sde3/ (not entirely sure how
this happened), but I readded it as /dev/sdk3, and the array started
rebuilding. This went on for several hours, and it hit 60% completed
around 8 hours ago.
However, a few hours after that, I lost both /dev/sdg3 and /dev/sdh3 -
and the array stopped completely. My OS was also running on that
array, so my entire system freezed forcing me to reboot.
I've now booted into busybox and md tells me that it cannot reassemble
the array as it can only find 7 out of 10 good drives..
I then read the instructions on
https://raid.wiki.kernel.org/index.php/RAID_Recovery, I started using
mdadm --examine and found that 8 out of 10 drives has identical event
counters. The two that deviates from this is /dev/sdg3 and /dev/sdh3.
I'm not entirely sure if the disk symbolic names are still the same,
so I further assume that the 8/10 drives that are still good are the
ones that were running OK before any of this started happening.
As such, I try forcing a reassemble using mdadm -A /dev/md4 --force.
When I now run mdadm -D /md4, it lists one device as spare (even
though my array does not use any spares) - and I guess this is the
main reason the array doesn't start. Can anyone confirm this, or am i
missing something?
Also, if I try setting the spare as failed by running mdadm -f
/dev/md4 /dev/sde3, I get "no such device" (as recalled from human
memory). Removing it works (mdadm -r /dev/md4 /dev/sde3), but every
time I reassemble, it gets the "spare" tag. I also tried reassemble
with force, but the same happened..
The initrd on this computer does not have networking unfortunately, so
I cannot attach any output from any logs.
If you agree with my summary so far, what should be my next action? Is
there anything left to try before running mdadm create (with level 6
and 2 missing drives)?
Best regards,
Vegard Haugland
--
Vegard Haugland <vegard@haugland.at>
Mob. +47 976 68 262
Web: www.v3gard.com | foto.haugland.at
If you want to send me encrypted email, my public GPG key is located
at http://v3gard.com/files/gpg.pub
^ permalink raw reply
* Re: dm stripe: add DAX support
From: Kani, Toshimitsu @ 2016-07-12 22:22 UTC (permalink / raw)
To: snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-nvdimm-y27Ovi1pjclAfugRpC6u6w@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160624182859.GD13898-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, 2016-06-24 at 14:29 -0400, Mike Snitzer wrote:
>
> BTW, if in your testing you could evaluate/quantify any extra overhead
> from DM that'd be useful to share. It could be there are bottlenecks
> that need to be fixed, etc.
Here are some results from fio benchmark. The test is single-threaded and is
bound to one CPU.
DAX LVM IOPS NOTE
---------------------------------------
Y N 790K
Y Y 754K 5% overhead with LVM
N N 567K
N Y 457K 20% overhead with LVM
DAX: Y: mount -o dax,noatime, N: mount -o noatime
LVM: Y: dm-linear on pmem0 device, N: pmem0 device
fio: bs=4k, size=2G, direct=1, rw=randread, numjobs=1
Among the 5% overhead with DAX/LVM, the new DM direct_access interfaces
account for less than 0.5%.
dm_blk_direct_access 0.28%
linear_direct_access 0.17%
The average latency increases slightly from 0.93us to 0.95us. I think most of
the overhead comes from the submit_bio() path, which is used only for
accessing metadata with DAX. I believe this is due to cloning bio for each
request in DM. There is 12% more L2 miss in total.
Without DAX, 20% overhead is observed with LVM. Average latency increases
from 1.39us to 1.82us. Without DAX, bio is cloned for both data and metadata.
Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
* Re: [PATCH RFC] md/raid1: fix deadlock between freeze_array() and wait_barrier().
From: NeilBrown @ 2016-07-12 22:14 UTC (permalink / raw)
To: Alexander Lyakas
Cc: 马建朋, linux-raid, Jes Sorensen, Shaohua Li
In-Reply-To: <CAGRgLy4UgT_gGHL=Lx_By1fqmE4Q4pwvbhBTh0J+u=9gxuQOYQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4987 bytes --]
On Tue, Jul 12 2016, Alexander Lyakas wrote:
> Hello Neil,
>
> Thank you for your response. I read an email about you retiring from
> MD/mdadm maintenance and delegating mdadm maintenance to Jes Sorensen.
> But I was wondering who will be responsible for MD maintenance, and
> was about to send an email asking that.
Yes, I no longer have maintainership responsibilities, though I'm still
involved to some extent. Jes Sorensen is looking after mdadm and
Shaohua Li is looking after the kernel driver (as listed in MAINTAINERS).
>
> On Fri, Jul 8, 2016 at 2:41 AM, NeilBrown <neilb@suse.com> wrote:
>> On Mon, Jun 27 2016, Alexander Lyakas wrote:
>>
>>> When we call wait_barrier, we might have some bios waiting
>>> in current->bio_list, which prevents the array_freeze call to
>>> complete. Those can only be internal READs, which have already
>>> passed the wait_barrier call (thus incrementing nr_pending), but
>>> still were not submitted to the lower level, due to generic_make_request
>>> logic to avoid recursive calls. In such case, we have a deadlock:
>>> - array_frozen is already set to 1, so wait_barrier unconditionally waits, so
>>> - internal READ bios will not be submitted, thus freeze_array will
>>> never completes
>>>
>>> This problem was originally fixed in commit:
>>> d6b42dc md/raid1,raid10: avoid deadlock during resync/recovery.
>>>
>>> But then it was broken in commit:
>>> b364e3d raid1: Add a field array_frozen to indicate whether raid in
>>> freeze state.
>>
>> Thanks for the great analysis.
>> I think this primarily a problem in generic_make_request(). It queues
>> requests in the *wrong* order.
>>
>> Please try the patch from
>> https://lkml.org/lkml/2016/7/7/428
>>
>> and see if it helps. If two requests for a raid1 are in the
>> generic_make_request queue, this patch causes the sub-requests created
>> by the first to be handled before the second is attempted.
> I have read this discussion and more or less (probably less than more)
> understood that the second patch by Lars is supposed to address our
> issue. However, we cannot easily apply that patch:
> - The patch is based on structures added by earlier patch "[RFC]
> block: fix blk_queue_split() resource exhaustion".
> - Both patches are not in the mainline tree yet.
> - Both patches are in block core, which requires to recompile the whole kernel.
> - Not sure if these patches are applicable for our production kernel
> 3.18 (long term)
>
> I am sure you understand that for production with our current kernel
> 3.18 (long term) we cannot go with these two patches.
This patch takes the basic concept of those two and applies it just to
raid1 and raid10. I think it should be sufficient. Can you test? The
patch is against 3.18
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 40b35be34f8d..99208aa2c1c8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1229,7 +1229,7 @@ read_again:
sectors_handled;
goto read_again;
} else
- generic_make_request(read_bio);
+ bio_list_add_head(current->bio_list, read_bio);
return;
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 32e282f4c83c..c528102b80b6 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1288,7 +1288,7 @@ read_again:
sectors_handled;
goto read_again;
} else
- generic_make_request(read_bio);
+ bio_list_add_head(¤t->bio_list, read_bio);
return;
}
>
> Since this issue is a real deadlock we are hitting in a long-term 3.18
> kernel, is there any chance for cc-stable fix? Currently we applied
> the rudimentary fix I posted. It basically switches context for
> problematic RAID1 READs, and runs them from a different context. With
> this fix we don't see the deadlock anymore.
>
> Also, can you please comment on another concern I expressed:
> freeze_array() is now not reentrant. Meaning that if two threads call
> it in parallel (and it could happen for the same MD), the first thread
> calling unfreeze_array will mess up things for the second thread.
Yes, that is a regression. This should be enough to fix it. Do you
agree?
Thanks,
NeilBrown
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 40b35be34f8d..5ad25c7d7453 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -984,7 +984,7 @@ static void freeze_array(struct r1conf *conf, int extra)
* we continue.
*/
spin_lock_irq(&conf->resync_lock);
- conf->array_frozen = 1;
+ conf->array_frozen += 1;
wait_event_lock_irq_cmd(conf->wait_barrier,
conf->nr_pending == conf->nr_queued+extra,
conf->resync_lock,
@@ -995,7 +995,7 @@ static void unfreeze_array(struct r1conf *conf)
{
/* reverse the effect of the freeze */
spin_lock_irq(&conf->resync_lock);
- conf->array_frozen = 0;
+ conf->array_frozen -= 1;
wake_up(&conf->wait_barrier);
spin_unlock_irq(&conf->resync_lock);
}
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related
* kernel checksumming performance vs actual raid device performance
From: Matt Garman @ 2016-07-12 21:09 UTC (permalink / raw)
To: Mdadm
We have a system with a 24-disk raid6 array, using 2TB SSDs. We use
this system in a workload that is 99.9% read-only (a few small
writes/day, versus countless reads). This system is an NFS server for
about 50 compute nodes that continually read its data.
In a non-degraded state, the system works wonderfully: the md0_raid6
process uses less than 1% CPU, each drive is around 20% utilization
(via iostat), no swapping is taking place. The outbound throughput
averages around 2.0 GB/sec, with 2.5 GB/sec peaks.
However, we had a disk fail, and the throughput dropped considerably,
with the md0_raid6 process pegged at 100% CPU.
I understand that data from the failed disk will need to be
reconstructed from parity, and this will cause the md0_raid6 process
to consume considerable CPU.
What I don't understand is how I can determine what kind of actual MD
device performance (throughput) I can expect in this state?
Dmesg seems to give some hints:
[ 6.386820] xor: automatically using best checksumming function:
[ 6.396690] avx : 24064.000 MB/sec
[ 6.414706] raid6: sse2x1 gen() 7636 MB/s
[ 6.431725] raid6: sse2x2 gen() 3656 MB/s
[ 6.448742] raid6: sse2x4 gen() 3917 MB/s
[ 6.465753] raid6: avx2x1 gen() 5425 MB/s
[ 6.482766] raid6: avx2x2 gen() 7593 MB/s
[ 6.499773] raid6: avx2x4 gen() 8648 MB/s
[ 6.499773] raid6: using algorithm avx2x4 gen() (8648 MB/s)
[ 6.499774] raid6: using avx2x2 recovery algorithm
(CPU is: Intel(R) Xeon(R) CPU E5-2620 v3 @ 2.40GHz.)
Perhaps naively, I would expect that second-to-last line:
[ 6.499773] raid6: using algorithm avx2x4 gen() (8648 MB/s)
to indicate what kind of throughput I could expect in a degraded
state, but clearly that is not right---or I have something
misconfigured.
So in other words, what does that gen() 8648 MB/s metric mean in terms
of real-world throughput? Is there a way I can "convert" that number
to expected throughput of a degraded array?
Thanks,
Matt
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Matthias Dahl @ 2016-07-12 14:56 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <20160712140715.GL14586@dhcp22.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]
Hello Michal...
On 2016-07-12 16:07, Michal Hocko wrote:
> /proc/slabinfo could at least point on who is eating that memory.
Thanks. I have made another test (and thus again put the RAID10 out of
sync for the 100th time, sigh) and made regular snapshots of slabinfo
which I have attached to this mail.
> Direct IO doesn't get throttled like buffered IO.
Is buffered i/o not used in both cases if I don't explicitly request
direct i/o?
dd if=/dev/zero /dev/md126p5 bs=512K
and dd if=/dev/zero /dev/mapper/test-device bs=512K
Given that the test-device is dm-crypt on md125p5. Aren't both using
buffered i/o?
> the number of pages under writeback was more or less same throughout
> the time but there are some local fluctuations when some pages do get
> completed.
The pages under writeback are those directly destined for the disk, so
after dm-crypt had done its encryption?
> If not you can enable allocator trace point for a particular object
> size (or range of sizes) and see who is requesting them.
If that support is baked into the Fedora provided kernel that is. If
you could give me a few hints or pointers, how to properly do a
allocator
trace point and get some decent data out of it, that would be nice.
Thanks,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
[-- Attachment #2: slabinfo.txt.gz --]
[-- Type: application/x-gzip, Size: 30968 bytes --]
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Michal Hocko @ 2016-07-12 14:07 UTC (permalink / raw)
To: Matthias Dahl; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <e6c2087730e530e77c2b12d50495bdc9@mail.ud19.udmedia.de>
On Tue 12-07-16 14:42:12, Matthias Dahl wrote:
> Hello Michal...
>
> On 2016-07-12 13:49, Michal Hocko wrote:
>
> > I am not a storage expert (not even mention dm-crypt). But what those
> > counters say is that the IO completion doesn't trigger so the
> > PageWriteback flag is still set. Such a page is not reclaimable
> > obviously. So I would check the IO delivery path and focus on the
> > potential dm-crypt involvement if you suspect this is a contributing
> > factor.
>
> Sounds reasonable... except that I have no clue how to trace that with
> the limited means I have at my disposal right now and with the limited
> knowledge I have of the kernel internals. ;-)
I guess dm resp. block layer experts would be much better to advise
here.
> > Who is consuming those objects? Where is the rest 70% of memory hiding?
>
> Is there any way to get a more detailed listing of where the memory is
> spent while dd is running? Something I could pipe every 500ms or so for
> later analysis or so?
/proc/slabinfo could at least point on who is eating that memory.
> > Writer will get throttled but the concurrent memory consumer will not
> > normally. So you can end up in this situation.
>
> Hm, okay. I am still confused though: If I, for example, let dd do the
> exact same thing on a raw partition on the RAID10, nothing like that
> happens. Wouldn't we have the same race and problem then too...?
Direct IO doesn't get throttled like buffered IO.
> It is
> only with dm-crypt in-between that all of this shows itself. But I do
> somehow suspect the RAID10 Intel Rapid Storage to be the cause or at
> least partially.
Well, there are many allocation failures for GFP_ATOMIC requests
from scsi_request_fn path. AFAIU the code, the request is deferred
and retried later. I cannot find any path which would do regular
__GFP_DIRECT_RECLAIM fallback allocation. So while GFP_ATOMIC is
___GFP_KSWAPD_RECLAIM so it would kick kswapd which should reclaim some
memory there is no guarantee for a forward progress. Anyway, I believe
even __GFP_DIRECT_RECLAIM allocation wouldn't help much in this
particular case. There is not much of a reclaimable memory left - most
of it is dirty/writeback - so we are close to a deadlock state.
But we do not seem to be stuck completely:
unevictable:12341 dirty:90458 writeback:651272 unstable:0
unevictable:12341 dirty:90458 writeback:651272 unstable:0
unevictable:12341 dirty:90458 writeback:651272 unstable:0
unevictable:12341 dirty:90222 writeback:651252 unstable:0
unevictable:12341 dirty:90222 writeback:651231 unstable:0
unevictable:12341 dirty:89321 writeback:651905 unstable:0
unevictable:12341 dirty:89212 writeback:652014 unstable:0
unevictable:12341 dirty:89212 writeback:651993 unstable:0
unevictable:12341 dirty:89212 writeback:651993 unstable:0
unevictable:12488 dirty:42892 writeback:656597 unstable:0
unevictable:12488 dirty:42783 writeback:656597 unstable:0
unevictable:12488 dirty:42125 writeback:657125 unstable:0
unevictable:12488 dirty:42125 writeback:657125 unstable:0
unevictable:12488 dirty:42125 writeback:657125 unstable:0
unevictable:12488 dirty:42125 writeback:657125 unstable:0
unevictable:12556 dirty:54778 writeback:648616 unstable:0
unevictable:12556 dirty:54168 writeback:648919 unstable:0
unevictable:12556 dirty:54168 writeback:648919 unstable:0
unevictable:12556 dirty:53237 writeback:649506 unstable:0
unevictable:12556 dirty:53237 writeback:649506 unstable:0
unevictable:12556 dirty:53128 writeback:649615 unstable:0
unevictable:12556 dirty:53128 writeback:649615 unstable:0
unevictable:12556 dirty:52256 writeback:650159 unstable:0
unevictable:12556 dirty:52256 writeback:650159 unstable:0
unevictable:12556 dirty:52256 writeback:650138 unstable:0
unevictable:12635 dirty:49929 writeback:650724 unstable:0
unevictable:12635 dirty:49820 writeback:650833 unstable:0
unevictable:12635 dirty:49820 writeback:650833 unstable:0
unevictable:12635 dirty:49820 writeback:650833 unstable:0
unevictable:13001 dirty:167859 writeback:651864 unstable:0
unevictable:13001 dirty:167672 writeback:652038 unstable:0
the number of pages under writeback was more or less same throughout
the time but there are some local fluctuations when some pages do get
completed.
That being said, I believe that IO is stuck due to lack of memory which
is caused by some memory leak or excessive memory consumption. Finding
out who that might be would be the first step. /proc/slabinfo should
show us which slab cache is backing so many unreclaimable objects. If we
are lucky it will be something easier to match to a code. If not you can
enable allocator trace point for a particular object size (or range of
sizes) and see who is requesting them.
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply
* Voy a estar esperando por su respuesta
From: CHAN CHAK @ 2016-07-12 13:44 UTC (permalink / raw)
Hola,
Mi nombre es Chan Chak un gerente de banco con un banco de inversión,
tengo un negocio de beneficio mutuo que implica la transferencia de
grandes sumas de dinero.
Volver a mí para obtener más información si está
interesado.(chanjohnchak@gmail.com)
CHAN CHAK
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Matthias Dahl @ 2016-07-12 12:42 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <20160712114920.GF14586@dhcp22.suse.cz>
Hello Michal...
On 2016-07-12 13:49, Michal Hocko wrote:
> I am not a storage expert (not even mention dm-crypt). But what those
> counters say is that the IO completion doesn't trigger so the
> PageWriteback flag is still set. Such a page is not reclaimable
> obviously. So I would check the IO delivery path and focus on the
> potential dm-crypt involvement if you suspect this is a contributing
> factor.
Sounds reasonable... except that I have no clue how to trace that with
the limited means I have at my disposal right now and with the limited
knowledge I have of the kernel internals. ;-)
> Who is consuming those objects? Where is the rest 70% of memory hiding?
Is there any way to get a more detailed listing of where the memory is
spent while dd is running? Something I could pipe every 500ms or so for
later analysis or so?
> Writer will get throttled but the concurrent memory consumer will not
> normally. So you can end up in this situation.
Hm, okay. I am still confused though: If I, for example, let dd do the
exact same thing on a raw partition on the RAID10, nothing like that
happens. Wouldn't we have the same race and problem then too...? It is
only with dm-crypt in-between that all of this shows itself. But I do
somehow suspect the RAID10 Intel Rapid Storage to be the cause or at
least partially.
Like I said, if you have any pointers how I could further trace this
or figure out who is exactly consuming what memory, that would be very
helpful... Thanks.
So long,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
--
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>
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Michal Hocko @ 2016-07-12 11:59 UTC (permalink / raw)
To: Matthias Dahl; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <20160712114920.GF14586@dhcp22.suse.cz>
On Tue 12-07-16 13:49:20, Michal Hocko wrote:
> On Tue 12-07-16 13:28:12, Matthias Dahl wrote:
> > Hello Michal...
> >
> > On 2016-07-12 11:50, Michal Hocko wrote:
> >
> > > This smells like file pages are stuck in the writeback somewhere and the
> > > anon memory is not reclaimable because you do not have any swap device.
> >
> > Not having a swap device shouldn't be a problem -- and in this case, it
> > would cause even more trouble as in disk i/o.
> >
> > What could cause the file pages to get stuck or stopped from being written
> > to the disk? And more importantly, what is so unique/special about the
> > Intel Rapid Storage that it happens (seemingly) exclusively with that
> > and not the the normal Linux s/w raid support?
>
> I am not a storage expert (not even mention dm-crypt). But what those
> counters say is that the IO completion doesn't trigger so the
> PageWriteback flag is still set. Such a page is not reclaimable
> obviously. So I would check the IO delivery path and focus on the
> potential dm-crypt involvement if you suspect this is a contributing
> factor.
>
> > Also, if the pages are not written to disk, shouldn't something error
> > out or slow dd down?
>
> Writers are normally throttled when we the dirty limit. You seem to have
> dirty_ratio set to 20% which is quite a lot considering how much memory
> you have.
And just to clarify. dirty_ratio refers to dirtyable memory which is
free_pages+file_lru pages. In your case you you have only 9% of the total
memory size dirty/writeback but that is 90% of dirtyable memory. This is
quite possible if somebody consumes free_pages racing with the writer.
Writer will get throttled but the concurrent memory consumer will not
normally. So you can end up in this situation.
> If you get back to the memory info from the OOM killer report:
> [18907.592209] active_anon:110314 inactive_anon:295 isolated_anon:0
> active_file:27534 inactive_file:819673 isolated_file:160
> unevictable:13001 dirty:167859 writeback:651864 unstable:0
> slab_reclaimable:177477 slab_unreclaimable:1817501
> mapped:934 shmem:588 pagetables:7109 bounce:0
> free:49928 free_pcp:45 free_cma:0
>
> The dirty+writeback is ~9%. What is more interesting, though, LRU
> pages are negligible to the memory size (~11%). Note the numer of
> unreclaimable slab pages (~20%). Who is consuming those objects?
> Where is the rest 70% of memory hiding?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Michal Hocko @ 2016-07-12 11:49 UTC (permalink / raw)
To: Matthias Dahl; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <d9dbe0328e938eb7544fdb2aa8b5a9c7@mail.ud19.udmedia.de>
On Tue 12-07-16 13:28:12, Matthias Dahl wrote:
> Hello Michal...
>
> On 2016-07-12 11:50, Michal Hocko wrote:
>
> > This smells like file pages are stuck in the writeback somewhere and the
> > anon memory is not reclaimable because you do not have any swap device.
>
> Not having a swap device shouldn't be a problem -- and in this case, it
> would cause even more trouble as in disk i/o.
>
> What could cause the file pages to get stuck or stopped from being written
> to the disk? And more importantly, what is so unique/special about the
> Intel Rapid Storage that it happens (seemingly) exclusively with that
> and not the the normal Linux s/w raid support?
I am not a storage expert (not even mention dm-crypt). But what those
counters say is that the IO completion doesn't trigger so the
PageWriteback flag is still set. Such a page is not reclaimable
obviously. So I would check the IO delivery path and focus on the
potential dm-crypt involvement if you suspect this is a contributing
factor.
> Also, if the pages are not written to disk, shouldn't something error
> out or slow dd down?
Writers are normally throttled when we the dirty limit. You seem to have
dirty_ratio set to 20% which is quite a lot considering how much memory
you have. If you get back to the memory info from the OOM killer report:
[18907.592209] active_anon:110314 inactive_anon:295 isolated_anon:0
active_file:27534 inactive_file:819673 isolated_file:160
unevictable:13001 dirty:167859 writeback:651864 unstable:0
slab_reclaimable:177477 slab_unreclaimable:1817501
mapped:934 shmem:588 pagetables:7109 bounce:0
free:49928 free_pcp:45 free_cma:0
The dirty+writeback is ~9%. What is more interesting, though, LRU
pages are negligible to the memory size (~11%). Note the numer of
unreclaimable slab pages (~20%). Who is consuming those objects?
Where is the rest 70% of memory hiding?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Matthias Dahl @ 2016-07-12 11:28 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <20160712095013.GA14591@dhcp22.suse.cz>
Hello Michal...
On 2016-07-12 11:50, Michal Hocko wrote:
> This smells like file pages are stuck in the writeback somewhere and
> the
> anon memory is not reclaimable because you do not have any swap device.
Not having a swap device shouldn't be a problem -- and in this case, it
would cause even more trouble as in disk i/o.
What could cause the file pages to get stuck or stopped from being
written
to the disk? And more importantly, what is so unique/special about the
Intel Rapid Storage that it happens (seemingly) exclusively with that
and not the the normal Linux s/w raid support?
Also, if the pages are not written to disk, shouldn't something error
out or slow dd down? Obviously dd is capable of copying zeros a lot
faster than they could ever be written to disk -- and still, it works
just fine without dm-crypt in-between. It is only when dm-crypt /is/
involved, that the memory gets filled up and things get out of control.
Thanks,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
--
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>
^ permalink raw reply
* Re: [PATCH RFC] md/raid1: fix deadlock between freeze_array() and wait_barrier().
From: Alexander Lyakas @ 2016-07-12 10:09 UTC (permalink / raw)
To: NeilBrown; +Cc: 马建朋, linux-raid, Jes Sorensen
In-Reply-To: <87poqpf23c.fsf@notabene.neil.brown.name>
Hello Neil,
Thank you for your response. I read an email about you retiring from
MD/mdadm maintenance and delegating mdadm maintenance to Jes Sorensen.
But I was wondering who will be responsible for MD maintenance, and
was about to send an email asking that.
On Fri, Jul 8, 2016 at 2:41 AM, NeilBrown <neilb@suse.com> wrote:
> On Mon, Jun 27 2016, Alexander Lyakas wrote:
>
>> When we call wait_barrier, we might have some bios waiting
>> in current->bio_list, which prevents the array_freeze call to
>> complete. Those can only be internal READs, which have already
>> passed the wait_barrier call (thus incrementing nr_pending), but
>> still were not submitted to the lower level, due to generic_make_request
>> logic to avoid recursive calls. In such case, we have a deadlock:
>> - array_frozen is already set to 1, so wait_barrier unconditionally waits, so
>> - internal READ bios will not be submitted, thus freeze_array will
>> never completes
>>
>> This problem was originally fixed in commit:
>> d6b42dc md/raid1,raid10: avoid deadlock during resync/recovery.
>>
>> But then it was broken in commit:
>> b364e3d raid1: Add a field array_frozen to indicate whether raid in
>> freeze state.
>
> Thanks for the great analysis.
> I think this primarily a problem in generic_make_request(). It queues
> requests in the *wrong* order.
>
> Please try the patch from
> https://lkml.org/lkml/2016/7/7/428
>
> and see if it helps. If two requests for a raid1 are in the
> generic_make_request queue, this patch causes the sub-requests created
> by the first to be handled before the second is attempted.
I have read this discussion and more or less (probably less than more)
understood that the second patch by Lars is supposed to address our
issue. However, we cannot easily apply that patch:
- The patch is based on structures added by earlier patch "[RFC]
block: fix blk_queue_split() resource exhaustion".
- Both patches are not in the mainline tree yet.
- Both patches are in block core, which requires to recompile the whole kernel.
- Not sure if these patches are applicable for our production kernel
3.18 (long term)
I am sure you understand that for production with our current kernel
3.18 (long term) we cannot go with these two patches.
Since this issue is a real deadlock we are hitting in a long-term 3.18
kernel, is there any chance for cc-stable fix? Currently we applied
the rudimentary fix I posted. It basically switches context for
problematic RAID1 READs, and runs them from a different context. With
this fix we don't see the deadlock anymore.
Also, can you please comment on another concern I expressed:
freeze_array() is now not reentrant. Meaning that if two threads call
it in parallel (and it could happen for the same MD), the first thread
calling unfreeze_array will mess up things for the second thread.
Thank you for your help,
Alex.
>
> Thanks,
> NeilBrown
^ permalink raw reply
* Re: Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Michal Hocko @ 2016-07-12 9:50 UTC (permalink / raw)
To: Matthias Dahl; +Cc: linux-raid, linux-mm, dm-devel, linux-kernel
In-Reply-To: <02580b0a303da26b669b4a9892624b13@mail.ud19.udmedia.de>
On Tue 12-07-16 10:27:37, Matthias Dahl wrote:
> Hello,
>
> I posted this issue already on linux-mm, linux-kernel and dm-devel a
> few days ago and after further investigation it seems like that this
> issue is somehow related to the fact that I am using an Intel Rapid
> Storage RAID10, so I am summarizing everything again in this mail
> and include linux-raid in my post. Sorry for the noise... :(
>
> I am currently setting up a new machine (since my old one broke down)
> and I ran into a lot of " Unable to allocate memory on node -1" warnings
> while using dm-crypt. I have attached as much of the full log as I could
> recover.
>
> The encrypted device is sitting on a RAID10 (software raid, Intel Rapid
> Storage). I am currently limited to testing via Linux live images since
> the machine is not yet properly setup but I did my tests across several
> of those.
>
> Steps to reproduce are:
>
> 1)
> cryptsetup -s 512 -d /dev/urandom -c aes-xts-plain64 open --type plain
> /dev/md126p5 test-device
>
> 2)
> dd if=/dev/zero of=/dev/mapper/test-device status=progress bs=512K
>
> While running and monitoring the memory usage with free, it can be seen
> that the used memory increases rapidly and after just a few seconds, the
> system is out of memory and page allocation failures start to be issued
> as well as the OOM killer gets involved.
Here are two instances of the oom killer Mem-Info:
[18907.592206] Mem-Info:
[18907.592209] active_anon:110314 inactive_anon:295 isolated_anon:0
active_file:27534 inactive_file:819673 isolated_file:160
unevictable:13001 dirty:167859 writeback:651864 unstable:0
slab_reclaimable:177477 slab_unreclaimable:1817501
mapped:934 shmem:588 pagetables:7109 bounce:0
free:49928 free_pcp:45 free_cma:0
[18908.976349] Mem-Info:
[18908.976352] active_anon:109647 inactive_anon:295 isolated_anon:0
active_file:27535 inactive_file:819602 isolated_file:128
unevictable:13001 dirty:167672 writeback:652038 unstable:0
slab_reclaimable:177477 slab_unreclaimable:1817828
mapped:934 shmem:588 pagetables:7109 bounce:0
free:50252 free_pcp:91 free_cma:0
This smells like file pages are stuck in the writeback somewhere and the
anon memory is not reclaimable because you do not have any swap device.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Page Allocation Failures/OOM with dm-crypt on software RAID10 (Intel Rapid Storage)
From: Matthias Dahl @ 2016-07-12 8:27 UTC (permalink / raw)
To: linux-raid; +Cc: linux-mm, dm-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3146 bytes --]
Hello,
I posted this issue already on linux-mm, linux-kernel and dm-devel a
few days ago and after further investigation it seems like that this
issue is somehow related to the fact that I am using an Intel Rapid
Storage RAID10, so I am summarizing everything again in this mail
and include linux-raid in my post. Sorry for the noise... :(
I am currently setting up a new machine (since my old one broke down)
and I ran into a lot of " Unable to allocate memory on node -1" warnings
while using dm-crypt. I have attached as much of the full log as I could
recover.
The encrypted device is sitting on a RAID10 (software raid, Intel Rapid
Storage). I am currently limited to testing via Linux live images since
the machine is not yet properly setup but I did my tests across several
of those.
Steps to reproduce are:
1)
cryptsetup -s 512 -d /dev/urandom -c aes-xts-plain64 open --type plain
/dev/md126p5 test-device
2)
dd if=/dev/zero of=/dev/mapper/test-device status=progress bs=512K
While running and monitoring the memory usage with free, it can be seen
that the used memory increases rapidly and after just a few seconds, the
system is out of memory and page allocation failures start to be issued
as well as the OOM killer gets involved.
I have also seen this behavior with mkfs.ext4 being used on the same
device -- at least with 1.43.1.
Using direct i/o will work fine and not cause any issue. Also if
dm-crypt
is out of the picture, the problem does also not occur.
I did further tests:
1) dd block size has no influence on the issue whatsoever
2) using dm-crypt on an image located on an ext2 on the RAID10 works
fine
3) using an external (connected through USB3) hd with two partitions
and using either a RAID1 or RAID10 on it via Linux s/w RAID with
dm-crypt on-top, does also work fine
But as soon as I use dm-crypt on the Intel Rapid Storage RAID10, the
issue is 100% reproducible.
I tested all of this on a Fedora Rawhide Live Image as I currently still
am
in the process of setting the new machine up. Those images are available
here to download:
download.fedoraproject.org/pub/fedora/linux/development/rawhide/Workstation/x86_64/iso/
The machine itself has 32 GiB of RAM (plenty), no swap (live image)
and is a 6700k on a Z170 chipset. The kernel is the default provided
with the live image... right now that is a very recent git after
4.7.0rc6 but before rc7. But the issue also shows on 4.4.8 and 4.5.5.
The stripe size of the RAID10 is 64k, if that matters.
I am now pretty much out of ideas what else to test and where the
problem
could stem from. Suffice to say that this has impacted my trust in this
particular setup. I hope I can help to find the cause of this.
If there is anything I can do to help, please let me know.
Also, since I am not subscribed to the lists right now (I have to make
due
with a crappy WebMail interface until everything is setup), please cc'
me
accordingly. Thanks a lot.
With Kind Regards from Germany,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
[-- Attachment #2: mdstat.txt --]
[-- Type: text/plain, Size: 296 bytes --]
Personalities : [raid10]
md126 : active raid10 sda[3] sdb[2] sdc[1] sdd[0]
3907023872 blocks super external:/md127/0 64K chunks 2 near-copies [4/4] [UUUU]
md127 : inactive sdc[3](S) sdb[2](S) sda[1](S) sdd[0](S)
10064 blocks super external:imsm
unused devices: <none>
[-- Attachment #3: vmstat.txt --]
[-- Type: text/plain, Size: 2738 bytes --]
nr_free_pages 7943696
nr_alloc_batch 5873
nr_inactive_anon 296
nr_active_anon 105347
nr_inactive_file 29921
nr_active_file 56980
nr_unevictable 13140
nr_mlock 2716
nr_anon_pages 107204
nr_mapped 27670
nr_file_pages 98500
nr_dirty 14
nr_writeback 0
nr_slab_reclaimable 8887
nr_slab_unreclaimable 16975
nr_page_table_pages 7137
nr_kernel_stack 490
nr_unstable 0
nr_bounce 0
nr_vmscan_write 9828326
nr_vmscan_immediate_reclaim 67360474
nr_writeback_temp 0
nr_isolated_anon 0
nr_isolated_file 0
nr_shmem 593
nr_dirtied 506466654
nr_written 506466595
nr_pages_scanned 0
numa_hit 5258670960
numa_miss 0
numa_foreign 0
numa_interleave 38217
numa_local 5258670960
numa_other 0
workingset_refault 336993
workingset_activate 61553
workingset_nodereclaim 7919435
nr_anon_transparent_hugepages 0
nr_free_cma 0
nr_dirty_threshold 1592267
nr_dirty_background_threshold 795161
pgpgin 10489537
pgpgout 2025884337
pswpin 0
pswpout 0
pgalloc_dma 684558
pgalloc_dma32 328009673
pgalloc_normal 5767713958
pgalloc_movable 0
pgfree 6106436104
pgactivate 813221
pgdeactivate 1284082
pgfault 1653795
pgmajfault 46351
pglazyfreed 0
pgrefill_dma 0
pgrefill_dma32 66114
pgrefill_normal 1407169
pgrefill_movable 0
pgsteal_kswapd_dma 0
pgsteal_kswapd_dma32 22181873
pgsteal_kswapd_normal 425875886
pgsteal_kswapd_movable 0
pgsteal_direct_dma 0
pgsteal_direct_dma32 10723905
pgsteal_direct_normal 45330060
pgsteal_direct_movable 0
pgscan_kswapd_dma 0
pgscan_kswapd_dma32 32470709
pgscan_kswapd_normal 758168190
pgscan_kswapd_movable 0
pgscan_direct_dma 0
pgscan_direct_dma32 55064390
pgscan_direct_normal 449388285
pgscan_direct_movable 0
pgscan_direct_throttle 16
zone_reclaim_failed 0
pginodesteal 329
slabs_scanned 75784518
kswapd_inodesteal 3324
kswapd_low_wmark_hit_quickly 18086579
kswapd_high_wmark_hit_quickly 562
pageoutrun 18100603
allocstall 739928
pgrotated 357590082
drop_pagecache 0
drop_slab 0
numa_pte_updates 0
numa_huge_pte_updates 0
numa_hint_faults 0
numa_hint_faults_local 0
numa_pages_migrated 0
pgmigrate_success 562476
pgmigrate_fail 34076511
compact_migrate_scanned 390290706
compact_free_scanned 17609026156
compact_isolated 37387419
compact_stall 17
compact_fail 10
compact_success 7
compact_daemon_wake 3013752
htlb_buddy_alloc_success 0
htlb_buddy_alloc_fail 0
unevictable_pgs_culled 69728
unevictable_pgs_scanned 0
unevictable_pgs_rescued 57566
unevictable_pgs_mlocked 62928
unevictable_pgs_munlocked 59182
unevictable_pgs_cleared 18
unevictable_pgs_stranded 18
thp_fault_alloc 0
thp_fault_fallback 0
thp_collapse_alloc 0
thp_collapse_alloc_failed 0
thp_split_page 0
thp_split_page_failed 0
thp_deferred_split_page 0
thp_split_pmd 0
thp_zero_page_alloc 0
thp_zero_page_alloc_failed 0
balloon_inflate 0
balloon_deflate 0
balloon_migrate 0
[-- Attachment #4: crypto.txt.gz --]
[-- Type: application/x-gzip, Size: 1197 bytes --]
[-- Attachment #5: kernel.log.txt.gz --]
[-- Type: application/x-gzip, Size: 24060 bytes --]
[-- Attachment #6: sysctl.txt.gz --]
[-- Type: application/x-gzip, Size: 7591 bytes --]
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 1/1] block: fix blk_queue_split() resource exhaustion
From: NeilBrown @ 2016-07-12 2:55 UTC (permalink / raw)
To: Lars Ellenberg, Jens Axboe
Cc: linux-block, Martin K. Petersen, Mike Snitzer, Peter Zijlstra,
Jiri Kosina, Ming Lei, Kirill A. Shutemov, linux-kernel,
linux-raid, Takashi Iwai, linux-bcache, Zheng Liu,
Kent Overstreet, Keith Busch, dm-devel, Shaohua Li, Ingo Molnar,
Alasdair Kergon, Roland Kammerer
In-Reply-To: <20160711141042.GY13335@soda.linbit>
[-- Attachment #1: Type: text/plain, Size: 962 bytes --]
On Tue, Jul 12 2016, Lars Ellenberg wrote:
....
>
> Instead, I suggest to distinguish between recursive calls to
> generic_make_request(), and pushing back the remainder part in
> blk_queue_split(), by pointing current->bio_lists to a
> struct recursion_to_iteration_bio_lists {
> struct bio_list recursion;
> struct bio_list queue;
> }
>
> By providing each q->make_request_fn() with an empty "recursion"
> bio_list, then merging any recursively submitted bios to the
> head of the "queue" list, we can make the recursion-to-iteration
> logic in generic_make_request() process deepest level bios first,
> and "sibling" bios of the same level in "natural" order.
>
> Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
> Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
Reviewed-by: NeilBrown <neilb@suse.com>
Thanks again for doing this - I think this is a very significant
improvement and could allow other simplifications.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-11 14:13 UTC (permalink / raw)
To: Jens Axboe
Cc: Mike Snitzer, linux-block, Keith Busch, Martin K. Petersen,
Peter Zijlstra, Jiri Kosina, Ming Lei, Kirill A. Shutemov,
NeilBrown, linux-kernel, linux-raid, Takashi Iwai, linux-bcache,
Zheng Liu, Kent Overstreet, dm-devel, Shaohua Li, Ingo Molnar,
Alasdair Kergon, Roland Kammerer
In-Reply-To: <20160708184957.GA10765@redhat.com>
Dropped the XXX comment (oops),
moved the current_has_pending_bios() helper to bio.h
and dropped the identical ones from bio.c and md.h.
Reposted in-thread as
[PATCH v2 1/1] block: fix blk_queue_split() resource exhaustion
Thanks,
Lars Ellenberg
^ permalink raw reply
* [PATCH v2 1/1] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-11 14:10 UTC (permalink / raw)
To: Jens Axboe
Cc: NeilBrown, linux-raid, Martin K. Petersen, Mike Snitzer,
Peter Zijlstra, Jiri Kosina, Ming Lei, linux-kernel, Zheng Liu,
linux-block, Takashi Iwai, linux-bcache, Ingo Molnar,
Alasdair Kergon, Keith Busch, dm-devel, Shaohua Li,
Kent Overstreet, Kirill A. Shutemov, Roland Kammerer
In-Reply-To: <1467990243-3531-2-git-send-email-lars.ellenberg@linbit.com>
For a long time, generic_make_request() converts recursion into
iteration by queuing recursive arguments on current->bio_list.
This is convenient for stacking drivers,
the top-most driver would take the originally submitted bio,
and re-submit a re-mapped version of it, or one or more clones,
or one or more new allocated bios to its backend(s). Which
are then simply processed in turn, and each can again queue
more "backend-bios" until we reach the bottom of the driver stack,
and actually dispatch to the real backend device.
Any stacking driver ->make_request_fn() could expect that,
once it returns, any backend-bios it submitted via recursive calls
to generic_make_request() would now be processed and dispatched, before
the current task would call into this driver again.
This is changed by commit
54efd50 block: make generic_make_request handle arbitrarily sized bios
Drivers may call blk_queue_split() inside their ->make_request_fn(),
which may split the current bio into a front-part to be dealt with
immediately, and a remainder-part, which may need to be split even
further. That remainder-part will simply also be pushed to
current->bio_list, and would end up being head-of-queue, in front
of any backend-bios the current make_request_fn() might submit during
processing of the fron-part.
Which means the current task would immediately end up back in the same
make_request_fn() of the same driver again, before any of its backend
bios have even been processed.
This can lead to resource starvation deadlock.
Drivers could avoid this by learning to not need blk_queue_split(),
or by submitting their backend bios in a different context (dedicated
kernel thread, work_queue context, ...). Or by playing funny re-ordering
games with entries on current->bio_list.
Instead, I suggest to distinguish between recursive calls to
generic_make_request(), and pushing back the remainder part in
blk_queue_split(), by pointing current->bio_lists to a
struct recursion_to_iteration_bio_lists {
struct bio_list recursion;
struct bio_list queue;
}
By providing each q->make_request_fn() with an empty "recursion"
bio_list, then merging any recursively submitted bios to the
head of the "queue" list, we can make the recursion-to-iteration
logic in generic_make_request() process deepest level bios first,
and "sibling" bios of the same level in "natural" order.
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
---
block/bio.c | 20 +++++++++++--------
block/blk-core.c | 49 +++++++++++++++++++++++++----------------------
block/blk-merge.c | 5 ++++-
drivers/md/bcache/btree.c | 12 ++++++------
drivers/md/dm-bufio.c | 2 +-
drivers/md/raid1.c | 5 ++---
drivers/md/raid10.c | 5 ++---
include/linux/bio.h | 25 ++++++++++++++++++++++++
include/linux/sched.h | 4 ++--
9 files changed, 80 insertions(+), 47 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 848cd35..c2606fd 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -366,12 +366,16 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
*/
bio_list_init(&punt);
- bio_list_init(&nopunt);
- while ((bio = bio_list_pop(current->bio_list)))
+ bio_list_init(&nopunt);
+ while ((bio = bio_list_pop(¤t->bio_lists->recursion)))
bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
+ current->bio_lists->recursion = nopunt;
- *current->bio_list = nopunt;
+ bio_list_init(&nopunt);
+ while ((bio = bio_list_pop(¤t->bio_lists->queue)))
+ bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
+ current->bio_lists->queue = nopunt;
spin_lock(&bs->rescue_lock);
bio_list_merge(&bs->rescue_list, &punt);
@@ -453,13 +457,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
*
* We solve this, and guarantee forward progress, with a rescuer
* workqueue per bio_set. If we go to allocate and there are
- * bios on current->bio_list, we first try the allocation
- * without __GFP_DIRECT_RECLAIM; if that fails, we punt those
- * bios we would be blocking to the rescuer workqueue before
- * we retry with the original gfp_flags.
+ * bios on current->bio_lists->{recursion,queue}, we first try the
+ * allocation without __GFP_DIRECT_RECLAIM; if that fails, we
+ * punt those bios we would be blocking to the rescuer
+ * workqueue before we retry with the original gfp_flags.
*/
- if (current->bio_list && !bio_list_empty(current->bio_list))
+ if (current_has_pending_bios())
gfp_mask &= ~__GFP_DIRECT_RECLAIM;
p = mempool_alloc(bs->bio_pool, gfp_mask);
diff --git a/block/blk-core.c b/block/blk-core.c
index 3cfd67d..2886a59b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2040,7 +2040,7 @@ end_io:
*/
blk_qc_t generic_make_request(struct bio *bio)
{
- struct bio_list bio_list_on_stack;
+ struct recursion_to_iteration_bio_lists bio_lists_on_stack;
blk_qc_t ret = BLK_QC_T_NONE;
if (!generic_make_request_checks(bio))
@@ -2049,15 +2049,20 @@ blk_qc_t generic_make_request(struct bio *bio)
/*
* We only want one ->make_request_fn to be active at a time, else
* stack usage with stacked devices could be a problem. So use
- * current->bio_list to keep a list of requests submited by a
- * make_request_fn function. current->bio_list is also used as a
+ * current->bio_lists to keep a list of requests submited by a
+ * make_request_fn function. current->bio_lists is also used as a
* flag to say if generic_make_request is currently active in this
* task or not. If it is NULL, then no make_request is active. If
* it is non-NULL, then a make_request is active, and new requests
- * should be added at the tail
+ * should be added at the tail of current->bio_lists->recursion;
+ * bios resulting from a call to blk_queue_split() from
+ * within ->make_request_fn() should be pushed back to the head of
+ * current->bio_lists->queue.
+ * After the current ->make_request_fn() returns, .recursion will be
+ * merged back to the head of .queue.
*/
- if (current->bio_list) {
- bio_list_add(current->bio_list, bio);
+ if (current->bio_lists) {
+ bio_list_add(¤t->bio_lists->recursion, bio);
goto out;
}
@@ -2066,35 +2071,33 @@ blk_qc_t generic_make_request(struct bio *bio)
* Before entering the loop, bio->bi_next is NULL (as all callers
* ensure that) so we have a list with a single bio.
* We pretend that we have just taken it off a longer list, so
- * we assign bio_list to a pointer to the bio_list_on_stack,
- * thus initialising the bio_list of new bios to be
- * added. ->make_request() may indeed add some more bios
- * through a recursive call to generic_make_request. If it
- * did, we find a non-NULL value in bio_list and re-enter the loop
- * from the top. In this case we really did just take the bio
- * of the top of the list (no pretending) and so remove it from
- * bio_list, and call into ->make_request() again.
+ * we assign bio_list to a pointer to the bio_lists_on_stack,
+ * thus initialising the bio_lists of new bios to be added.
+ * ->make_request() may indeed add some more bios to .recursion
+ * through a recursive call to generic_make_request. If it did,
+ * we find a non-NULL value in .recursion, merge .recursion back to the
+ * head of .queue, and re-enter the loop from the top. In this case we
+ * really did just take the bio of the top of the list (no pretending)
+ * and so remove it from .queue, and call into ->make_request() again.
*/
BUG_ON(bio->bi_next);
- bio_list_init(&bio_list_on_stack);
- current->bio_list = &bio_list_on_stack;
+ bio_list_init(&bio_lists_on_stack.queue);
+ current->bio_lists = &bio_lists_on_stack;
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
if (likely(blk_queue_enter(q, false) == 0)) {
+ bio_list_init(&bio_lists_on_stack.recursion);
ret = q->make_request_fn(q, bio);
-
blk_queue_exit(q);
-
- bio = bio_list_pop(current->bio_list);
+ bio_list_merge_head(&bio_lists_on_stack.queue,
+ &bio_lists_on_stack.recursion);
} else {
- struct bio *bio_next = bio_list_pop(current->bio_list);
-
bio_io_error(bio);
- bio = bio_next;
}
+ bio = bio_list_pop(¤t->bio_lists->queue);
} while (bio);
- current->bio_list = NULL; /* deactivate */
+ current->bio_lists = NULL; /* deactivate */
out:
return ret;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index c265348..df96327 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -172,6 +172,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
struct bio *split, *res;
unsigned nsegs;
+ BUG_ON(!current->bio_lists);
if (bio_op(*bio) == REQ_OP_DISCARD)
split = blk_bio_discard_split(q, *bio, bs, &nsegs);
else if (bio_op(*bio) == REQ_OP_WRITE_SAME)
@@ -190,7 +191,9 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
bio_chain(split, *bio);
trace_block_split(q, split, (*bio)->bi_iter.bi_sector);
- generic_make_request(*bio);
+ /* push back remainder, it may later be split further */
+ bio_list_add_head(¤t->bio_lists->queue, *bio);
+ /* and fake submission of a suitably sized piece */
*bio = split;
}
}
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 76f7534..731ec3b 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -450,7 +450,7 @@ void __bch_btree_node_write(struct btree *b, struct closure *parent)
trace_bcache_btree_write(b);
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
BUG_ON(b->written >= btree_blocks(b));
BUG_ON(b->written && !i->keys);
BUG_ON(btree_bset_first(b)->seq != i->seq);
@@ -544,7 +544,7 @@ static void bch_btree_leaf_dirty(struct btree *b, atomic_t *journal_ref)
/* Force write if set is too big */
if (set_bytes(i) > PAGE_SIZE - 48 &&
- !current->bio_list)
+ !current->bio_lists)
bch_btree_node_write(b, NULL);
}
@@ -889,7 +889,7 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
{
struct btree *b;
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
lockdep_assert_held(&c->bucket_lock);
@@ -976,7 +976,7 @@ retry:
b = mca_find(c, k);
if (!b) {
- if (current->bio_list)
+ if (current->bio_lists)
return ERR_PTR(-EAGAIN);
mutex_lock(&c->bucket_lock);
@@ -2127,7 +2127,7 @@ static int bch_btree_insert_node(struct btree *b, struct btree_op *op,
return 0;
split:
- if (current->bio_list) {
+ if (current->bio_lists) {
op->lock = b->c->root->level + 1;
return -EAGAIN;
} else if (op->lock <= b->c->root->level) {
@@ -2209,7 +2209,7 @@ int bch_btree_insert(struct cache_set *c, struct keylist *keys,
struct btree_insert_op op;
int ret = 0;
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
BUG_ON(bch_keylist_empty(keys));
bch_btree_op_init(&op.op, 0);
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 6571c81..ba0c325 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -174,7 +174,7 @@ static inline int dm_bufio_cache_index(struct dm_bufio_client *c)
#define DM_BUFIO_CACHE(c) (dm_bufio_caches[dm_bufio_cache_index(c)])
#define DM_BUFIO_CACHE_NAME(c) (dm_bufio_cache_names[dm_bufio_cache_index(c)])
-#define dm_bufio_in_request() (!!current->bio_list)
+#define dm_bufio_in_request() (!!current->bio_lists)
static void dm_bufio_lock(struct dm_bufio_client *c)
{
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 10e53cd..38790e3 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -876,8 +876,7 @@ static sector_t wait_barrier(struct r1conf *conf, struct bio *bio)
(!conf->barrier ||
((conf->start_next_window <
conf->next_resync + RESYNC_SECTORS) &&
- current->bio_list &&
- !bio_list_empty(current->bio_list))),
+ current_has_pending_bios())),
conf->resync_lock);
conf->nr_waiting--;
}
@@ -1014,7 +1013,7 @@ static void raid1_unplug(struct blk_plug_cb *cb, bool from_schedule)
struct r1conf *conf = mddev->private;
struct bio *bio;
- if (from_schedule || current->bio_list) {
+ if (from_schedule || current->bio_lists) {
spin_lock_irq(&conf->device_lock);
bio_list_merge(&conf->pending_bio_list, &plug->pending);
conf->pending_count += plug->pending_cnt;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 245640b..13a5341 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -945,8 +945,7 @@ static void wait_barrier(struct r10conf *conf)
wait_event_lock_irq(conf->wait_barrier,
!conf->barrier ||
(conf->nr_pending &&
- current->bio_list &&
- !bio_list_empty(current->bio_list)),
+ current_has_pending_bios()),
conf->resync_lock);
conf->nr_waiting--;
}
@@ -1022,7 +1021,7 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
struct r10conf *conf = mddev->private;
struct bio *bio;
- if (from_schedule || current->bio_list) {
+ if (from_schedule || current->bio_lists) {
spin_lock_irq(&conf->device_lock);
bio_list_merge(&conf->pending_bio_list, &plug->pending);
conf->pending_count += plug->pending_cnt;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b7e1a008..2f8a361 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -541,6 +541,24 @@ struct bio_list {
struct bio *tail;
};
+/* for generic_make_request() */
+struct recursion_to_iteration_bio_lists {
+ /* For stacking drivers submitting to their respective backend,
+ * bios are added to the tail of .recursion, which is re-initialized
+ * before each call to ->make_request_fn() and after that returns,
+ * the whole .recursion queue is then merged back to the head of .queue.
+ *
+ * The recursion-to-iteration logic in generic_make_request() will
+ * peel off of .queue.head, processing bios in deepest-level-first
+ * "natural" order. */
+ struct bio_list recursion;
+
+ /* This keeps a list of to-be-processed bios.
+ * The "remainder" part resulting from calling blk_queue_split()
+ * will be pushed back to its head. */
+ struct bio_list queue;
+};
+
static inline int bio_list_empty(const struct bio_list *bl)
{
return bl->head == NULL;
@@ -551,6 +569,13 @@ static inline void bio_list_init(struct bio_list *bl)
bl->head = bl->tail = NULL;
}
+static inline bool current_has_pending_bios(void)
+{
+ return current->bio_lists &&
+ (!bio_list_empty(¤t->bio_lists->queue) ||
+ !bio_list_empty(¤t->bio_lists->recursion));
+}
+
#define BIO_EMPTY_LIST { NULL, NULL }
#define bio_list_for_each(bio, bl) \
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada..146eedc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -128,7 +128,7 @@ struct sched_attr {
struct futex_pi_state;
struct robust_list_head;
-struct bio_list;
+struct recursion_to_iteration_bio_lists;
struct fs_struct;
struct perf_event_context;
struct blk_plug;
@@ -1727,7 +1727,7 @@ struct task_struct {
void *journal_info;
/* stacked block device info */
- struct bio_list *bio_list;
+ struct recursion_to_iteration_bio_lists *bio_lists;
#ifdef CONFIG_BLOCK
/* stack plugging */
--
1.9.1
^ permalink raw reply related
* Recover filenames from failed RAID0
From: Michel Dubois @ 2016-07-10 23:03 UTC (permalink / raw)
To: linux-raid
Cc: Jens Axboe, Keith Busch, dm-devel, Martin K. Petersen,
Ingo Molnar, Peter Zijlstra, Jiri Kosina, Ming Lei, NeilBrown,
linux-kernel, linux-block, Takashi Iwai, linux-bcache, Zheng Liu,
Mike Snitzer, Alasdair Kergon, Lars Ellenberg, Shaohua Li,
Kent Overstreet, Kirill A. Shutemov, Roland Kammerer
[-- Attachment #1.1: Type: text/plain, Size: 4796 bytes --]
Dear linux-raid mailing list,
I have a RAID0 array of four 3TB disks that failed on the "third" disk.
I am aware of the non-redundancy of RAID0 but I would like to recover the
filenames from that RAID0. If I could recover some data it would be a bonus.
Below you'll find the outputs of the following commands
mdadm --examine /dev/sd[abcd]1
fdisk -l
where sda1, sdb1, sdc1 and sdd1 should be the 4 RAID devices.
What could be my next step?
I thank you for your time
Michel Dubois
======================
mdadm --examine /dev/sd[abcd]1
/dev/sda1:
Magic : a92b4efc
Version : 00.90.00
UUID : 7d247a6e:7b5d46c8:f52d9c89:db304b21
Creation Time : Mon Apr 23 19:55:36 2012
Raid Level : raid1
Used Dev Size : 20980800 (20.01 GiB 21.48 GB)
Array Size : 20980800 (20.01 GiB 21.48 GB)
Raid Devices : 4
Total Devices : 3
Preferred Minor : 0
Update Time : Mon Jun 27 21:12:23 2016
State : clean
Active Devices : 3
Working Devices : 3
Failed Devices : 1
Spare Devices : 0
Checksum : 1a57db60 - correct
Events : 164275
Number Major Minor RaidDevice State
this 0 8 1 0 active sync /dev/sda1
0 0 8 1 0 active sync /dev/sda1
1 1 8 17 1 active sync /dev/sdb1
2 2 0 0 2 faulty removed
3 3 8 33 3 active sync /dev/sdc1
/dev/sdb1:
Magic : a92b4efc
Version : 00.90.00
UUID : 7d247a6e:7b5d46c8:f52d9c89:db304b21
Creation Time : Mon Apr 23 19:55:36 2012
Raid Level : raid1
Used Dev Size : 20980800 (20.01 GiB 21.48 GB)
Array Size : 20980800 (20.01 GiB 21.48 GB)
Raid Devices : 4
Total Devices : 3
Preferred Minor : 0
Update Time : Mon Jun 27 21:12:23 2016
State : clean
Active Devices : 3
Working Devices : 3
Failed Devices : 1
Spare Devices : 0
Checksum : 1a57db72 - correct
Events : 164275
Number Major Minor RaidDevice State
this 1 8 17 1 active sync /dev/sdb1
0 0 8 1 0 active sync /dev/sda1
1 1 8 17 1 active sync /dev/sdb1
2 2 0 0 2 faulty removed
3 3 8 33 3 active sync /dev/sdc1
/dev/sdc1:
Magic : a92b4efc
Version : 00.90.00
UUID : 7d247a6e:7b5d46c8:f52d9c89:db304b21
Creation Time : Mon Apr 23 19:55:36 2012
Raid Level : raid1
Used Dev Size : 20980800 (20.01 GiB 21.48 GB)
Array Size : 20980800 (20.01 GiB 21.48 GB)
Raid Devices : 4
Total Devices : 3
Preferred Minor : 0
Update Time : Mon Jun 27 21:12:23 2016
State : clean
Active Devices : 3
Working Devices : 3
Failed Devices : 1
Spare Devices : 0
Checksum : 1a57db86 - correct
Events : 164275
Number Major Minor RaidDevice State
this 3 8 33 3 active sync /dev/sdc1
0 0 8 1 0 active sync /dev/sda1
1 1 8 17 1 active sync /dev/sdb1
2 2 0 0 2 faulty removed
3 3 8 33 3 active sync /dev/sdc1
======================
fdisk -l
WARNING: GPT (GUID Partition Table) detected on '/dev/sda'! The util fdisk
doesn't support GPT. Use GNU Parted.
Disk /dev/sda: 3000.5 GB, 3000592982016 bytes
255 heads, 63 sectors/track, 364801 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes
Disk identifier: 0x03afffbe
Device Boot Start End Blocks Id System
/dev/sda1 1 267350 2147483647+ ee EFI GPT
WARNING: GPT (GUID Partition Table) detected on '/dev/sdb'! The util fdisk
doesn't support GPT. Use GNU Parted.
Disk /dev/sdb: 3000.5 GB, 3000592982016 bytes
255 heads, 63 sectors/track, 364801 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes
Disk identifier: 0x142a889c
Device Boot Start End Blocks Id System
/dev/sdb1 1 267350 2147483647+ ee EFI GPT
WARNING: GPT (GUID Partition Table) detected on '/dev/sdc'! The util fdisk
doesn't support GPT. Use GNU Parted.
Disk /dev/sdc: 3000.5 GB, 3000592982016 bytes
255 heads, 63 sectors/track, 364801 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes
Disk identifier: 0x3daebd50
Device Boot Start End Blocks Id System
/dev/sdc1 1 267350 2147483647+ ee EFI GPT
Disk /dev/md0: 21.4 GB, 21484339200 bytes
2 heads, 4 sectors/track, 5245200 cylinders
Units = cylinders of 8 * 512 = 4096 bytes
Disk identifier: 0x00000000
Disk /dev/md0 doesn't contain a valid partition table
[-- Attachment #1.2: Type: text/html, Size: 13702 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] block: fix blk_queue_split() resource exhaustion
From: Mike Snitzer @ 2016-07-08 18:49 UTC (permalink / raw)
To: Lars Ellenberg
Cc: Jens Axboe, linux-block, Keith Busch, Martin K. Petersen,
Peter Zijlstra, Jiri Kosina, Ming Lei, Kirill A. Shutemov,
NeilBrown, linux-kernel, linux-raid, Takashi Iwai, linux-bcache,
Zheng Liu, Kent Overstreet, dm-devel, Shaohua Li, Ingo Molnar,
Alasdair Kergon, Roland Kammerer
In-Reply-To: <1467990243-3531-2-git-send-email-lars.ellenberg@linbit.com>
On Fri, Jul 08 2016 at 11:04am -0400,
Lars Ellenberg <lars.ellenberg@linbit.com> wrote:
> For a long time, generic_make_request() converts recursion into
> iteration by queuing recursive arguments on current->bio_list.
>
> This is convenient for stacking drivers,
> the top-most driver would take the originally submitted bio,
> and re-submit a re-mapped version of it, or one or more clones,
> or one or more new allocated bios to its backend(s). Which
> are then simply processed in turn, and each can again queue
> more "backend-bios" until we reach the bottom of the driver stack,
> and actually dispatch to the real backend device.
>
> Any stacking driver ->make_request_fn() could expect that,
> once it returns, any backend-bios it submitted via recursive calls
> to generic_make_request() would now be processed and dispatched, before
> the current task would call into this driver again.
>
> This is changed by commit
> 54efd50 block: make generic_make_request handle arbitrarily sized bios
>
> Drivers may call blk_queue_split() inside their ->make_request_fn(),
> which may split the current bio into a front-part to be dealt with
> immediately, and a remainder-part, which may need to be split even
> further. That remainder-part will simply also be pushed to
> current->bio_list, and would end up being head-of-queue, in front
> of any backend-bios the current make_request_fn() might submit during
> processing of the fron-part.
>
> Which means the current task would immediately end up back in the same
> make_request_fn() of the same driver again, before any of its backend
> bios have even been processed.
>
> This can lead to resource starvation deadlock.
> Drivers could avoid this by learning to not need blk_queue_split(),
> or by submitting their backend bios in a different context (dedicated
> kernel thread, work_queue context, ...). Or by playing funny re-ordering
> games with entries on current->bio_list.
>
> Instead, I suggest to distinguish between recursive calls to
> generic_make_request(), and pushing back the remainder part in
> blk_queue_split(), by pointing current->bio_lists to a
> struct recursion_to_iteration_bio_lists {
> struct bio_list recursion;
> struct bio_list queue;
> }
>
> By providing each q->make_request_fn() with an empty "recursion"
> bio_list, then merging any recursively submitted bios to the
> head of the "queue" list, we can make the recursion-to-iteration
> logic in generic_make_request() process deepest level bios first,
> and "sibling" bios of the same level in "natural" order.
>
> Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
> Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
> ---
> block/bio.c | 27 +++++++++++++++++--------
> block/blk-core.c | 50 +++++++++++++++++++++++++----------------------
> block/blk-merge.c | 5 ++++-
> drivers/md/bcache/btree.c | 12 ++++++------
> drivers/md/dm-bufio.c | 2 +-
> drivers/md/md.h | 7 +++++++
> drivers/md/raid1.c | 5 ++---
> drivers/md/raid10.c | 5 ++---
> include/linux/bio.h | 18 +++++++++++++++++
> include/linux/sched.h | 4 ++--
> 10 files changed, 88 insertions(+), 47 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 848cd35..1f9fcf0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -366,12 +366,16 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
> */
>
> bio_list_init(&punt);
> - bio_list_init(&nopunt);
>
> - while ((bio = bio_list_pop(current->bio_list)))
> + bio_list_init(&nopunt);
> + while ((bio = bio_list_pop(¤t->bio_lists->recursion)))
> bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
> + current->bio_lists->recursion = nopunt;
>
> - *current->bio_list = nopunt;
> + bio_list_init(&nopunt);
> + while ((bio = bio_list_pop(¤t->bio_lists->queue)))
> + bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
> + current->bio_lists->queue = nopunt;
>
> spin_lock(&bs->rescue_lock);
> bio_list_merge(&bs->rescue_list, &punt);
> @@ -380,6 +384,13 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
> queue_work(bs->rescue_workqueue, &bs->rescue_work);
> }
>
> +static bool current_has_pending_bios(void)
> +{
> + return current->bio_lists &&
> + (!bio_list_empty(¤t->bio_lists->queue) ||
> + !bio_list_empty(¤t->bio_lists->recursion));
> +}
> +
This should be moved to include/linux/bio.h
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 3cfd67d..74bceea 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2066,35 +2071,34 @@ blk_qc_t generic_make_request(struct bio *bio)
> * Before entering the loop, bio->bi_next is NULL (as all callers
> * ensure that) so we have a list with a single bio.
> * We pretend that we have just taken it off a longer list, so
> - * we assign bio_list to a pointer to the bio_list_on_stack,
> - * thus initialising the bio_list of new bios to be
> - * added. ->make_request() may indeed add some more bios
> - * through a recursive call to generic_make_request. If it
> - * did, we find a non-NULL value in bio_list and re-enter the loop
> - * from the top. In this case we really did just take the bio
> - * of the top of the list (no pretending) and so remove it from
> - * bio_list, and call into ->make_request() again.
> + * we assign bio_list to a pointer to the bio_lists_on_stack,
> + * thus initialising the bio_lists of new bios to be added.
> + * ->make_request() may indeed add some more bios to .recursion
> + * through a recursive call to generic_make_request. If it did,
> + * we find a non-NULL value in .recursion, merge .recursion back to the
> + * head of .queue, and re-enter the loop from the top. In this case we
> + * really did just take the bio of the top of the list (no pretending)
> + * and so remove it from .queue, and call into ->make_request() again.
> */
> BUG_ON(bio->bi_next);
> - bio_list_init(&bio_list_on_stack);
> - current->bio_list = &bio_list_on_stack;
> + bio_list_init(&bio_lists_on_stack.queue);
> + current->bio_lists = &bio_lists_on_stack;
> do {
> struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>
> if (likely(blk_queue_enter(q, false) == 0)) {
> + bio_list_init(&bio_lists_on_stack.recursion);
> ret = q->make_request_fn(q, bio);
> -
> blk_queue_exit(q);
> -
> - bio = bio_list_pop(current->bio_list);
> + bio_list_merge_head(&bio_lists_on_stack.queue,
> + &bio_lists_on_stack.recursion);
> + /* XXX bio_list_init(&bio_lists_on_stack.recursion); */
Please remove this XXX commented code.
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b4f3352..b62e65f4 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -664,6 +664,13 @@ static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
> }
> }
>
> +static inline bool current_has_pending_bios(void)
> +{
> + return current->bio_lists && (
> + !bio_list_empty(¤t->bio_lists->queue) ||
> + !bio_list_empty(¤t->bio_lists->recursion));
> +}
> +
This can be removed now that include/linux/bio.h exports the same.
^ permalink raw reply
* [PATCH 1/1] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-08 15:04 UTC (permalink / raw)
To: Jens Axboe
Cc: Lars Ellenberg, NeilBrown, linux-raid, Martin K. Petersen,
Mike Snitzer, Peter Zijlstra, Jiri Kosina, Ming Lei, linux-kernel,
Zheng Liu, linux-block, Takashi Iwai, linux-bcache, Ingo Molnar,
Alasdair Kergon, Keith Busch, dm-devel, Shaohua Li,
Kent Overstreet, Kirill A. Shutemov, Roland Kammerer
In-Reply-To: <1467990243-3531-1-git-send-email-lars.ellenberg@linbit.com>
For a long time, generic_make_request() converts recursion into
iteration by queuing recursive arguments on current->bio_list.
This is convenient for stacking drivers,
the top-most driver would take the originally submitted bio,
and re-submit a re-mapped version of it, or one or more clones,
or one or more new allocated bios to its backend(s). Which
are then simply processed in turn, and each can again queue
more "backend-bios" until we reach the bottom of the driver stack,
and actually dispatch to the real backend device.
Any stacking driver ->make_request_fn() could expect that,
once it returns, any backend-bios it submitted via recursive calls
to generic_make_request() would now be processed and dispatched, before
the current task would call into this driver again.
This is changed by commit
54efd50 block: make generic_make_request handle arbitrarily sized bios
Drivers may call blk_queue_split() inside their ->make_request_fn(),
which may split the current bio into a front-part to be dealt with
immediately, and a remainder-part, which may need to be split even
further. That remainder-part will simply also be pushed to
current->bio_list, and would end up being head-of-queue, in front
of any backend-bios the current make_request_fn() might submit during
processing of the fron-part.
Which means the current task would immediately end up back in the same
make_request_fn() of the same driver again, before any of its backend
bios have even been processed.
This can lead to resource starvation deadlock.
Drivers could avoid this by learning to not need blk_queue_split(),
or by submitting their backend bios in a different context (dedicated
kernel thread, work_queue context, ...). Or by playing funny re-ordering
games with entries on current->bio_list.
Instead, I suggest to distinguish between recursive calls to
generic_make_request(), and pushing back the remainder part in
blk_queue_split(), by pointing current->bio_lists to a
struct recursion_to_iteration_bio_lists {
struct bio_list recursion;
struct bio_list queue;
}
By providing each q->make_request_fn() with an empty "recursion"
bio_list, then merging any recursively submitted bios to the
head of the "queue" list, we can make the recursion-to-iteration
logic in generic_make_request() process deepest level bios first,
and "sibling" bios of the same level in "natural" order.
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
---
block/bio.c | 27 +++++++++++++++++--------
block/blk-core.c | 50 +++++++++++++++++++++++++----------------------
block/blk-merge.c | 5 ++++-
drivers/md/bcache/btree.c | 12 ++++++------
drivers/md/dm-bufio.c | 2 +-
drivers/md/md.h | 7 +++++++
drivers/md/raid1.c | 5 ++---
drivers/md/raid10.c | 5 ++---
include/linux/bio.h | 18 +++++++++++++++++
include/linux/sched.h | 4 ++--
10 files changed, 88 insertions(+), 47 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 848cd35..1f9fcf0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -366,12 +366,16 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
*/
bio_list_init(&punt);
- bio_list_init(&nopunt);
- while ((bio = bio_list_pop(current->bio_list)))
+ bio_list_init(&nopunt);
+ while ((bio = bio_list_pop(¤t->bio_lists->recursion)))
bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
+ current->bio_lists->recursion = nopunt;
- *current->bio_list = nopunt;
+ bio_list_init(&nopunt);
+ while ((bio = bio_list_pop(¤t->bio_lists->queue)))
+ bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
+ current->bio_lists->queue = nopunt;
spin_lock(&bs->rescue_lock);
bio_list_merge(&bs->rescue_list, &punt);
@@ -380,6 +384,13 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
queue_work(bs->rescue_workqueue, &bs->rescue_work);
}
+static bool current_has_pending_bios(void)
+{
+ return current->bio_lists &&
+ (!bio_list_empty(¤t->bio_lists->queue) ||
+ !bio_list_empty(¤t->bio_lists->recursion));
+}
+
/**
* bio_alloc_bioset - allocate a bio for I/O
* @gfp_mask: the GFP_ mask given to the slab allocator
@@ -453,13 +464,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
*
* We solve this, and guarantee forward progress, with a rescuer
* workqueue per bio_set. If we go to allocate and there are
- * bios on current->bio_list, we first try the allocation
- * without __GFP_DIRECT_RECLAIM; if that fails, we punt those
- * bios we would be blocking to the rescuer workqueue before
- * we retry with the original gfp_flags.
+ * bios on current->bio_lists->{recursion,queue}, we first try the
+ * allocation without __GFP_DIRECT_RECLAIM; if that fails, we
+ * punt those bios we would be blocking to the rescuer
+ * workqueue before we retry with the original gfp_flags.
*/
- if (current->bio_list && !bio_list_empty(current->bio_list))
+ if (current_has_pending_bios())
gfp_mask &= ~__GFP_DIRECT_RECLAIM;
p = mempool_alloc(bs->bio_pool, gfp_mask);
diff --git a/block/blk-core.c b/block/blk-core.c
index 3cfd67d..74bceea 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2040,7 +2040,7 @@ end_io:
*/
blk_qc_t generic_make_request(struct bio *bio)
{
- struct bio_list bio_list_on_stack;
+ struct recursion_to_iteration_bio_lists bio_lists_on_stack;
blk_qc_t ret = BLK_QC_T_NONE;
if (!generic_make_request_checks(bio))
@@ -2049,15 +2049,20 @@ blk_qc_t generic_make_request(struct bio *bio)
/*
* We only want one ->make_request_fn to be active at a time, else
* stack usage with stacked devices could be a problem. So use
- * current->bio_list to keep a list of requests submited by a
- * make_request_fn function. current->bio_list is also used as a
+ * current->bio_lists to keep a list of requests submited by a
+ * make_request_fn function. current->bio_lists is also used as a
* flag to say if generic_make_request is currently active in this
* task or not. If it is NULL, then no make_request is active. If
* it is non-NULL, then a make_request is active, and new requests
- * should be added at the tail
+ * should be added at the tail of current->bio_lists->recursion;
+ * bios resulting from a call to blk_queue_split() from
+ * within ->make_request_fn() should be pushed back to the head of
+ * current->bio_lists->queue.
+ * After the current ->make_request_fn() returns, .recursion will be
+ * merged back to the head of .queue.
*/
- if (current->bio_list) {
- bio_list_add(current->bio_list, bio);
+ if (current->bio_lists) {
+ bio_list_add(¤t->bio_lists->recursion, bio);
goto out;
}
@@ -2066,35 +2071,34 @@ blk_qc_t generic_make_request(struct bio *bio)
* Before entering the loop, bio->bi_next is NULL (as all callers
* ensure that) so we have a list with a single bio.
* We pretend that we have just taken it off a longer list, so
- * we assign bio_list to a pointer to the bio_list_on_stack,
- * thus initialising the bio_list of new bios to be
- * added. ->make_request() may indeed add some more bios
- * through a recursive call to generic_make_request. If it
- * did, we find a non-NULL value in bio_list and re-enter the loop
- * from the top. In this case we really did just take the bio
- * of the top of the list (no pretending) and so remove it from
- * bio_list, and call into ->make_request() again.
+ * we assign bio_list to a pointer to the bio_lists_on_stack,
+ * thus initialising the bio_lists of new bios to be added.
+ * ->make_request() may indeed add some more bios to .recursion
+ * through a recursive call to generic_make_request. If it did,
+ * we find a non-NULL value in .recursion, merge .recursion back to the
+ * head of .queue, and re-enter the loop from the top. In this case we
+ * really did just take the bio of the top of the list (no pretending)
+ * and so remove it from .queue, and call into ->make_request() again.
*/
BUG_ON(bio->bi_next);
- bio_list_init(&bio_list_on_stack);
- current->bio_list = &bio_list_on_stack;
+ bio_list_init(&bio_lists_on_stack.queue);
+ current->bio_lists = &bio_lists_on_stack;
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
if (likely(blk_queue_enter(q, false) == 0)) {
+ bio_list_init(&bio_lists_on_stack.recursion);
ret = q->make_request_fn(q, bio);
-
blk_queue_exit(q);
-
- bio = bio_list_pop(current->bio_list);
+ bio_list_merge_head(&bio_lists_on_stack.queue,
+ &bio_lists_on_stack.recursion);
+ /* XXX bio_list_init(&bio_lists_on_stack.recursion); */
} else {
- struct bio *bio_next = bio_list_pop(current->bio_list);
-
bio_io_error(bio);
- bio = bio_next;
}
+ bio = bio_list_pop(¤t->bio_lists->queue);
} while (bio);
- current->bio_list = NULL; /* deactivate */
+ current->bio_lists = NULL; /* deactivate */
out:
return ret;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index c265348..df96327 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -172,6 +172,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
struct bio *split, *res;
unsigned nsegs;
+ BUG_ON(!current->bio_lists);
if (bio_op(*bio) == REQ_OP_DISCARD)
split = blk_bio_discard_split(q, *bio, bs, &nsegs);
else if (bio_op(*bio) == REQ_OP_WRITE_SAME)
@@ -190,7 +191,9 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
bio_chain(split, *bio);
trace_block_split(q, split, (*bio)->bi_iter.bi_sector);
- generic_make_request(*bio);
+ /* push back remainder, it may later be split further */
+ bio_list_add_head(¤t->bio_lists->queue, *bio);
+ /* and fake submission of a suitably sized piece */
*bio = split;
}
}
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 76f7534..731ec3b 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -450,7 +450,7 @@ void __bch_btree_node_write(struct btree *b, struct closure *parent)
trace_bcache_btree_write(b);
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
BUG_ON(b->written >= btree_blocks(b));
BUG_ON(b->written && !i->keys);
BUG_ON(btree_bset_first(b)->seq != i->seq);
@@ -544,7 +544,7 @@ static void bch_btree_leaf_dirty(struct btree *b, atomic_t *journal_ref)
/* Force write if set is too big */
if (set_bytes(i) > PAGE_SIZE - 48 &&
- !current->bio_list)
+ !current->bio_lists)
bch_btree_node_write(b, NULL);
}
@@ -889,7 +889,7 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
{
struct btree *b;
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
lockdep_assert_held(&c->bucket_lock);
@@ -976,7 +976,7 @@ retry:
b = mca_find(c, k);
if (!b) {
- if (current->bio_list)
+ if (current->bio_lists)
return ERR_PTR(-EAGAIN);
mutex_lock(&c->bucket_lock);
@@ -2127,7 +2127,7 @@ static int bch_btree_insert_node(struct btree *b, struct btree_op *op,
return 0;
split:
- if (current->bio_list) {
+ if (current->bio_lists) {
op->lock = b->c->root->level + 1;
return -EAGAIN;
} else if (op->lock <= b->c->root->level) {
@@ -2209,7 +2209,7 @@ int bch_btree_insert(struct cache_set *c, struct keylist *keys,
struct btree_insert_op op;
int ret = 0;
- BUG_ON(current->bio_list);
+ BUG_ON(current->bio_lists);
BUG_ON(bch_keylist_empty(keys));
bch_btree_op_init(&op.op, 0);
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 6571c81..ba0c325 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -174,7 +174,7 @@ static inline int dm_bufio_cache_index(struct dm_bufio_client *c)
#define DM_BUFIO_CACHE(c) (dm_bufio_caches[dm_bufio_cache_index(c)])
#define DM_BUFIO_CACHE_NAME(c) (dm_bufio_cache_names[dm_bufio_cache_index(c)])
-#define dm_bufio_in_request() (!!current->bio_list)
+#define dm_bufio_in_request() (!!current->bio_lists)
static void dm_bufio_lock(struct dm_bufio_client *c)
{
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b4f3352..b62e65f4 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -664,6 +664,13 @@ static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
}
}
+static inline bool current_has_pending_bios(void)
+{
+ return current->bio_lists && (
+ !bio_list_empty(¤t->bio_lists->queue) ||
+ !bio_list_empty(¤t->bio_lists->recursion));
+}
+
extern struct md_cluster_operations *md_cluster_ops;
static inline int mddev_is_clustered(struct mddev *mddev)
{
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 10e53cd..38790e3 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -876,8 +876,7 @@ static sector_t wait_barrier(struct r1conf *conf, struct bio *bio)
(!conf->barrier ||
((conf->start_next_window <
conf->next_resync + RESYNC_SECTORS) &&
- current->bio_list &&
- !bio_list_empty(current->bio_list))),
+ current_has_pending_bios())),
conf->resync_lock);
conf->nr_waiting--;
}
@@ -1014,7 +1013,7 @@ static void raid1_unplug(struct blk_plug_cb *cb, bool from_schedule)
struct r1conf *conf = mddev->private;
struct bio *bio;
- if (from_schedule || current->bio_list) {
+ if (from_schedule || current->bio_lists) {
spin_lock_irq(&conf->device_lock);
bio_list_merge(&conf->pending_bio_list, &plug->pending);
conf->pending_count += plug->pending_cnt;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 245640b..13a5341 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -945,8 +945,7 @@ static void wait_barrier(struct r10conf *conf)
wait_event_lock_irq(conf->wait_barrier,
!conf->barrier ||
(conf->nr_pending &&
- current->bio_list &&
- !bio_list_empty(current->bio_list)),
+ current_has_pending_bios()),
conf->resync_lock);
conf->nr_waiting--;
}
@@ -1022,7 +1021,7 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
struct r10conf *conf = mddev->private;
struct bio *bio;
- if (from_schedule || current->bio_list) {
+ if (from_schedule || current->bio_lists) {
spin_lock_irq(&conf->device_lock);
bio_list_merge(&conf->pending_bio_list, &plug->pending);
conf->pending_count += plug->pending_cnt;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b7e1a008..0b2b28e 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -541,6 +541,24 @@ struct bio_list {
struct bio *tail;
};
+/* for generic_make_request() */
+struct recursion_to_iteration_bio_lists {
+ /* For stacking drivers submitting to their respective backend,
+ * bios are added to the tail of .recursion, which is re-initialized
+ * before each call to ->make_request_fn() and after that returns,
+ * the whole .recursion queue is then merged back to the head of .queue.
+ *
+ * The recursion-to-iteration logic in generic_make_request() will
+ * peel off of .queue.head, processing bios in deepest-level-first
+ * "natural" order. */
+ struct bio_list recursion;
+
+ /* This keeps a list of to-be-processed bios.
+ * The "remainder" part resulting from calling blk_queue_split()
+ * will be pushed back to its head. */
+ struct bio_list queue;
+};
+
static inline int bio_list_empty(const struct bio_list *bl)
{
return bl->head == NULL;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada..146eedc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -128,7 +128,7 @@ struct sched_attr {
struct futex_pi_state;
struct robust_list_head;
-struct bio_list;
+struct recursion_to_iteration_bio_lists;
struct fs_struct;
struct perf_event_context;
struct blk_plug;
@@ -1727,7 +1727,7 @@ struct task_struct {
void *journal_info;
/* stacked block device info */
- struct bio_list *bio_list;
+ struct recursion_to_iteration_bio_lists *bio_lists;
#ifdef CONFIG_BLOCK
/* stack plugging */
--
1.9.1
^ permalink raw reply related
* [PATCH 0/1] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-08 15:04 UTC (permalink / raw)
To: Jens Axboe
Cc: Lars Ellenberg, NeilBrown, linux-raid, Martin K. Petersen,
Mike Snitzer, Peter Zijlstra, Jiri Kosina, Ming Lei, linux-kernel,
Zheng Liu, linux-block, Takashi Iwai, linux-bcache, Ingo Molnar,
Alasdair Kergon, Keith Busch, dm-devel, Shaohua Li,
Kent Overstreet, Kirill A. Shutemov, Roland Kammerer
Result of RFC previously discussed here:
https://lkml.org/lkml/2016/6/22/172
[RFC] block: fix blk_queue_split() resource exhaustion
Rebased to linux-block/for-4.8/core as of today.
Would also need to go to Stable 4.3 and later.
Lars Ellenberg (1):
block: fix blk_queue_split() resource exhaustion
block/bio.c | 27 +++++++++++++++++--------
block/blk-core.c | 50 +++++++++++++++++++++++++----------------------
block/blk-merge.c | 5 ++++-
drivers/md/bcache/btree.c | 12 ++++++------
drivers/md/dm-bufio.c | 2 +-
drivers/md/md.h | 7 +++++++
drivers/md/raid1.c | 5 ++---
drivers/md/raid10.c | 5 ++---
include/linux/bio.h | 18 +++++++++++++++++
include/linux/sched.h | 4 ++--
10 files changed, 88 insertions(+), 47 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: dm: raid456 basic support
From: Dan Carpenter @ 2016-07-08 14:13 UTC (permalink / raw)
To: NeilBrown, Heinz Mauelshagen; +Cc: linux-raid, Mike Snitzer
In-Reply-To: <87mvltf1d0.fsf@notabene.neil.brown.name>
Oops. Sorry. No, that won't help. We're looking at different code,
hence the confusion. I'm working off linux-next.
The problem is commit 6e20902e8f9e ('dm raid: fix failed
takeover/reshapes by keeping raid set frozen') where we changed "value"
to unsigned to int, but forgot to add underflow checks.
The warnings are:
drivers/md/dm-raid.c:1217 parse_raid_params() warn: no lower bound on 'value'
drivers/md/dm-raid.c:1264 parse_raid_params() warn: no lower bound on 'value'
drivers/md/dm-raid.c:1274 parse_raid_params() warn: no lower bound on 'value'
drivers/md/dm-raid.c
1147 if (kstrtoint(arg, 10, &value) < 0) {
^^^^^^
Set here.
1148 rs->ti->error = "Bad numerical argument given in raid params";
1149 return -EINVAL;
1150 }
1151
1152 if (!strcasecmp(key, dm_raid_arg_name_by_flag(CTR_FLAG_REBUILD))) {
1153 /*
1154 * "rebuild" is being passed in by userspace to provide
1155 * indexes of replaced devices and to set up additional
1156 * devices on raid level takeover.
1157 */
1158 if (!__within_range(value, 0, rs->raid_disks - 1)) {
1159 rs->ti->error = "Invalid rebuild index given";
1160 return -EINVAL;
1161 }
1162
1163 if (test_and_set_bit(value, (void *) rs->rebuild_disks)) {
1164 rs->ti->error = "rebuild for this index already given";
1165 return -EINVAL;
1166 }
1167
1168 rd = rs->dev + value;
1169 clear_bit(In_sync, &rd->rdev.flags);
1170 clear_bit(Faulty, &rd->rdev.flags);
1171 rd->rdev.recovery_offset = 0;
1172 set_bit(__CTR_FLAG_REBUILD, &rs->ctr_flags);
1173 } else if (!strcasecmp(key, dm_raid_arg_name_by_flag(CTR_FLAG_WRITE_MOSTLY))) {
1174 if (!rt_is_raid1(rt)) {
1175 rs->ti->error = "write_mostly option is only valid for RAID1";
1176 return -EINVAL;
1177 }
1178
1179 if (!__within_range(value, 0, rs->md.raid_disks - 1)) {
1180 rs->ti->error = "Invalid write_mostly index given";
1181 return -EINVAL;
1182 }
1183
1184 set_bit(WriteMostly, &rs->dev[value].rdev.flags);
1185 set_bit(__CTR_FLAG_WRITE_MOSTLY, &rs->ctr_flags);
1186 } else if (!strcasecmp(key, dm_raid_arg_name_by_flag(CTR_FLAG_MAX_WRITE_BEHIND))) {
1187 if (!rt_is_raid1(rt)) {
1188 rs->ti->error = "max_write_behind option is only valid for RAID1";
1189 return -EINVAL;
1190 }
1191
1192 if (test_and_set_bit(__CTR_FLAG_MAX_WRITE_BEHIND, &rs->ctr_flags)) {
1193 rs->ti->error = "Only one max_write_behind argument pair allowed";
1194 return -EINVAL;
1195 }
1196
1197 /*
1198 * In device-mapper, we specify things in sectors, but
1199 * MD records this value in kB
1200 */
1201 value /= 2;
1202 if (value > COUNTER_MAX) {
1203 rs->ti->error = "Max write-behind limit out of range";
1204 return -EINVAL;
1205 }
1206
1207 rs->md.bitmap_info.max_write_behind = value;
1208 } else if (!strcasecmp(key, dm_raid_arg_name_by_flag(CTR_FLAG_DAEMON_SLEEP))) {
1209 if (test_and_set_bit(__CTR_FLAG_DAEMON_SLEEP, &rs->ctr_flags)) {
1210 rs->ti->error = "Only one daemon_sleep argument pair allowed";
1211 return -EINVAL;
1212 }
1213 if (!value || (value > MAX_SCHEDULE_TIMEOUT)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Not checked for negatives.
1214 rs->ti->error = "daemon sleep period out of range";
1215 return -EINVAL;
1216 }
1217 rs->md.bitmap_info.daemon_sleep = value;
regards,
dan carpenter
^ permalink raw reply
* Re: [dm-devel] [RFC] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-08 13:59 UTC (permalink / raw)
To: NeilBrown
Cc: Jens Axboe, linux-raid, Martin K. Petersen, Mike Snitzer,
Peter Zijlstra, Jiri Kosina, Ming Lei, linux-kernel, Zheng Liu,
linux-block, Takashi Iwai, linux-bcache, Ingo Molnar,
Alasdair Kergon, Keith Busch, dm-devel, Shaohua Li,
Kent Overstreet, Kirill A. Shutemov, Roland Kammerer
In-Reply-To: <20160708130035.GW13335@soda.linbit>
On Fri, Jul 08, 2016 at 07:24:24PM +0600, Pavel Goran wrote:
> Friday, July 8, 2016, 7:00:35 PM, you wrote:
> > On Fri, Jul 08, 2016 at 07:39:36PM +1000, NeilBrown wrote:
> >> I think we just might be in violent agreement.
>
> > I thought so, too :-)
>
> > Should I merge both patches,
> > rename to ".queue" and ".tmp",
> > and submit for inclusion?
>
> Could you please *not* use ".tmp"? I have a feeling it's even worse than "X"
> and "Y". :)
>
> I suggest to leave it as ".recursion", or maybe use something like
> ".downstream".
"Naming is hard" :-)
.recursion and .queue, then,
because .remainder does no longer fit.
Also, what about "Cc: Stable"?
The patch does not strictly fall into the "stable" rules,
(too many lines...).
But it should go there anyways, I think.
I'll just add those tags, and wait for the heat?
Lars Ellenberg
^ permalink raw reply
* Dear Beneficiary,
From: Sarah Michael @ 2016-07-08 13:49 UTC (permalink / raw)
Dear Beneficiary,
I'm happy to inform you that your email address has been selected
among those that have compensation payment of ($2.5 Million USD), The
INTERNATIONAL MONETARY FUND contacted us for the release of your money
a couple of hours ago due to your allocated security code. We are
unable to complete a transfer directed at an email address, so we
require some more information in order to complete this transfer, But
for security purpose the money has been loaded in a prepaid Visa Debit
Card on your behalf, You Withdraw cash at ATMs and make purchases
anywhere debit cards are accepted, including retail stores,grocery
stores, restaurants and pharmacies, You are advised that a maximum
withdrawal value of $10,000.00 USD is permitted daily.
Therefore arrangements has been perfectly concluded to effect the
delivery of your Visa Debit Card to your doorstep as soon as
youre-confirm the information's below.
1) Your full name:
2) Mobile number:
3) Delivery address:
4) Zip code:
I am waiting to hear from you soon,
With Best Regards,
Ms:Sarah Michael.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox