diff options
author | Farhan Ali <alifm@linux.ibm.com> | 2019-04-03 14:22:27 -0400 |
---|---|---|
committer | Alex Williamson <alex.williamson@redhat.com> | 2019-04-23 13:30:46 -0400 |
commit | 41be3e2618174fdf3361e49e64f2bf530f40c6b0 (patch) | |
tree | 674ff4df20436dfdec2ced3f1cce980a5b07dc55 | |
parent | a88a7b3eb076ade6205176915fd2ee73a60f4a32 (diff) |
vfio: Fix WARNING "do not call blocking ops when !TASK_RUNNING"
vfio_dev_present() which is the condition to
wait_event_interruptible_timeout(), will call vfio_group_get_device
and try to acquire the mutex group->device_lock.
wait_event_interruptible_timeout() will set the state of the current
task to TASK_INTERRUPTIBLE, before doing the condition check. This
means that we will try to acquire the mutex while already in a
sleeping state. The scheduler warns us by giving the following
warning:
[ 4050.264464] ------------[ cut here ]------------
[ 4050.264508] do not call blocking ops when !TASK_RUNNING; state=1 set at [<00000000b33c00e2>] prepare_to_wait_event+0x14a/0x188
[ 4050.264529] WARNING: CPU: 12 PID: 35924 at kernel/sched/core.c:6112 __might_sleep+0x76/0x90
....
4050.264756] Call Trace:
[ 4050.264765] ([<000000000017bbaa>] __might_sleep+0x72/0x90)
[ 4050.264774] [<0000000000b97edc>] __mutex_lock+0x44/0x8c0
[ 4050.264782] [<0000000000b9878a>] mutex_lock_nested+0x32/0x40
[ 4050.264793] [<000003ff800d7abe>] vfio_group_get_device+0x36/0xa8 [vfio]
[ 4050.264803] [<000003ff800d87c0>] vfio_del_group_dev+0x238/0x378 [vfio]
[ 4050.264813] [<000003ff8015f67c>] mdev_remove+0x3c/0x68 [mdev]
[ 4050.264825] [<00000000008e01b0>] device_release_driver_internal+0x168/0x268
[ 4050.264834] [<00000000008de692>] bus_remove_device+0x162/0x190
[ 4050.264843] [<00000000008daf42>] device_del+0x1e2/0x368
[ 4050.264851] [<00000000008db12c>] device_unregister+0x64/0x88
[ 4050.264862] [<000003ff8015ed84>] mdev_device_remove+0xec/0x130 [mdev]
[ 4050.264872] [<000003ff8015f074>] remove_store+0x6c/0xa8 [mdev]
[ 4050.264881] [<000000000046f494>] kernfs_fop_write+0x14c/0x1f8
[ 4050.264890] [<00000000003c1530>] __vfs_write+0x38/0x1a8
[ 4050.264899] [<00000000003c187c>] vfs_write+0xb4/0x198
[ 4050.264908] [<00000000003c1af2>] ksys_write+0x5a/0xb0
[ 4050.264916] [<0000000000b9e270>] system_call+0xdc/0x2d8
[ 4050.264925] 4 locks held by sh/35924:
[ 4050.264933] #0: 000000001ef90325 (sb_writers#4){.+.+}, at: vfs_write+0x9e/0x198
[ 4050.264948] #1: 000000005c1ab0b3 (&of->mutex){+.+.}, at: kernfs_fop_write+0x1cc/0x1f8
[ 4050.264963] #2: 0000000034831ab8 (kn->count#297){++++}, at: kernfs_remove_self+0x12e/0x150
[ 4050.264979] #3: 00000000e152484f (&dev->mutex){....}, at: device_release_driver_internal+0x5c/0x268
[ 4050.264993] Last Breaking-Event-Address:
[ 4050.265002] [<000000000017bbaa>] __might_sleep+0x72/0x90
[ 4050.265010] irq event stamp: 7039
[ 4050.265020] hardirqs last enabled at (7047): [<00000000001cee7a>] console_unlock+0x6d2/0x740
[ 4050.265029] hardirqs last disabled at (7054): [<00000000001ce87e>] console_unlock+0xd6/0x740
[ 4050.265040] softirqs last enabled at (6416): [<0000000000b8fe26>] __udelay+0xb6/0x100
[ 4050.265049] softirqs last disabled at (6415): [<0000000000b8fe06>] __udelay+0x96/0x100
[ 4050.265057] ---[ end trace d04a07d39d99a9f9 ]---
Let's fix this as described in the article
https://lwn.net/Articles/628628/.
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
[remove now redundant vfio_dev_present()]
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
-rw-r--r-- | drivers/vfio/vfio.c | 30 |
1 files changed, 10 insertions, 20 deletions
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 7fb68968097a..82fcf07fa9ea 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c | |||
@@ -34,6 +34,7 @@ | |||
34 | #include <linux/uaccess.h> | 34 | #include <linux/uaccess.h> |
35 | #include <linux/vfio.h> | 35 | #include <linux/vfio.h> |
36 | #include <linux/wait.h> | 36 | #include <linux/wait.h> |
37 | #include <linux/sched/signal.h> | ||
37 | 38 | ||
38 | #define DRIVER_VERSION "0.3" | 39 | #define DRIVER_VERSION "0.3" |
39 | #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>" | 40 | #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>" |
@@ -901,30 +902,17 @@ void *vfio_device_data(struct vfio_device *device) | |||
901 | } | 902 | } |
902 | EXPORT_SYMBOL_GPL(vfio_device_data); | 903 | EXPORT_SYMBOL_GPL(vfio_device_data); |
903 | 904 | ||
904 | /* Given a referenced group, check if it contains the device */ | ||
905 | static bool vfio_dev_present(struct vfio_group *group, struct device *dev) | ||
906 | { | ||
907 | struct vfio_device *device; | ||
908 | |||
909 | device = vfio_group_get_device(group, dev); | ||
910 | if (!device) | ||
911 | return false; | ||
912 | |||
913 | vfio_device_put(device); | ||
914 | return true; | ||
915 | } | ||
916 | |||
917 | /* | 905 | /* |
918 | * Decrement the device reference count and wait for the device to be | 906 | * Decrement the device reference count and wait for the device to be |
919 | * removed. Open file descriptors for the device... */ | 907 | * removed. Open file descriptors for the device... */ |
920 | void *vfio_del_group_dev(struct device *dev) | 908 | void *vfio_del_group_dev(struct device *dev) |
921 | { | 909 | { |
910 | DEFINE_WAIT_FUNC(wait, woken_wake_function); | ||
922 | struct vfio_device *device = dev_get_drvdata(dev); | 911 | struct vfio_device *device = dev_get_drvdata(dev); |
923 | struct vfio_group *group = device->group; | 912 | struct vfio_group *group = device->group; |
924 | void *device_data = device->device_data; | 913 | void *device_data = device->device_data; |
925 | struct vfio_unbound_dev *unbound; | 914 | struct vfio_unbound_dev *unbound; |
926 | unsigned int i = 0; | 915 | unsigned int i = 0; |
927 | long ret; | ||
928 | bool interrupted = false; | 916 | bool interrupted = false; |
929 | 917 | ||
930 | /* | 918 | /* |
@@ -961,6 +949,8 @@ void *vfio_del_group_dev(struct device *dev) | |||
961 | * interval with counter to allow the driver to take escalating | 949 | * interval with counter to allow the driver to take escalating |
962 | * measures to release the device if it has the ability to do so. | 950 | * measures to release the device if it has the ability to do so. |
963 | */ | 951 | */ |
952 | add_wait_queue(&vfio.release_q, &wait); | ||
953 | |||
964 | do { | 954 | do { |
965 | device = vfio_group_get_device(group, dev); | 955 | device = vfio_group_get_device(group, dev); |
966 | if (!device) | 956 | if (!device) |
@@ -972,12 +962,10 @@ void *vfio_del_group_dev(struct device *dev) | |||
972 | vfio_device_put(device); | 962 | vfio_device_put(device); |
973 | 963 | ||
974 | if (interrupted) { | 964 | if (interrupted) { |
975 | ret = wait_event_timeout(vfio.release_q, | 965 | wait_woken(&wait, TASK_UNINTERRUPTIBLE, HZ * 10); |
976 | !vfio_dev_present(group, dev), HZ * 10); | ||
977 | } else { | 966 | } else { |
978 | ret = wait_event_interruptible_timeout(vfio.release_q, | 967 | wait_woken(&wait, TASK_INTERRUPTIBLE, HZ * 10); |
979 | !vfio_dev_present(group, dev), HZ * 10); | 968 | if (signal_pending(current)) { |
980 | if (ret == -ERESTARTSYS) { | ||
981 | interrupted = true; | 969 | interrupted = true; |
982 | dev_warn(dev, | 970 | dev_warn(dev, |
983 | "Device is currently in use, task" | 971 | "Device is currently in use, task" |
@@ -986,8 +974,10 @@ void *vfio_del_group_dev(struct device *dev) | |||
986 | current->comm, task_pid_nr(current)); | 974 | current->comm, task_pid_nr(current)); |
987 | } | 975 | } |
988 | } | 976 | } |
989 | } while (ret <= 0); | ||
990 | 977 | ||
978 | } while (1); | ||
979 | |||
980 | remove_wait_queue(&vfio.release_q, &wait); | ||
991 | /* | 981 | /* |
992 | * In order to support multiple devices per group, devices can be | 982 | * In order to support multiple devices per group, devices can be |
993 | * plucked from the group while other devices in the group are still | 983 | * plucked from the group while other devices in the group are still |