* raid5 async_xor: sleep in atomic @ 2015-12-22 11:58 Stanislav Samsonov 2015-12-23 2:34 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Stanislav Samsonov @ 2015-12-22 11:58 UTC (permalink / raw) To: linux-raid Hi, Kernel 4.1.3 : there is some troubling kernel message that shows up after enabling CONFIG_DEBUG_ATOMIC_SLEEP and testing DMA XOR acceleration for raid5: BUG: sleeping function called from invalid context at mm/mempool.c:320 in_atomic(): 1, irqs_disabled(): 0, pid: 1048, name: md127_raid5 INFO: lockdep is turned off. CPU: 1 PID: 1048 Comm: md127_raid5 Not tainted 4.1.15.alpine.1-dirty #1 Hardware name: Annapurna Labs Alpine [<c00169d8>] (unwind_backtrace) from [<c0012a78>] (show_stack+0x10/0x14) [<c0012a78>] (show_stack) from [<c07462ec>] (dump_stack+0x80/0xb4) [<c07462ec>] (dump_stack) from [<c00bf2f0>] (mempool_alloc+0x68/0x13c) [<c00bf2f0>] (mempool_alloc) from [<c041c9b4>] (dmaengine_get_unmap_data+0x24/0x4c) [<c041c9b4>] (dmaengine_get_unmap_data) from [<c03a8084>] (async_xor_val+0x60/0x3a0) [<c03a8084>] (async_xor_val) from [<c058e4c0>] (raid_run_ops+0xb70/0x1248) [<c058e4c0>] (raid_run_ops) from [<c05915d4>] (handle_stripe+0x1068/0x22a8) [<c05915d4>] (handle_stripe) from [<c0592ae4>] (handle_active_stripes+0x2d0/0x3dc) [<c0592ae4>] (handle_active_stripes) from [<c059300c>] (raid5d+0x384/0x5b0) [<c059300c>] (raid5d) from [<c059db6c>] (md_thread+0x114/0x138) [<c059db6c>] (md_thread) from [<c0042d54>] (kthread+0xe4/0x104) [<c0042d54>] (kthread) from [<c000f658>] (ret_from_fork+0x14/0x3c) The reason is that async_xor_val() in crypto/async_tx/async_xor.c is called in atomic context (preemption disabled) by raid_run_ops(). Then it calls dmaengine_get_unmap_data() an then mempool_alloc() with GFP_NOIO flag - this allocation type might sleep under some condition. Checked latest kernel 4.3 and it has exactly same flow. Any advice regarding this issue? Thanks, Slava Samsonov ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-22 11:58 raid5 async_xor: sleep in atomic Stanislav Samsonov @ 2015-12-23 2:34 ` NeilBrown 2015-12-23 17:35 ` Dan Williams 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2015-12-23 2:34 UTC (permalink / raw) To: Stanislav Samsonov, linux-raid; +Cc: dan.j.williams [-- Attachment #1: Type: text/plain, Size: 2197 bytes --] On Tue, Dec 22 2015, Stanislav Samsonov wrote: > Hi, > > Kernel 4.1.3 : there is some troubling kernel message that shows up > after enabling CONFIG_DEBUG_ATOMIC_SLEEP and testing DMA XOR > acceleration for raid5: > > BUG: sleeping function called from invalid context at mm/mempool.c:320 > in_atomic(): 1, irqs_disabled(): 0, pid: 1048, name: md127_raid5 > INFO: lockdep is turned off. > CPU: 1 PID: 1048 Comm: md127_raid5 Not tainted 4.1.15.alpine.1-dirty #1 > Hardware name: Annapurna Labs Alpine > [<c00169d8>] (unwind_backtrace) from [<c0012a78>] (show_stack+0x10/0x14) > [<c0012a78>] (show_stack) from [<c07462ec>] (dump_stack+0x80/0xb4) > [<c07462ec>] (dump_stack) from [<c00bf2f0>] (mempool_alloc+0x68/0x13c) > [<c00bf2f0>] (mempool_alloc) from [<c041c9b4>] > (dmaengine_get_unmap_data+0x24/0x4c) > [<c041c9b4>] (dmaengine_get_unmap_data) from [<c03a8084>] > (async_xor_val+0x60/0x3a0) > [<c03a8084>] (async_xor_val) from [<c058e4c0>] (raid_run_ops+0xb70/0x1248) > [<c058e4c0>] (raid_run_ops) from [<c05915d4>] (handle_stripe+0x1068/0x22a8) > [<c05915d4>] (handle_stripe) from [<c0592ae4>] > (handle_active_stripes+0x2d0/0x3dc) > [<c0592ae4>] (handle_active_stripes) from [<c059300c>] (raid5d+0x384/0x5b0) > [<c059300c>] (raid5d) from [<c059db6c>] (md_thread+0x114/0x138) > [<c059db6c>] (md_thread) from [<c0042d54>] (kthread+0xe4/0x104) > [<c0042d54>] (kthread) from [<c000f658>] (ret_from_fork+0x14/0x3c) > > The reason is that async_xor_val() in crypto/async_tx/async_xor.c is > called in atomic context (preemption disabled) by raid_run_ops(). Then > it calls dmaengine_get_unmap_data() an then mempool_alloc() with > GFP_NOIO flag - this allocation type might sleep under some condition. > > Checked latest kernel 4.3 and it has exactly same flow. > > Any advice regarding this issue? Changing the GFP_NOIO to GFP_ATOMIC in all the calls to dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the issue... or make it crash even worse :-) Dan: do you have any wisdom here? The xor is using the percpu data in raid5, so it cannot be sleep, but GFP_NOIO allows sleep. Does the code handle failure to get_unmap_data() safely? It looks like it probably does. NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-23 2:34 ` NeilBrown @ 2015-12-23 17:35 ` Dan Williams 2015-12-23 22:39 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Dan Williams @ 2015-12-23 17:35 UTC (permalink / raw) To: NeilBrown; +Cc: Stanislav Samsonov, linux-raid On Tue, Dec 22, 2015 at 6:34 PM, NeilBrown <neilb@suse.com> wrote: > On Tue, Dec 22 2015, Stanislav Samsonov wrote: > >> Hi, >> >> Kernel 4.1.3 : there is some troubling kernel message that shows up >> after enabling CONFIG_DEBUG_ATOMIC_SLEEP and testing DMA XOR >> acceleration for raid5: >> >> BUG: sleeping function called from invalid context at mm/mempool.c:320 >> in_atomic(): 1, irqs_disabled(): 0, pid: 1048, name: md127_raid5 >> INFO: lockdep is turned off. >> CPU: 1 PID: 1048 Comm: md127_raid5 Not tainted 4.1.15.alpine.1-dirty #1 >> Hardware name: Annapurna Labs Alpine >> [<c00169d8>] (unwind_backtrace) from [<c0012a78>] (show_stack+0x10/0x14) >> [<c0012a78>] (show_stack) from [<c07462ec>] (dump_stack+0x80/0xb4) >> [<c07462ec>] (dump_stack) from [<c00bf2f0>] (mempool_alloc+0x68/0x13c) >> [<c00bf2f0>] (mempool_alloc) from [<c041c9b4>] >> (dmaengine_get_unmap_data+0x24/0x4c) >> [<c041c9b4>] (dmaengine_get_unmap_data) from [<c03a8084>] >> (async_xor_val+0x60/0x3a0) >> [<c03a8084>] (async_xor_val) from [<c058e4c0>] (raid_run_ops+0xb70/0x1248) >> [<c058e4c0>] (raid_run_ops) from [<c05915d4>] (handle_stripe+0x1068/0x22a8) >> [<c05915d4>] (handle_stripe) from [<c0592ae4>] >> (handle_active_stripes+0x2d0/0x3dc) >> [<c0592ae4>] (handle_active_stripes) from [<c059300c>] (raid5d+0x384/0x5b0) >> [<c059300c>] (raid5d) from [<c059db6c>] (md_thread+0x114/0x138) >> [<c059db6c>] (md_thread) from [<c0042d54>] (kthread+0xe4/0x104) >> [<c0042d54>] (kthread) from [<c000f658>] (ret_from_fork+0x14/0x3c) >> >> The reason is that async_xor_val() in crypto/async_tx/async_xor.c is >> called in atomic context (preemption disabled) by raid_run_ops(). Then >> it calls dmaengine_get_unmap_data() an then mempool_alloc() with >> GFP_NOIO flag - this allocation type might sleep under some condition. >> >> Checked latest kernel 4.3 and it has exactly same flow. >> >> Any advice regarding this issue? > > Changing the GFP_NOIO to GFP_ATOMIC in all the calls to > dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the > issue... or make it crash even worse :-) > > Dan: do you have any wisdom here? The xor is using the percpu data in > raid5, so it cannot be sleep, but GFP_NOIO allows sleep. > Does the code handle failure to get_unmap_data() safely? It looks like > it probably does. Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC allocations to consume emergency reserves for a performance optimization. Longer term async_tx needs to be merged into md directly as we can allocate this unmap data statically per-stripe rather than per request. This asyntc_tx re-write has been on the todo list for years, but never seems to make it to the top. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-23 17:35 ` Dan Williams @ 2015-12-23 22:39 ` NeilBrown 2015-12-23 22:46 ` Dan Williams 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2015-12-23 22:39 UTC (permalink / raw) To: Dan Williams; +Cc: Stanislav Samsonov, linux-raid [-- Attachment #1: Type: text/plain, Size: 4947 bytes --] On Thu, Dec 24 2015, Dan Williams wrote: >> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to >> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the >> issue... or make it crash even worse :-) >> >> Dan: do you have any wisdom here? The xor is using the percpu data in >> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. >> Does the code handle failure to get_unmap_data() safely? It looks like >> it probably does. > > Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC > allocations to consume emergency reserves for a performance > optimization. Longer term async_tx needs to be merged into md > directly as we can allocate this unmap data statically per-stripe > rather than per request. This asyntc_tx re-write has been on the todo > list for years, but never seems to make it to the top. So the following maybe? If I could get an acked-by from you Dan, and a Tested-by: from you Slava, I'll submit upstream. Thanks, NeilBrown From: NeilBrown <neilb@suse.com> Date: Thu, 24 Dec 2015 09:35:18 +1100 Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO These async_XX functions are called from md/raid5 in an atomic section, between get_cpu() and put_cpu(), so they must not sleep. So use GFP_NOWAIT rather than GFP_IO. Dan Williams writes: Longer term async_tx needs to be merged into md directly as we can allocate this unmap data statically per-stripe rather than per request. Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> Signed-off-by: NeilBrown <neilb@suse.com> diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c index f8c0b8dbeb75..88bc8e6b2a54 100644 --- a/crypto/async_tx/async_memcpy.c +++ b/crypto/async_tx/async_memcpy.c @@ -53,7 +53,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset, struct dmaengine_unmap_data *unmap = NULL; if (device) - unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOWAIT); if (unmap && is_dma_copy_aligned(device, src_offset, dest_offset, len)) { unsigned long dma_prep_flags = 0; diff --git a/crypto/async_tx/async_pq.c b/crypto/async_tx/async_pq.c index 5d355e0c2633..c0748bbd4c08 100644 --- a/crypto/async_tx/async_pq.c +++ b/crypto/async_tx/async_pq.c @@ -188,7 +188,7 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks, BUG_ON(disks > 255 || !(P(blocks, disks) || Q(blocks, disks))); if (device) - unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOWAIT); /* XORing P/Q is only implemented in software */ if (unmap && !(submit->flags & ASYNC_TX_PQ_XOR_DST) && @@ -307,7 +307,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks, BUG_ON(disks < 4); if (device) - unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOWAIT); if (unmap && disks <= dma_maxpq(device, 0) && is_dma_pq_aligned(device, offset, 0, len)) { diff --git a/crypto/async_tx/async_raid6_recov.c b/crypto/async_tx/async_raid6_recov.c index 934a84981495..8fab6275ea1f 100644 --- a/crypto/async_tx/async_raid6_recov.c +++ b/crypto/async_tx/async_raid6_recov.c @@ -41,7 +41,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef, u8 *a, *b, *c; if (dma) - unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOIO); + unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOWAIT); if (unmap) { struct device *dev = dma->dev; @@ -105,7 +105,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len, u8 *d, *s; if (dma) - unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOIO); + unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOWAIT); if (unmap) { dma_addr_t dma_dest[2]; diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c index e1bce26cd4f9..da75777f2b3f 100644 --- a/crypto/async_tx/async_xor.c +++ b/crypto/async_tx/async_xor.c @@ -182,7 +182,7 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset, BUG_ON(src_cnt <= 1); if (device) - unmap = dmaengine_get_unmap_data(device->dev, src_cnt+1, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, src_cnt+1, GFP_NOWAIT); if (unmap && is_dma_xor_aligned(device, offset, 0, len)) { struct dma_async_tx_descriptor *tx; @@ -278,7 +278,7 @@ async_xor_val(struct page *dest, struct page **src_list, unsigned int offset, BUG_ON(src_cnt <= 1); if (device) - unmap = dmaengine_get_unmap_data(device->dev, src_cnt, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, src_cnt, GFP_NOWAIT); if (unmap && src_cnt <= device->max_xor && is_dma_xor_aligned(device, offset, 0, len)) { [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-23 22:39 ` NeilBrown @ 2015-12-23 22:46 ` Dan Williams 2015-12-28 8:43 ` Stanislav Samsonov 0 siblings, 1 reply; 11+ messages in thread From: Dan Williams @ 2015-12-23 22:46 UTC (permalink / raw) To: NeilBrown; +Cc: Stanislav Samsonov, linux-raid On Wed, Dec 23, 2015 at 2:39 PM, NeilBrown <neilb@suse.com> wrote: > On Thu, Dec 24 2015, Dan Williams wrote: >>> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to >>> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the >>> issue... or make it crash even worse :-) >>> >>> Dan: do you have any wisdom here? The xor is using the percpu data in >>> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. >>> Does the code handle failure to get_unmap_data() safely? It looks like >>> it probably does. >> >> Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC >> allocations to consume emergency reserves for a performance >> optimization. Longer term async_tx needs to be merged into md >> directly as we can allocate this unmap data statically per-stripe >> rather than per request. This asyntc_tx re-write has been on the todo >> list for years, but never seems to make it to the top. > > So the following maybe? > If I could get an acked-by from you Dan, and a Tested-by: from you > Slava, I'll submit upstream. > > Thanks, > NeilBrown > > From: NeilBrown <neilb@suse.com> > Date: Thu, 24 Dec 2015 09:35:18 +1100 > Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO > > These async_XX functions are called from md/raid5 in an atomic > section, between get_cpu() and put_cpu(), so they must not sleep. > So use GFP_NOWAIT rather than GFP_IO. > > Dan Williams writes: Longer term async_tx needs to be merged into md > directly as we can allocate this unmap data statically per-stripe > rather than per request. > > Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> > Signed-off-by: NeilBrown <neilb@suse.com> Acked-by: Dan Williams <dan.j.williams@intel.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-23 22:46 ` Dan Williams @ 2015-12-28 8:43 ` Stanislav Samsonov 2016-01-04 1:33 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Stanislav Samsonov @ 2015-12-28 8:43 UTC (permalink / raw) To: Dan Williams; +Cc: NeilBrown, linux-raid On 24 December 2015 at 00:46, Dan Williams <dan.j.williams@intel.com> wrote: > > On Wed, Dec 23, 2015 at 2:39 PM, NeilBrown <neilb@suse.com> wrote: > > On Thu, Dec 24 2015, Dan Williams wrote: > >>> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to > >>> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the > >>> issue... or make it crash even worse :-) > >>> > >>> Dan: do you have any wisdom here? The xor is using the percpu data in > >>> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. > >>> Does the code handle failure to get_unmap_data() safely? It looks like > >>> it probably does. > >> > >> Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC > >> allocations to consume emergency reserves for a performance > >> optimization. Longer term async_tx needs to be merged into md > >> directly as we can allocate this unmap data statically per-stripe > >> rather than per request. This asyntc_tx re-write has been on the todo > >> list for years, but never seems to make it to the top. > > > > So the following maybe? > > If I could get an acked-by from you Dan, and a Tested-by: from you > > Slava, I'll submit upstream. > > > > Thanks, > > NeilBrown > > > > From: NeilBrown <neilb@suse.com> > > Date: Thu, 24 Dec 2015 09:35:18 +1100 > > Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO > > > > These async_XX functions are called from md/raid5 in an atomic > > section, between get_cpu() and put_cpu(), so they must not sleep. > > So use GFP_NOWAIT rather than GFP_IO. > > > > Dan Williams writes: Longer term async_tx needs to be merged into md > > directly as we can allocate this unmap data statically per-stripe > > rather than per request. > > > > Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> > > Signed-off-by: NeilBrown <neilb@suse.com> > > Acked-by: Dan Williams <dan.j.williams@intel.com> Tested-by: Slava Samsonov <slava@annapurnalabs.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2015-12-28 8:43 ` Stanislav Samsonov @ 2016-01-04 1:33 ` NeilBrown 2016-01-04 17:28 ` Dan Williams 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2016-01-04 1:33 UTC (permalink / raw) To: Stanislav Samsonov, Dan Williams; +Cc: linux-raid [-- Attachment #1: Type: text/plain, Size: 2376 bytes --] On Mon, Dec 28 2015, Stanislav Samsonov wrote: > On 24 December 2015 at 00:46, Dan Williams <dan.j.williams@intel.com> wrote: >> >> On Wed, Dec 23, 2015 at 2:39 PM, NeilBrown <neilb@suse.com> wrote: >> > On Thu, Dec 24 2015, Dan Williams wrote: >> >>> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to >> >>> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the >> >>> issue... or make it crash even worse :-) >> >>> >> >>> Dan: do you have any wisdom here? The xor is using the percpu data in >> >>> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. >> >>> Does the code handle failure to get_unmap_data() safely? It looks like >> >>> it probably does. >> >> >> >> Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC >> >> allocations to consume emergency reserves for a performance >> >> optimization. Longer term async_tx needs to be merged into md >> >> directly as we can allocate this unmap data statically per-stripe >> >> rather than per request. This asyntc_tx re-write has been on the todo >> >> list for years, but never seems to make it to the top. >> > >> > So the following maybe? >> > If I could get an acked-by from you Dan, and a Tested-by: from you >> > Slava, I'll submit upstream. >> > >> > Thanks, >> > NeilBrown >> > >> > From: NeilBrown <neilb@suse.com> >> > Date: Thu, 24 Dec 2015 09:35:18 +1100 >> > Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO >> > >> > These async_XX functions are called from md/raid5 in an atomic >> > section, between get_cpu() and put_cpu(), so they must not sleep. >> > So use GFP_NOWAIT rather than GFP_IO. >> > >> > Dan Williams writes: Longer term async_tx needs to be merged into md >> > directly as we can allocate this unmap data statically per-stripe >> > rather than per request. >> > >> > Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> >> > Signed-off-by: NeilBrown <neilb@suse.com> >> >> Acked-by: Dan Williams <dan.j.williams@intel.com> > > Tested-by: Slava Samsonov <slava@annapurnalabs.com> Thanks. I guess this was problem was introduced by Commit: 7476bd79fc01 ("async_pq: convert to dmaengine_unmap_data") in 3.13. Do we think it deserves to go to -stable? (I just realised that this is really Dan's code more than mine, so why am I submitting it ??? But we are here now so it may as well go in through the md tree.) NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2016-01-04 1:33 ` NeilBrown @ 2016-01-04 17:28 ` Dan Williams 2016-01-06 9:08 ` Vinod Koul 0 siblings, 1 reply; 11+ messages in thread From: Dan Williams @ 2016-01-04 17:28 UTC (permalink / raw) To: NeilBrown; +Cc: Stanislav Samsonov, linux-raid, dmaengine, Vinod Koul On Sun, Jan 3, 2016 at 5:33 PM, NeilBrown <neilb@suse.com> wrote: > On Mon, Dec 28 2015, Stanislav Samsonov wrote: > >> On 24 December 2015 at 00:46, Dan Williams <dan.j.williams@intel.com> wrote: >>> >>> On Wed, Dec 23, 2015 at 2:39 PM, NeilBrown <neilb@suse.com> wrote: >>> > On Thu, Dec 24 2015, Dan Williams wrote: >>> >>> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to >>> >>> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the >>> >>> issue... or make it crash even worse :-) >>> >>> >>> >>> Dan: do you have any wisdom here? The xor is using the percpu data in >>> >>> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. >>> >>> Does the code handle failure to get_unmap_data() safely? It looks like >>> >>> it probably does. >>> >> >>> >> Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC >>> >> allocations to consume emergency reserves for a performance >>> >> optimization. Longer term async_tx needs to be merged into md >>> >> directly as we can allocate this unmap data statically per-stripe >>> >> rather than per request. This asyntc_tx re-write has been on the todo >>> >> list for years, but never seems to make it to the top. >>> > >>> > So the following maybe? >>> > If I could get an acked-by from you Dan, and a Tested-by: from you >>> > Slava, I'll submit upstream. >>> > >>> > Thanks, >>> > NeilBrown >>> > >>> > From: NeilBrown <neilb@suse.com> >>> > Date: Thu, 24 Dec 2015 09:35:18 +1100 >>> > Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO >>> > >>> > These async_XX functions are called from md/raid5 in an atomic >>> > section, between get_cpu() and put_cpu(), so they must not sleep. >>> > So use GFP_NOWAIT rather than GFP_IO. >>> > >>> > Dan Williams writes: Longer term async_tx needs to be merged into md >>> > directly as we can allocate this unmap data statically per-stripe >>> > rather than per request. >>> > >>> > Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> >>> > Signed-off-by: NeilBrown <neilb@suse.com> >>> >>> Acked-by: Dan Williams <dan.j.williams@intel.com> >> >> Tested-by: Slava Samsonov <slava@annapurnalabs.com> > > Thanks. > > I guess this was problem was introduced by > Commit: 7476bd79fc01 ("async_pq: convert to dmaengine_unmap_data") > in 3.13. Yes. > Do we think it deserves to go to -stable? I think so, yes. > (I just realised that this is really Dan's code more than mine, > so why am I submitting it ??? True! I was grateful for your offer, but I should have taken over coordination... > But we are here now so it may as well go > in through the md tree.) That or Vinod is maintaining drivers/dma/ these days (added Cc's). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: raid5 async_xor: sleep in atomic 2016-01-04 17:28 ` Dan Williams @ 2016-01-06 9:08 ` Vinod Koul 2016-01-07 0:02 ` [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Vinod Koul @ 2016-01-06 9:08 UTC (permalink / raw) To: Dan Williams; +Cc: NeilBrown, Stanislav Samsonov, linux-raid, dmaengine On Mon, Jan 04, 2016 at 09:28:52AM -0800, Dan Williams wrote: > On Sun, Jan 3, 2016 at 5:33 PM, NeilBrown <neilb@suse.com> wrote: > > On Mon, Dec 28 2015, Stanislav Samsonov wrote: > > > >> On 24 December 2015 at 00:46, Dan Williams <dan.j.williams@intel.com> wrote: > >>> > >>> On Wed, Dec 23, 2015 at 2:39 PM, NeilBrown <neilb@suse.com> wrote: > >>> > On Thu, Dec 24 2015, Dan Williams wrote: > >>> >>> Changing the GFP_NOIO to GFP_ATOMIC in all the calls to > >>> >>> dmaengine_get_unmap_data() in crypto/async_tx/ would probably fix the > >>> >>> issue... or make it crash even worse :-) > >>> >>> > >>> >>> Dan: do you have any wisdom here? The xor is using the percpu data in > >>> >>> raid5, so it cannot be sleep, but GFP_NOIO allows sleep. > >>> >>> Does the code handle failure to get_unmap_data() safely? It looks like > >>> >>> it probably does. > >>> >> > >>> >> Those GFP_NOIO should move to GFP_NOWAIT. We don't want GFP_ATOMIC > >>> >> allocations to consume emergency reserves for a performance > >>> >> optimization. Longer term async_tx needs to be merged into md > >>> >> directly as we can allocate this unmap data statically per-stripe > >>> >> rather than per request. This asyntc_tx re-write has been on the todo > >>> >> list for years, but never seems to make it to the top. > >>> > > >>> > So the following maybe? > >>> > If I could get an acked-by from you Dan, and a Tested-by: from you > >>> > Slava, I'll submit upstream. > >>> > > >>> > Thanks, > >>> > NeilBrown > >>> > > >>> > From: NeilBrown <neilb@suse.com> > >>> > Date: Thu, 24 Dec 2015 09:35:18 +1100 > >>> > Subject: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO > >>> > > >>> > These async_XX functions are called from md/raid5 in an atomic > >>> > section, between get_cpu() and put_cpu(), so they must not sleep. > >>> > So use GFP_NOWAIT rather than GFP_IO. > >>> > > >>> > Dan Williams writes: Longer term async_tx needs to be merged into md > >>> > directly as we can allocate this unmap data statically per-stripe > >>> > rather than per request. > >>> > > >>> > Reported-by: Stanislav Samsonov <slava@annapurnalabs.com> > >>> > Signed-off-by: NeilBrown <neilb@suse.com> > >>> > >>> Acked-by: Dan Williams <dan.j.williams@intel.com> > >> > >> Tested-by: Slava Samsonov <slava@annapurnalabs.com> > > > > Thanks. > > > > I guess this was problem was introduced by > > Commit: 7476bd79fc01 ("async_pq: convert to dmaengine_unmap_data") > > in 3.13. > > Yes. > > > Do we think it deserves to go to -stable? > > I think so, yes. > > > (I just realised that this is really Dan's code more than mine, > > so why am I submitting it ??? > > True! I was grateful for your offer, but I should have taken over > coordination... > > > But we are here now so it may as well go > > in through the md tree.) > > That or Vinod is maintaining drivers/dma/ these days (added Cc's). I can queue it up, pls send me the patch with ACKs. Looks like this might be 4.4 material, I am finalizing that in next day or so :) -- ~Vinod ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO 2016-01-06 9:08 ` Vinod Koul @ 2016-01-07 0:02 ` NeilBrown 2016-01-07 5:39 ` Vinod Koul 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2016-01-07 0:02 UTC (permalink / raw) To: Vinod Koul, Dan Williams; +Cc: Stanislav Samsonov, linux-raid, dmaengine [-- Attachment #1: Type: text/plain, Size: 4391 bytes --] These async_XX functions are called from md/raid5 in an atomic section, between get_cpu() and put_cpu(), so they must not sleep. So use GFP_NOWAIT rather than GFP_IO. Dan Williams writes: Longer term async_tx needs to be merged into md directly as we can allocate this unmap data statically per-stripe rather than per request. Fixed: 7476bd79fc01 ("async_pq: convert to dmaengine_unmap_data") Cc: stable@vger.kernel.org (v3.13+) Reported-and-tested-by: Stanislav Samsonov <slava@annapurnalabs.com> Acked-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: NeilBrown <neilb@suse.com> --- Thanks for taking this Vinod. It is currently in linux-next from my md tree, but I've just de-staged it so the next linux-next won't have it from me. NeilBrown crypto/async_tx/async_memcpy.c | 2 +- crypto/async_tx/async_pq.c | 4 ++-- crypto/async_tx/async_raid6_recov.c | 4 ++-- crypto/async_tx/async_xor.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c index f8c0b8dbeb75..88bc8e6b2a54 100644 --- a/crypto/async_tx/async_memcpy.c +++ b/crypto/async_tx/async_memcpy.c @@ -53,7 +53,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset, struct dmaengine_unmap_data *unmap = NULL; if (device) - unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOWAIT); if (unmap && is_dma_copy_aligned(device, src_offset, dest_offset, len)) { unsigned long dma_prep_flags = 0; diff --git a/crypto/async_tx/async_pq.c b/crypto/async_tx/async_pq.c index 5d355e0c2633..c0748bbd4c08 100644 --- a/crypto/async_tx/async_pq.c +++ b/crypto/async_tx/async_pq.c @@ -188,7 +188,7 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks, BUG_ON(disks > 255 || !(P(blocks, disks) || Q(blocks, disks))); if (device) - unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOWAIT); /* XORing P/Q is only implemented in software */ if (unmap && !(submit->flags & ASYNC_TX_PQ_XOR_DST) && @@ -307,7 +307,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks, BUG_ON(disks < 4); if (device) - unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, disks, GFP_NOWAIT); if (unmap && disks <= dma_maxpq(device, 0) && is_dma_pq_aligned(device, offset, 0, len)) { diff --git a/crypto/async_tx/async_raid6_recov.c b/crypto/async_tx/async_raid6_recov.c index 934a84981495..8fab6275ea1f 100644 --- a/crypto/async_tx/async_raid6_recov.c +++ b/crypto/async_tx/async_raid6_recov.c @@ -41,7 +41,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef, u8 *a, *b, *c; if (dma) - unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOIO); + unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOWAIT); if (unmap) { struct device *dev = dma->dev; @@ -105,7 +105,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len, u8 *d, *s; if (dma) - unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOIO); + unmap = dmaengine_get_unmap_data(dma->dev, 3, GFP_NOWAIT); if (unmap) { dma_addr_t dma_dest[2]; diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c index e1bce26cd4f9..da75777f2b3f 100644 --- a/crypto/async_tx/async_xor.c +++ b/crypto/async_tx/async_xor.c @@ -182,7 +182,7 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset, BUG_ON(src_cnt <= 1); if (device) - unmap = dmaengine_get_unmap_data(device->dev, src_cnt+1, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, src_cnt+1, GFP_NOWAIT); if (unmap && is_dma_xor_aligned(device, offset, 0, len)) { struct dma_async_tx_descriptor *tx; @@ -278,7 +278,7 @@ async_xor_val(struct page *dest, struct page **src_list, unsigned int offset, BUG_ON(src_cnt <= 1); if (device) - unmap = dmaengine_get_unmap_data(device->dev, src_cnt, GFP_NOIO); + unmap = dmaengine_get_unmap_data(device->dev, src_cnt, GFP_NOWAIT); if (unmap && src_cnt <= device->max_xor && is_dma_xor_aligned(device, offset, 0, len)) { -- 2.6.4 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO 2016-01-07 0:02 ` [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO NeilBrown @ 2016-01-07 5:39 ` Vinod Koul 0 siblings, 0 replies; 11+ messages in thread From: Vinod Koul @ 2016-01-07 5:39 UTC (permalink / raw) To: NeilBrown; +Cc: Dan Williams, Stanislav Samsonov, linux-raid, dmaengine [-- Attachment #1: Type: text/plain, Size: 1043 bytes --] On Thu, Jan 07, 2016 at 11:02:34AM +1100, NeilBrown wrote: > > These async_XX functions are called from md/raid5 in an atomic > section, between get_cpu() and put_cpu(), so they must not sleep. > So use GFP_NOWAIT rather than GFP_IO. > > Dan Williams writes: Longer term async_tx needs to be merged into md > directly as we can allocate this unmap data statically per-stripe > rather than per request. > > Fixed: 7476bd79fc01 ("async_pq: convert to dmaengine_unmap_data") > Cc: stable@vger.kernel.org (v3.13+) > Reported-and-tested-by: Stanislav Samsonov <slava@annapurnalabs.com> > Acked-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: NeilBrown <neilb@suse.com> > --- > > Thanks for taking this Vinod. > It is currently in linux-next from my md tree, but I've just de-staged > it so the next linux-next won't have it from me. Okay this is not in dmaengine. But since we all agreed, I have picked and will send to Linus later today. If anyone has any objections please speak up Thanks -- ~Vinod [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-01-07 5:39 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-22 11:58 raid5 async_xor: sleep in atomic Stanislav Samsonov 2015-12-23 2:34 ` NeilBrown 2015-12-23 17:35 ` Dan Williams 2015-12-23 22:39 ` NeilBrown 2015-12-23 22:46 ` Dan Williams 2015-12-28 8:43 ` Stanislav Samsonov 2016-01-04 1:33 ` NeilBrown 2016-01-04 17:28 ` Dan Williams 2016-01-06 9:08 ` Vinod Koul 2016-01-07 0:02 ` [PATCH] async_tx: use GFP_NOWAIT rather than GFP_IO NeilBrown 2016-01-07 5:39 ` Vinod Koul
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.