Messages in this thread | | | Subject | Re: [PATCH] lightnvm: pblk: Fix two sleep-in-atomic-context bugs in pblk_line_submit_smeta_io() | From | Matias Bjørling <> | Date | Tue, 4 Sep 2018 12:11:51 +0200 |
| |
On 09/01/2018 01:53 PM, Jia-Ju Bai wrote: > The driver may sleep with holding a spinlock. > > The function call paths (from bottom to top) in Linux-4.16 are: > > [FUNC] nvm_dev_dma_alloc(GFP_KERNEL) > drivers/lightnvm/pblk-core.c, 754: > nvm_dev_dma_alloc in pblk_line_submit_smeta_io > drivers/lightnvm/pblk-core.c, 1048: > pblk_line_submit_smeta_io in pblk_line_init_bb > drivers/lightnvm/pblk-core.c, 1434: > pblk_line_init_bb in pblk_line_replace_data > drivers/lightnvm/pblk-recovery.c, 980: > pblk_line_replace_data in pblk_recov_l2p > drivers/lightnvm/pblk-recovery.c, 976: > spin_lock in pblk_recov_l2p > > [FUNC] bio_map_kern(GFP_KERNEL) > drivers/lightnvm/pblk-core.c, 762: > bio_map_kern in pblk_line_submit_smeta_io > drivers/lightnvm/pblk-core.c, 1048: > pblk_line_submit_smeta_io in pblk_line_init_bb > drivers/lightnvm/pblk-core.c, 1434: > pblk_line_init_bb in pblk_line_replace_data > drivers/lightnvm/pblk-recovery.c, 980: > pblk_line_replace_data in pblk_recov_l2p > drivers/lightnvm/pblk-recovery.c, 976: > spin_lock in pblk_recov_l2p > > To fix these bugs, GFP_KERNEL is replaced with GFP_ATOMIC. > > These bugs are found by my static analysis tool DSAC. > > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> > --- > drivers/lightnvm/pblk-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index ed9cc977c8b3..5d915c93b6cf 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -802,7 +802,7 @@ static int pblk_line_submit_smeta_io(struct pblk *pblk, struct pblk_line *line, > > memset(&rqd, 0, sizeof(struct nvm_rq)); > > - rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, > + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_ATOMIC, > &rqd.dma_meta_list); > if (!rqd.meta_list) > return -ENOMEM; > @@ -810,7 +810,7 @@ static int pblk_line_submit_smeta_io(struct pblk *pblk, struct pblk_line *line, > rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; > rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; > > - bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); > + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_ATOMIC); > if (IS_ERR(bio)) { > ret = PTR_ERR(bio); > goto free_ppa_list; >
Javier,
What do you think? I'm OK with applying this, but one could also move the allocs outside the spinlocks?
-Matias
| |