Skip to content

Commit

Permalink
libnvdimm/bus: Stop holding nvdimm_bus_list_mutex over __nd_ioctl()
Browse files Browse the repository at this point in the history
commit b70d31d upstream.

In preparation for fixing a deadlock between wait_for_bus_probe_idle()
and the nvdimm_bus_list_mutex arrange for __nd_ioctl() without
nvdimm_bus_list_mutex held. This also unifies the 'dimm' and 'bus' level
ioctls into a common nd_ioctl() preamble implementation.

Marked for -stable as it is a pre-requisite for a follow-on fix.

Cc: <stable@vger.kernel.org>
Fixes: bf9bccc ("libnvdimm: pmem label sets and namespace instantiation")
Cc: Vishal Verma <vishal.l.verma@intel.com>
Tested-by: Jane Chu <jane.chu@oracle.com>
Link: https://lore.kernel.org/r/156341209518.292348.7183897251740665198.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
djbw authored and gregkh committed Jul 31, 2019
1 parent b993a66 commit 1a547d2
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 39 deletions.
96 changes: 58 additions & 38 deletions drivers/nvdimm/bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ static void nvdimm_bus_probe_end(struct nvdimm_bus *nvdimm_bus)
{
nvdimm_bus_lock(&nvdimm_bus->dev);
if (--nvdimm_bus->probe_active == 0)
wake_up(&nvdimm_bus->probe_wait);
wake_up(&nvdimm_bus->wait);
nvdimm_bus_unlock(&nvdimm_bus->dev);
}

Expand Down Expand Up @@ -348,7 +348,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
return NULL;
INIT_LIST_HEAD(&nvdimm_bus->list);
INIT_LIST_HEAD(&nvdimm_bus->mapping_list);
init_waitqueue_head(&nvdimm_bus->probe_wait);
init_waitqueue_head(&nvdimm_bus->wait);
nvdimm_bus->id = ida_simple_get(&nd_ida, 0, 0, GFP_KERNEL);
mutex_init(&nvdimm_bus->reconfig_mutex);
badrange_init(&nvdimm_bus->badrange);
Expand Down Expand Up @@ -418,6 +418,9 @@ static int nd_bus_remove(struct device *dev)
list_del_init(&nvdimm_bus->list);
mutex_unlock(&nvdimm_bus_list_mutex);

wait_event(nvdimm_bus->wait,
atomic_read(&nvdimm_bus->ioctl_active) == 0);

nd_synchronize();
device_for_each_child(&nvdimm_bus->dev, NULL, child_unregister);

Expand Down Expand Up @@ -838,7 +841,7 @@ void wait_nvdimm_bus_probe_idle(struct device *dev)
if (nvdimm_bus->probe_active == 0)
break;
nvdimm_bus_unlock(&nvdimm_bus->dev);
wait_event(nvdimm_bus->probe_wait,
wait_event(nvdimm_bus->wait,
nvdimm_bus->probe_active == 0);
nvdimm_bus_lock(&nvdimm_bus->dev);
} while (true);
Expand Down Expand Up @@ -1068,24 +1071,10 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
return rc;
}

static long nd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
long id = (long) file->private_data;
int rc = -ENXIO, ro;
struct nvdimm_bus *nvdimm_bus;

ro = ((file->f_flags & O_ACCMODE) == O_RDONLY);
mutex_lock(&nvdimm_bus_list_mutex);
list_for_each_entry(nvdimm_bus, &nvdimm_bus_list, list) {
if (nvdimm_bus->id == id) {
rc = __nd_ioctl(nvdimm_bus, NULL, ro, cmd, arg);
break;
}
}
mutex_unlock(&nvdimm_bus_list_mutex);

return rc;
}
enum nd_ioctl_mode {
BUS_IOCTL,
DIMM_IOCTL,
};

static int match_dimm(struct device *dev, void *data)
{
Expand All @@ -1100,31 +1089,62 @@ static int match_dimm(struct device *dev, void *data)
return 0;
}

static long nvdimm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
static long nd_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
enum nd_ioctl_mode mode)

{
int rc = -ENXIO, ro;
struct nvdimm_bus *nvdimm_bus;
struct nvdimm_bus *nvdimm_bus, *found = NULL;
long id = (long) file->private_data;
struct nvdimm *nvdimm = NULL;
int rc, ro;

ro = ((file->f_flags & O_ACCMODE) == O_RDONLY);
mutex_lock(&nvdimm_bus_list_mutex);
list_for_each_entry(nvdimm_bus, &nvdimm_bus_list, list) {
struct device *dev = device_find_child(&nvdimm_bus->dev,
file->private_data, match_dimm);
struct nvdimm *nvdimm;

if (!dev)
continue;
if (mode == DIMM_IOCTL) {
struct device *dev;

dev = device_find_child(&nvdimm_bus->dev,
file->private_data, match_dimm);
if (!dev)
continue;
nvdimm = to_nvdimm(dev);
found = nvdimm_bus;
} else if (nvdimm_bus->id == id) {
found = nvdimm_bus;
}

nvdimm = to_nvdimm(dev);
rc = __nd_ioctl(nvdimm_bus, nvdimm, ro, cmd, arg);
put_device(dev);
break;
if (found) {
atomic_inc(&nvdimm_bus->ioctl_active);
break;
}
}
mutex_unlock(&nvdimm_bus_list_mutex);

if (!found)
return -ENXIO;

nvdimm_bus = found;
rc = __nd_ioctl(nvdimm_bus, nvdimm, ro, cmd, arg);

if (nvdimm)
put_device(&nvdimm->dev);
if (atomic_dec_and_test(&nvdimm_bus->ioctl_active))
wake_up(&nvdimm_bus->wait);

return rc;
}

static long bus_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
return nd_ioctl(file, cmd, arg, BUS_IOCTL);
}

static long dimm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
return nd_ioctl(file, cmd, arg, DIMM_IOCTL);
}

static int nd_open(struct inode *inode, struct file *file)
{
long minor = iminor(inode);
Expand All @@ -1136,16 +1156,16 @@ static int nd_open(struct inode *inode, struct file *file)
static const struct file_operations nvdimm_bus_fops = {
.owner = THIS_MODULE,
.open = nd_open,
.unlocked_ioctl = nd_ioctl,
.compat_ioctl = nd_ioctl,
.unlocked_ioctl = bus_ioctl,
.compat_ioctl = bus_ioctl,
.llseek = noop_llseek,
};

static const struct file_operations nvdimm_fops = {
.owner = THIS_MODULE,
.open = nd_open,
.unlocked_ioctl = nvdimm_ioctl,
.compat_ioctl = nvdimm_ioctl,
.unlocked_ioctl = dimm_ioctl,
.compat_ioctl = dimm_ioctl,
.llseek = noop_llseek,
};

Expand Down
3 changes: 2 additions & 1 deletion drivers/nvdimm/nd-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ extern int nvdimm_major;

struct nvdimm_bus {
struct nvdimm_bus_descriptor *nd_desc;
wait_queue_head_t probe_wait;
wait_queue_head_t wait;
struct list_head list;
struct device dev;
int id, probe_active;
atomic_t ioctl_active;
struct list_head mapping_list;
struct mutex reconfig_mutex;
struct badrange badrange;
Expand Down

0 comments on commit 1a547d2

Please sign in to comment.