| Message ID | 149194856068.47527.14334115580831385512.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2017-04-11 at 15:09 -0700, Dave Jiang wrote: > The following warning results from holding a lane spinlock, > preempt_disable(), or the btt map spinlock and then trying to take the > reconfig_mutex to walk the poison list and potentially add new > entries. > > BUG: sleeping function called from invalid context at > kernel/locking/mutex. > c:747 > in_atomic(): 1, irqs_disabled(): 0, pid: 17159, name: dd > [..] > Call Trace: > dump_stack+0x85/0xc8 > ___might_sleep+0x184/0x250 > __might_sleep+0x4a/0x90 > __mutex_lock+0x58/0x9b0 > ? nvdimm_bus_lock+0x21/0x30 [libnvdimm] > ? __nvdimm_bus_badblocks_clear+0x2f/0x60 [libnvdimm] > ? acpi_nfit_forget_poison+0x79/0x80 [nfit] > ? _raw_spin_unlock+0x27/0x40 > mutex_lock_nested+0x1b/0x20 > nvdimm_bus_lock+0x21/0x30 [libnvdimm] > nvdimm_forget_poison+0x25/0x50 [libnvdimm] > nvdimm_clear_poison+0x106/0x140 [libnvdimm] > nsio_rw_bytes+0x164/0x270 [libnvdimm] > btt_write_pg+0x1de/0x3e0 [nd_btt] > ? blk_queue_enter+0x30/0x290 > btt_make_request+0x11a/0x310 [nd_btt] > ? blk_queue_enter+0xb7/0x290 > ? blk_queue_enter+0x30/0x290 > generic_make_request+0x118/0x3b0 > > Two things are done to address this issue. First, we introduce a > spinlock to > protect the poison list. This allows us to not having to acquire the > reconfig_mutex for touching the poison list. Second, we allocate the > poison > entries using GFP_NOWAIT and avoid the sleep with GFP_KERNEL. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/nvdimm/bus.c | 12 +++++++----- > drivers/nvdimm/core.c | 25 ++++++++----------------- > drivers/nvdimm/nd-core.h | 1 + > include/linux/libnvdimm.h | 2 -- > 4 files changed, 16 insertions(+), 24 deletions(-) > > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > index 5ad2e59..fe41146 100644 > --- a/drivers/nvdimm/bus.c > +++ b/drivers/nvdimm/bus.c > @@ -296,6 +296,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct > device *parent, > init_waitqueue_head(&nvdimm_bus->probe_wait); > nvdimm_bus->id = ida_simple_get(&nd_ida, 0, 0, GFP_KERNEL); > mutex_init(&nvdimm_bus->reconfig_mutex); > + spin_lock_init(&nvdimm_bus->poison_lock); > if (nvdimm_bus->id < 0) { > kfree(nvdimm_bus); > return NULL; > @@ -342,8 +343,9 @@ static int child_unregister(struct device *dev, > void *data) > return 0; > } > > -static void free_poison_list(struct list_head *poison_list) > +static void free_poison_list(struct nvdimm_bus *nvdimm_bus) > { > + struct list_head *poison_list = &nvdimm_bus->poison_list; > struct nd_poison *pl, *next; This change isn't needed anymore in v2 right? (and the caller below) > > list_for_each_entry_safe(pl, next, poison_list, list) { > @@ -364,9 +366,9 @@ static int nd_bus_remove(struct device *dev) > nd_synchronize(); > device_for_each_child(&nvdimm_bus->dev, NULL, > child_unregister); > > - nvdimm_bus_lock(&nvdimm_bus->dev); > - free_poison_list(&nvdimm_bus->poison_list); > - nvdimm_bus_unlock(&nvdimm_bus->dev); > + spin_lock(&nvdimm_bus->poison_lock); > + free_poison_list(nvdimm_bus); > + spin_unlock(&nvdimm_bus->poison_lock); > > nvdimm_bus_destroy_ndctl(nvdimm_bus); > > @@ -990,7 +992,7 @@ static int __nd_ioctl(struct nvdimm_bus > *nvdimm_bus, struct nvdimm *nvdimm, > > if (clear_err->cleared) { > /* clearing the poison list we keep track of > */ > - __nvdimm_forget_poison(nvdimm_bus, clear_err- > >address, > + nvdimm_forget_poison(nvdimm_bus, clear_err- > >address, > clear_err->cleared); > > /* now sync the badblocks lists */ > diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c > index 40a3da0..9567b08 100644 > --- a/drivers/nvdimm/core.c > +++ b/drivers/nvdimm/core.c > @@ -539,7 +539,7 @@ static int bus_add_poison(struct nvdimm_bus > *nvdimm_bus, u64 addr, u64 length) > struct nd_poison *pl; > > if (list_empty(&nvdimm_bus->poison_list)) > - return add_poison(nvdimm_bus, addr, length, > GFP_KERNEL); > + return add_poison(nvdimm_bus, addr, length, > GFP_NOWAIT); > > /* > * There is a chance this is a duplicate, check for those > first. > @@ -559,30 +559,29 @@ static int bus_add_poison(struct nvdimm_bus > *nvdimm_bus, u64 addr, u64 length) > * as any overlapping ranges will get resolved when the list > is consumed > * and converted to badblocks > */ > - return add_poison(nvdimm_bus, addr, length, GFP_KERNEL); > + return add_poison(nvdimm_bus, addr, length, GFP_NOWAIT); > } > > int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, > u64 length) > { > int rc; > > - nvdimm_bus_lock(&nvdimm_bus->dev); > + spin_lock(&nvdimm_bus->poison_lock); > rc = bus_add_poison(nvdimm_bus, addr, length); > - nvdimm_bus_unlock(&nvdimm_bus->dev); > + spin_unlock(&nvdimm_bus->poison_lock); > > return rc; > } > EXPORT_SYMBOL_GPL(nvdimm_bus_add_poison); > > -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, > phys_addr_t start, > +void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t > start, > unsigned int len) > { > struct list_head *poison_list = &nvdimm_bus->poison_list; > u64 clr_end = start + len - 1; > struct nd_poison *pl, *next; > > - lockdep_assert_held(&nvdimm_bus->reconfig_mutex); > - > + spin_lock(&nvdimm_bus->poison_lock); > WARN_ON_ONCE(list_empty(poison_list)); > > /* > @@ -629,21 +628,13 @@ void __nvdimm_forget_poison(struct nvdimm_bus > *nvdimm_bus, phys_addr_t start, > u64 new_len = pl_end - new_start + 1; > > /* Add new entry covering the right half */ > - add_poison(nvdimm_bus, new_start, new_len, > GFP_NOIO); > + add_poison(nvdimm_bus, new_start, new_len, > GFP_NOWAIT); > /* Adjust this entry to cover the left half > */ > pl->length = start - pl->start; > continue; > } > } > -} > -EXPORT_SYMBOL_GPL(__nvdimm_forget_poison); > - > -void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, > - phys_addr_t start, unsigned int len) > -{ > - nvdimm_bus_lock(&nvdimm_bus->dev); > - __nvdimm_forget_poison(nvdimm_bus, start, len); > - nvdimm_bus_unlock(&nvdimm_bus->dev); > + spin_unlock(&nvdimm_bus->poison_lock); > } > EXPORT_SYMBOL_GPL(nvdimm_forget_poison); > > diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h > index 8623e57..4c4bd20 100644 > --- a/drivers/nvdimm/nd-core.h > +++ b/drivers/nvdimm/nd-core.h > @@ -32,6 +32,7 @@ struct nvdimm_bus { > struct list_head poison_list; > struct list_head mapping_list; > struct mutex reconfig_mutex; > + spinlock_t poison_lock; > }; > > struct nvdimm { > diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h > index 1c609e8..98b2076 100644 > --- a/include/linux/libnvdimm.h > +++ b/include/linux/libnvdimm.h > @@ -122,8 +122,6 @@ static inline struct nd_blk_region_desc > *to_blk_region_desc( > int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, > u64 length); > void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, > phys_addr_t start, unsigned int len); > -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, > - phys_addr_t start, unsigned int len); > struct nvdimm_bus *nvdimm_bus_register(struct device *parent, > struct nvdimm_bus_descriptor *nfit_desc); > void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_bus); >
On 04/11/2017 03:13 PM, Verma, Vishal L wrote: > On Tue, 2017-04-11 at 15:09 -0700, Dave Jiang wrote: >> The following warning results from holding a lane spinlock, >> preempt_disable(), or the btt map spinlock and then trying to take the >> reconfig_mutex to walk the poison list and potentially add new >> entries. >> >> BUG: sleeping function called from invalid context at >> kernel/locking/mutex. >> c:747 >> in_atomic(): 1, irqs_disabled(): 0, pid: 17159, name: dd >> [..] >> Call Trace: >> dump_stack+0x85/0xc8 >> ___might_sleep+0x184/0x250 >> __might_sleep+0x4a/0x90 >> __mutex_lock+0x58/0x9b0 >> ? nvdimm_bus_lock+0x21/0x30 [libnvdimm] >> ? __nvdimm_bus_badblocks_clear+0x2f/0x60 [libnvdimm] >> ? acpi_nfit_forget_poison+0x79/0x80 [nfit] >> ? _raw_spin_unlock+0x27/0x40 >> mutex_lock_nested+0x1b/0x20 >> nvdimm_bus_lock+0x21/0x30 [libnvdimm] >> nvdimm_forget_poison+0x25/0x50 [libnvdimm] >> nvdimm_clear_poison+0x106/0x140 [libnvdimm] >> nsio_rw_bytes+0x164/0x270 [libnvdimm] >> btt_write_pg+0x1de/0x3e0 [nd_btt] >> ? blk_queue_enter+0x30/0x290 >> btt_make_request+0x11a/0x310 [nd_btt] >> ? blk_queue_enter+0xb7/0x290 >> ? blk_queue_enter+0x30/0x290 >> generic_make_request+0x118/0x3b0 >> >> Two things are done to address this issue. First, we introduce a >> spinlock to >> protect the poison list. This allows us to not having to acquire the >> reconfig_mutex for touching the poison list. Second, we allocate the >> poison >> entries using GFP_NOWAIT and avoid the sleep with GFP_KERNEL. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/nvdimm/bus.c | 12 +++++++----- >> drivers/nvdimm/core.c | 25 ++++++++----------------- >> drivers/nvdimm/nd-core.h | 1 + >> include/linux/libnvdimm.h | 2 -- >> 4 files changed, 16 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c >> index 5ad2e59..fe41146 100644 >> --- a/drivers/nvdimm/bus.c >> +++ b/drivers/nvdimm/bus.c >> @@ -296,6 +296,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct >> device *parent, >> init_waitqueue_head(&nvdimm_bus->probe_wait); >> nvdimm_bus->id = ida_simple_get(&nd_ida, 0, 0, GFP_KERNEL); >> mutex_init(&nvdimm_bus->reconfig_mutex); >> + spin_lock_init(&nvdimm_bus->poison_lock); >> if (nvdimm_bus->id < 0) { >> kfree(nvdimm_bus); >> return NULL; >> @@ -342,8 +343,9 @@ static int child_unregister(struct device *dev, >> void *data) >> return 0; >> } >> >> -static void free_poison_list(struct list_head *poison_list) >> +static void free_poison_list(struct nvdimm_bus *nvdimm_bus) >> { >> + struct list_head *poison_list = &nvdimm_bus->poison_list; >> struct nd_poison *pl, *next; > > This change isn't needed anymore in v2 right? (and the caller below) Good catch! > >> >> list_for_each_entry_safe(pl, next, poison_list, list) { >> @@ -364,9 +366,9 @@ static int nd_bus_remove(struct device *dev) >> nd_synchronize(); >> device_for_each_child(&nvdimm_bus->dev, NULL, >> child_unregister); >> >> - nvdimm_bus_lock(&nvdimm_bus->dev); >> - free_poison_list(&nvdimm_bus->poison_list); >> - nvdimm_bus_unlock(&nvdimm_bus->dev); >> + spin_lock(&nvdimm_bus->poison_lock); >> + free_poison_list(nvdimm_bus); >> + spin_unlock(&nvdimm_bus->poison_lock); >> >> nvdimm_bus_destroy_ndctl(nvdimm_bus); >> >> @@ -990,7 +992,7 @@ static int __nd_ioctl(struct nvdimm_bus >> *nvdimm_bus, struct nvdimm *nvdimm, >> >> if (clear_err->cleared) { >> /* clearing the poison list we keep track of >> */ >> - __nvdimm_forget_poison(nvdimm_bus, clear_err- >>> address, >> + nvdimm_forget_poison(nvdimm_bus, clear_err- >>> address, >> clear_err->cleared); >> >> /* now sync the badblocks lists */ >> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c >> index 40a3da0..9567b08 100644 >> --- a/drivers/nvdimm/core.c >> +++ b/drivers/nvdimm/core.c >> @@ -539,7 +539,7 @@ static int bus_add_poison(struct nvdimm_bus >> *nvdimm_bus, u64 addr, u64 length) >> struct nd_poison *pl; >> >> if (list_empty(&nvdimm_bus->poison_list)) >> - return add_poison(nvdimm_bus, addr, length, >> GFP_KERNEL); >> + return add_poison(nvdimm_bus, addr, length, >> GFP_NOWAIT); >> >> /* >> * There is a chance this is a duplicate, check for those >> first. >> @@ -559,30 +559,29 @@ static int bus_add_poison(struct nvdimm_bus >> *nvdimm_bus, u64 addr, u64 length) >> * as any overlapping ranges will get resolved when the list >> is consumed >> * and converted to badblocks >> */ >> - return add_poison(nvdimm_bus, addr, length, GFP_KERNEL); >> + return add_poison(nvdimm_bus, addr, length, GFP_NOWAIT); >> } >> >> int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, >> u64 length) >> { >> int rc; >> >> - nvdimm_bus_lock(&nvdimm_bus->dev); >> + spin_lock(&nvdimm_bus->poison_lock); >> rc = bus_add_poison(nvdimm_bus, addr, length); >> - nvdimm_bus_unlock(&nvdimm_bus->dev); >> + spin_unlock(&nvdimm_bus->poison_lock); >> >> return rc; >> } >> EXPORT_SYMBOL_GPL(nvdimm_bus_add_poison); >> >> -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, >> phys_addr_t start, >> +void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t >> start, >> unsigned int len) >> { >> struct list_head *poison_list = &nvdimm_bus->poison_list; >> u64 clr_end = start + len - 1; >> struct nd_poison *pl, *next; >> >> - lockdep_assert_held(&nvdimm_bus->reconfig_mutex); >> - >> + spin_lock(&nvdimm_bus->poison_lock); >> WARN_ON_ONCE(list_empty(poison_list)); >> >> /* >> @@ -629,21 +628,13 @@ void __nvdimm_forget_poison(struct nvdimm_bus >> *nvdimm_bus, phys_addr_t start, >> u64 new_len = pl_end - new_start + 1; >> >> /* Add new entry covering the right half */ >> - add_poison(nvdimm_bus, new_start, new_len, >> GFP_NOIO); >> + add_poison(nvdimm_bus, new_start, new_len, >> GFP_NOWAIT); >> /* Adjust this entry to cover the left half >> */ >> pl->length = start - pl->start; >> continue; >> } >> } >> -} >> -EXPORT_SYMBOL_GPL(__nvdimm_forget_poison); >> - >> -void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, >> - phys_addr_t start, unsigned int len) >> -{ >> - nvdimm_bus_lock(&nvdimm_bus->dev); >> - __nvdimm_forget_poison(nvdimm_bus, start, len); >> - nvdimm_bus_unlock(&nvdimm_bus->dev); >> + spin_unlock(&nvdimm_bus->poison_lock); >> } >> EXPORT_SYMBOL_GPL(nvdimm_forget_poison); >> >> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h >> index 8623e57..4c4bd20 100644 >> --- a/drivers/nvdimm/nd-core.h >> +++ b/drivers/nvdimm/nd-core.h >> @@ -32,6 +32,7 @@ struct nvdimm_bus { >> struct list_head poison_list; >> struct list_head mapping_list; >> struct mutex reconfig_mutex; >> + spinlock_t poison_lock; >> }; >> >> struct nvdimm { >> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h >> index 1c609e8..98b2076 100644 >> --- a/include/linux/libnvdimm.h >> +++ b/include/linux/libnvdimm.h >> @@ -122,8 +122,6 @@ static inline struct nd_blk_region_desc >> *to_blk_region_desc( >> int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, >> u64 length); >> void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, >> phys_addr_t start, unsigned int len); >> -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, >> - phys_addr_t start, unsigned int len); >> struct nvdimm_bus *nvdimm_bus_register(struct device *parent, >> struct nvdimm_bus_descriptor *nfit_desc); >> void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_bus);
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index 5ad2e59..fe41146 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -296,6 +296,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent, init_waitqueue_head(&nvdimm_bus->probe_wait); nvdimm_bus->id = ida_simple_get(&nd_ida, 0, 0, GFP_KERNEL); mutex_init(&nvdimm_bus->reconfig_mutex); + spin_lock_init(&nvdimm_bus->poison_lock); if (nvdimm_bus->id < 0) { kfree(nvdimm_bus); return NULL; @@ -342,8 +343,9 @@ static int child_unregister(struct device *dev, void *data) return 0; } -static void free_poison_list(struct list_head *poison_list) +static void free_poison_list(struct nvdimm_bus *nvdimm_bus) { + struct list_head *poison_list = &nvdimm_bus->poison_list; struct nd_poison *pl, *next; list_for_each_entry_safe(pl, next, poison_list, list) { @@ -364,9 +366,9 @@ static int nd_bus_remove(struct device *dev) nd_synchronize(); device_for_each_child(&nvdimm_bus->dev, NULL, child_unregister); - nvdimm_bus_lock(&nvdimm_bus->dev); - free_poison_list(&nvdimm_bus->poison_list); - nvdimm_bus_unlock(&nvdimm_bus->dev); + spin_lock(&nvdimm_bus->poison_lock); + free_poison_list(nvdimm_bus); + spin_unlock(&nvdimm_bus->poison_lock); nvdimm_bus_destroy_ndctl(nvdimm_bus); @@ -990,7 +992,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, if (clear_err->cleared) { /* clearing the poison list we keep track of */ - __nvdimm_forget_poison(nvdimm_bus, clear_err->address, + nvdimm_forget_poison(nvdimm_bus, clear_err->address, clear_err->cleared); /* now sync the badblocks lists */ diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c index 40a3da0..9567b08 100644 --- a/drivers/nvdimm/core.c +++ b/drivers/nvdimm/core.c @@ -539,7 +539,7 @@ static int bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length) struct nd_poison *pl; if (list_empty(&nvdimm_bus->poison_list)) - return add_poison(nvdimm_bus, addr, length, GFP_KERNEL); + return add_poison(nvdimm_bus, addr, length, GFP_NOWAIT); /* * There is a chance this is a duplicate, check for those first. @@ -559,30 +559,29 @@ static int bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length) * as any overlapping ranges will get resolved when the list is consumed * and converted to badblocks */ - return add_poison(nvdimm_bus, addr, length, GFP_KERNEL); + return add_poison(nvdimm_bus, addr, length, GFP_NOWAIT); } int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length) { int rc; - nvdimm_bus_lock(&nvdimm_bus->dev); + spin_lock(&nvdimm_bus->poison_lock); rc = bus_add_poison(nvdimm_bus, addr, length); - nvdimm_bus_unlock(&nvdimm_bus->dev); + spin_unlock(&nvdimm_bus->poison_lock); return rc; } EXPORT_SYMBOL_GPL(nvdimm_bus_add_poison); -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t start, +void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t start, unsigned int len) { struct list_head *poison_list = &nvdimm_bus->poison_list; u64 clr_end = start + len - 1; struct nd_poison *pl, *next; - lockdep_assert_held(&nvdimm_bus->reconfig_mutex); - + spin_lock(&nvdimm_bus->poison_lock); WARN_ON_ONCE(list_empty(poison_list)); /* @@ -629,21 +628,13 @@ void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t start, u64 new_len = pl_end - new_start + 1; /* Add new entry covering the right half */ - add_poison(nvdimm_bus, new_start, new_len, GFP_NOIO); + add_poison(nvdimm_bus, new_start, new_len, GFP_NOWAIT); /* Adjust this entry to cover the left half */ pl->length = start - pl->start; continue; } } -} -EXPORT_SYMBOL_GPL(__nvdimm_forget_poison); - -void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, - phys_addr_t start, unsigned int len) -{ - nvdimm_bus_lock(&nvdimm_bus->dev); - __nvdimm_forget_poison(nvdimm_bus, start, len); - nvdimm_bus_unlock(&nvdimm_bus->dev); + spin_unlock(&nvdimm_bus->poison_lock); } EXPORT_SYMBOL_GPL(nvdimm_forget_poison); diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h index 8623e57..4c4bd20 100644 --- a/drivers/nvdimm/nd-core.h +++ b/drivers/nvdimm/nd-core.h @@ -32,6 +32,7 @@ struct nvdimm_bus { struct list_head poison_list; struct list_head mapping_list; struct mutex reconfig_mutex; + spinlock_t poison_lock; }; struct nvdimm { diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h index 1c609e8..98b2076 100644 --- a/include/linux/libnvdimm.h +++ b/include/linux/libnvdimm.h @@ -122,8 +122,6 @@ static inline struct nd_blk_region_desc *to_blk_region_desc( int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length); void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t start, unsigned int len); -void __nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, - phys_addr_t start, unsigned int len); struct nvdimm_bus *nvdimm_bus_register(struct device *parent, struct nvdimm_bus_descriptor *nfit_desc); void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_bus);
The following warning results from holding a lane spinlock, preempt_disable(), or the btt map spinlock and then trying to take the reconfig_mutex to walk the poison list and potentially add new entries. BUG: sleeping function called from invalid context at kernel/locking/mutex. c:747 in_atomic(): 1, irqs_disabled(): 0, pid: 17159, name: dd [..] Call Trace: dump_stack+0x85/0xc8 ___might_sleep+0x184/0x250 __might_sleep+0x4a/0x90 __mutex_lock+0x58/0x9b0 ? nvdimm_bus_lock+0x21/0x30 [libnvdimm] ? __nvdimm_bus_badblocks_clear+0x2f/0x60 [libnvdimm] ? acpi_nfit_forget_poison+0x79/0x80 [nfit] ? _raw_spin_unlock+0x27/0x40 mutex_lock_nested+0x1b/0x20 nvdimm_bus_lock+0x21/0x30 [libnvdimm] nvdimm_forget_poison+0x25/0x50 [libnvdimm] nvdimm_clear_poison+0x106/0x140 [libnvdimm] nsio_rw_bytes+0x164/0x270 [libnvdimm] btt_write_pg+0x1de/0x3e0 [nd_btt] ? blk_queue_enter+0x30/0x290 btt_make_request+0x11a/0x310 [nd_btt] ? blk_queue_enter+0xb7/0x290 ? blk_queue_enter+0x30/0x290 generic_make_request+0x118/0x3b0 Two things are done to address this issue. First, we introduce a spinlock to protect the poison list. This allows us to not having to acquire the reconfig_mutex for touching the poison list. Second, we allocate the poison entries using GFP_NOWAIT and avoid the sleep with GFP_KERNEL. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/nvdimm/bus.c | 12 +++++++----- drivers/nvdimm/core.c | 25 ++++++++----------------- drivers/nvdimm/nd-core.h | 1 + include/linux/libnvdimm.h | 2 -- 4 files changed, 16 insertions(+), 24 deletions(-)