Skip to content

Commit

Permalink
netdevsim: disable devlink reload when resources are being used
Browse files Browse the repository at this point in the history 
devlink reload destroys resources and allocates resources again.
So, when devices and ports resources are being used, devlink reload
function should not be executed. In order to avoid this race, a new
lock is added and new_port() and del_port() call devlink_reload_disable()
and devlink_reload_enable().

Thread0                      Thread1
{new/del}_port()             {new/del}_port()
devlink_reload_disable()
                             devlink_reload_disable()
devlink_reload_enable()      //here
                             devlink_reload_enable()

Before Thread1's devlink_reload_enable(), the devlink is already allowed
to execute reload because Thread0 allows it. devlink reload disable/enable
variable type is bool. So the above case would exist.
So, disable/enable should be executed atomically.
In order to do that, a new lock is used.

Test commands:
    modprobe netdevsim
    echo 1 > /sys/bus/netdevsim/new_device

    while :
    do
        echo 1 > /sys/devices/netdevsim1/new_port &
        echo 1 > /sys/devices/netdevsim1/del_port &
        devlink dev reload netdevsim/netdevsim1 &
    done

Splat looks like:
[ 1067.313531][ T1480] kernel BUG at lib/list_debug.c:53!
[ 1067.314519][ T1480] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 1067.315948][ T1480] CPU: 3 PID: 1480 Comm: bash Tainted: G        W         5.5.0-rc6+ torvalds#318
[ 1067.326082][ T1480] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 1067.356308][ T1480] RIP: 0010:__list_del_entry_valid+0xe6/0x150
[ 1067.357006][ T1480] Code: 89 ea 48 c7 c7 a0 64 1e 9f e8 7f 5b 4d ff 0f 0b 48 c7 c7 00 65 1e 9f e8 71 5b 4d ff 4
[ 1067.395359][ T1480] RSP: 0018:ffff8880a316fb58 EFLAGS: 00010282
[ 1067.396016][ T1480] RAX: 0000000000000054 RBX: ffff8880c0e76718 RCX: 0000000000000000
[ 1067.402370][ T1480] RDX: 0000000000000054 RSI: 0000000000000008 RDI: ffffed101462df61
[ 1067.430844][ T1480] RBP: ffff8880a31bfca0 R08: ffffed101b5404f9 R09: ffffed101b5404f9
[ 1067.432165][ T1480] R10: 0000000000000001 R11: ffffed101b5404f8 R12: ffff8880a316fcb0
[ 1067.433526][ T1480] R13: ffff8880a310d440 R14: ffffffffa117a7c0 R15: ffff8880c0e766c0
[ 1067.435818][ T1480] FS:  00007f001c026740(0000) GS:ffff8880da800000(0000) knlGS:0000000000000000
[ 1067.441677][ T1480] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1067.451305][ T1480] CR2: 00007f001afb7180 CR3: 00000000a3170003 CR4: 00000000000606e0
[ 1067.453416][ T1480] Call Trace:
[ 1067.453832][ T1480]  mutex_remove_waiter+0x101/0x520
[ 1067.455949][ T1480]  __mutex_lock+0xac7/0x14b0
[ 1067.456880][ T1480]  ? nsim_dev_port_add+0x50/0x150 [netdevsim]
[ 1067.458946][ T1480]  ? mutex_lock_io_nested+0x1380/0x1380
[ 1067.460614][ T1480]  ? _parse_integer+0xf0/0xf0
[ 1067.472498][ T1480]  ? kstrtouint+0x86/0x110
[ 1067.473327][ T1480]  ? nsim_dev_port_add+0x50/0x150 [netdevsim]
[ 1067.474187][ T1480]  nsim_dev_port_add+0x50/0x150 [netdevsim]
[ 1067.474980][ T1480]  new_port_store+0xc4/0xf0 [netdevsim]
[ 1067.475717][ T1480]  ? del_port_store+0xf0/0xf0 [netdevsim]
[ 1067.476478][ T1480]  ? sysfs_kf_write+0x3b/0x180
[ 1067.477106][ T1480]  ? sysfs_file_ops+0x160/0x160
[ 1067.477744][ T1480]  kernfs_fop_write+0x276/0x410
[ ... ]

Fixes: 4418f86 ("netdevsim: implement support for devlink region and snapshots")
Fixes: 75ba029 ("netdevsim: implement proper devlink reload")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
  • Loading branch information
@TaeheeYoo @intel-lab-lkp
TaeheeYoo authored and intel-lab-lkp committed Jan 27, 2020
1 parent 16e5184 commit fc070c0
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
21 changes: 20 additions & 1 deletion drivers/net/netdevsim/bus.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

static DEFINE_IDA(nsim_bus_dev_ids);
static LIST_HEAD(nsim_bus_dev_list);
static DEFINE_MUTEX(nsim_bus_dev_ops_lock);
DEFINE_MUTEX(nsim_bus_dev_ops_lock);
static bool enable;

static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
Expand Down Expand Up @@ -97,6 +97,8 @@ new_port_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
struct nsim_dev *nsim_dev = dev_get_drvdata(dev);
struct devlink *devlink;
unsigned int port_index;
int ret;

Expand All @@ -105,7 +107,14 @@ new_port_store(struct device *dev, struct device_attribute *attr,
ret = kstrtouint(buf, 0, &port_index);
if (ret)
return ret;

devlink = priv_to_devlink(nsim_dev);

mutex_lock(&nsim_bus_dev->port_ops_lock);
devlink_reload_disable(devlink);
ret = nsim_dev_port_add(nsim_bus_dev, port_index);
devlink_reload_enable(devlink);
mutex_unlock(&nsim_bus_dev->port_ops_lock);
return ret ? ret : count;
}

Expand All @@ -116,6 +125,8 @@ del_port_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
struct nsim_dev *nsim_dev = dev_get_drvdata(dev);
struct devlink *devlink;
unsigned int port_index;
int ret;

Expand All @@ -124,7 +135,14 @@ del_port_store(struct device *dev, struct device_attribute *attr,
ret = kstrtouint(buf, 0, &port_index);
if (ret)
return ret;

devlink = priv_to_devlink(nsim_dev);

mutex_lock(&nsim_bus_dev->port_ops_lock);
devlink_reload_disable(devlink);
ret = nsim_dev_port_del(nsim_bus_dev, port_index);
devlink_reload_enable(devlink);
mutex_unlock(&nsim_bus_dev->port_ops_lock);
return ret ? ret : count;
}

Expand Down Expand Up @@ -301,6 +319,7 @@ nsim_bus_dev_new(unsigned int id, unsigned int port_count)
nsim_bus_dev->dev.type = &nsim_bus_dev_type;
nsim_bus_dev->port_count = port_count;
nsim_bus_dev->initial_net = current->nsproxy->net_ns;
mutex_init(&nsim_bus_dev->port_ops_lock);

err = device_register(&nsim_bus_dev->dev);
if (err)
Expand Down
14 changes: 14 additions & 0 deletions drivers/net/netdevsim/dev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
size_t count, loff_t *ppos)
{
struct nsim_dev *nsim_dev = file->private_data;
struct nsim_bus_dev *nsim_bus_dev;
struct devlink *devlink;
void *dummy_data;
int err;
u32 id;
Expand All @@ -51,11 +53,23 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
if (!dummy_data)
return -ENOMEM;

devlink = priv_to_devlink(nsim_dev);
nsim_bus_dev = nsim_dev->nsim_bus_dev;

get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);

mutex_lock(&nsim_bus_dev_ops_lock);
mutex_lock(&nsim_bus_dev->port_ops_lock);
devlink_reload_disable(devlink);

id = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev));
err = devlink_region_snapshot_create(nsim_dev->dummy_region,
dummy_data, id, kfree);

devlink_reload_enable(devlink);
mutex_unlock(&nsim_bus_dev->port_ops_lock);
mutex_unlock(&nsim_bus_dev_ops_lock);

if (err) {
pr_err("Failed to create region snapshot\n");
kfree(dummy_data);
Expand Down
4 changes: 4 additions & 0 deletions drivers/net/netdevsim/netdevsim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ struct netdevsim {
struct nsim_ipsec ipsec;
};

extern struct mutex nsim_bus_dev_ops_lock;

struct netdevsim *
nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port);
void nsim_destroy(struct netdevsim *ns);
Expand Down Expand Up @@ -240,6 +242,8 @@ struct nsim_bus_dev {
*/
unsigned int num_vfs;
struct nsim_vf_config *vfconfigs;
/* Lock for new_port() and del_port() to disable devlink reload */
struct mutex port_ops_lock;
bool init;
};

Expand Down

0 comments on commit fc070c0

Please sign in to comment.