From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, chayang <chayang@redhat.com>,
YOGANANTH SUBRAMANIAN <anantyog@in.ibm.com>,
FuXiangChun <xfu@redhat.com>, Qunfang Zhang <qzhang@redhat.com>,
Sibiao Luo <sluo@redhat.com>, Amit Shah <amit.shah@redhat.com>,
Rusty Russell <rusty@rustcorp.com.au>
Subject: [ 07/17] virtio: console: clean up port data immediately at time of unplug
Date: Mon, 12 Aug 2013 23:35:53 -0700 [thread overview]
Message-ID: <20130813063502.332957367@linuxfoundation.org> (raw)
In-Reply-To: <20130813063501.728847844@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Amit Shah <amit.shah@redhat.com>
commit ea3768b4386a8d1790f4cc9a35de4f55b92d6442 upstream.
We used to keep the port's char device structs and the /sys entries
around till the last reference to the port was dropped. This is
actually unnecessary, and resulted in buggy behaviour:
1. Open port in guest
2. Hot-unplug port
3. Hot-plug a port with the same 'name' property as the unplugged one
This resulted in hot-plug being unsuccessful, as a port with the same
name already exists (even though it was unplugged).
This behaviour resulted in a warning message like this one:
-------------------8<---------------------------------------
WARNING: at fs/sysfs/dir.c:512 sysfs_add_one+0xc9/0x130() (Not tainted)
Hardware name: KVM
sysfs: cannot create duplicate filename
'/devices/pci0000:00/0000:00:04.0/virtio0/virtio-ports/vport0p1'
Call Trace:
[<ffffffff8106b607>] ? warn_slowpath_common+0x87/0xc0
[<ffffffff8106b6f6>] ? warn_slowpath_fmt+0x46/0x50
[<ffffffff811f2319>] ? sysfs_add_one+0xc9/0x130
[<ffffffff811f23e8>] ? create_dir+0x68/0xb0
[<ffffffff811f2469>] ? sysfs_create_dir+0x39/0x50
[<ffffffff81273129>] ? kobject_add_internal+0xb9/0x260
[<ffffffff812733d8>] ? kobject_add_varg+0x38/0x60
[<ffffffff812734b4>] ? kobject_add+0x44/0x70
[<ffffffff81349de4>] ? get_device_parent+0xf4/0x1d0
[<ffffffff8134b389>] ? device_add+0xc9/0x650
-------------------8<---------------------------------------
Instead of relying on guest applications to release all references to
the ports, we should go ahead and unregister the port from all the core
layers. Any open/read calls on the port will then just return errors,
and an unplug/plug operation on the host will succeed as expected.
This also caused buggy behaviour in case of the device removal (not just
a port): when the device was removed (which means all ports on that
device are removed automatically as well), the ports with active
users would clean up only when the last references were dropped -- and
it would be too late then to be referencing char device pointers,
resulting in oopses:
-------------------8<---------------------------------------
PID: 6162 TASK: ffff8801147ad500 CPU: 0 COMMAND: "cat"
#0 [ffff88011b9d5a90] machine_kexec at ffffffff8103232b
#1 [ffff88011b9d5af0] crash_kexec at ffffffff810b9322
#2 [ffff88011b9d5bc0] oops_end at ffffffff814f4a50
#3 [ffff88011b9d5bf0] die at ffffffff8100f26b
#4 [ffff88011b9d5c20] do_general_protection at ffffffff814f45e2
#5 [ffff88011b9d5c50] general_protection at ffffffff814f3db5
[exception RIP: strlen+2]
RIP: ffffffff81272ae2 RSP: ffff88011b9d5d00 RFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff880118901c18 RCX: 0000000000000000
RDX: ffff88011799982c RSI: 00000000000000d0 RDI: 3a303030302f3030
RBP: ffff88011b9d5d38 R8: 0000000000000006 R9: ffffffffa0134500
R10: 0000000000001000 R11: 0000000000001000 R12: ffff880117a1cc10
R13: 00000000000000d0 R14: 0000000000000017 R15: ffffffff81aff700
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#6 [ffff88011b9d5d00] kobject_get_path at ffffffff8126dc5d
#7 [ffff88011b9d5d40] kobject_uevent_env at ffffffff8126e551
#8 [ffff88011b9d5dd0] kobject_uevent at ffffffff8126e9eb
#9 [ffff88011b9d5de0] device_del at ffffffff813440c7
-------------------8<---------------------------------------
So clean up when we have all the context, and all that's left to do when
the references to the port have dropped is to free up the port struct
itself.
Reported-by: chayang <chayang@redhat.com>
Reported-by: YOGANANTH SUBRAMANIAN <anantyog@in.ibm.com>
Reported-by: FuXiangChun <xfu@redhat.com>
Reported-by: Qunfang Zhang <qzhang@redhat.com>
Reported-by: Sibiao Luo <sluo@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/char/virtio_console.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1257,14 +1257,6 @@ static void remove_port(struct kref *kre
port = container_of(kref, struct port, kref);
- sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
- device_destroy(pdrvdata.class, port->dev->devt);
- cdev_del(port->cdev);
-
- kfree(port->name);
-
- debugfs_remove(port->debugfs_file);
-
kfree(port);
}
@@ -1318,6 +1310,14 @@ static void unplug_port(struct port *por
*/
port->portdev = NULL;
+ sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
+ device_destroy(pdrvdata.class, port->dev->devt);
+ cdev_del(port->cdev);
+
+ kfree(port->name);
+
+ debugfs_remove(port->debugfs_file);
+
/*
* Locks around here are not necessary - a port can't be
* opened after we removed the port struct from ports_list
next prev parent reply other threads:[~2013-08-13 6:36 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-13 6:35 [ 00/17] 3.4.58-stable review Greg Kroah-Hartman
2013-08-13 6:35 ` [ 01/17] SCSI: Dont attempt to send extended INQUIRY command if skip_vpd_pages is set Greg Kroah-Hartman
2013-08-13 6:35 ` [ 02/17] SCSI: megaraid_sas: megaraid_sas driver init fails in kdump kernel Greg Kroah-Hartman
2013-08-13 6:35 ` [ 03/17] ext4: make sure group number is bumped after a inode allocation race Greg Kroah-Hartman
2013-08-13 6:35 ` [ 04/17] hwmon: (adt7470) Fix incorrect return code check Greg Kroah-Hartman
2013-08-13 6:35 ` [ 05/17] virtio: console: fix race with port unplug and open/close Greg Kroah-Hartman
2013-08-13 6:35 ` [ 06/17] virtio: console: fix race in port_fops_open() and port unplug Greg Kroah-Hartman
2013-08-13 6:35 ` Greg Kroah-Hartman [this message]
2013-08-13 6:35 ` [ 08/17] virtio: console: fix raising SIGIO after " Greg Kroah-Hartman
2013-08-13 6:35 ` [ 09/17] virtio: console: return -ENODEV on all read operations after unplug Greg Kroah-Hartman
2013-08-13 6:35 ` [ 10/17] ext4: fix mount/remount error messages for incompatible mount options Greg Kroah-Hartman
2013-08-13 6:35 ` [ 11/17] cifs: extend the buffer length enought for sprintf() using Greg Kroah-Hartman
2013-08-13 6:35 ` [ 12/17] usb: core: dont try to reset_device() a port that got just disconnected Greg Kroah-Hartman
2013-08-13 6:35 ` [ 13/17] debugfs: debugfs_remove_recursive() must not rely on list_empty(d_subdirs) Greg Kroah-Hartman
2013-08-13 6:36 ` [ 14/17] tracing: Fix fields of struct trace_iterator that are zeroed by mistake Greg Kroah-Hartman
2013-08-13 6:36 ` [ 15/17] SCSI: nsp32: use mdelay instead of large udelay constants Greg Kroah-Hartman
2013-08-13 6:36 ` [ 16/17] vfs: d_obtain_alias() needs to use "/" as default name Greg Kroah-Hartman
2013-08-13 6:36 ` [ 17/17] perf tools: Add anonymous huge page recognition Greg Kroah-Hartman
2013-08-13 11:49 ` [ 00/17] 3.4.58-stable review Guenter Roeck
2013-08-13 17:58 ` Greg Kroah-Hartman
2013-08-13 20:19 ` Guenter Roeck
2013-08-13 20:49 ` Geert Uytterhoeven
2013-08-13 22:36 ` Guenter Roeck
2013-08-14 8:26 ` Geert Uytterhoeven
2013-08-14 10:14 ` Guenter Roeck
2013-08-15 6:36 ` Greg Kroah-Hartman
2013-08-15 7:08 ` Guenter Roeck
2013-08-15 8:40 ` Guenter Roeck
2013-08-16 0:54 ` Greg Kroah-Hartman
2013-08-15 9:07 ` Guenter Roeck
2013-08-16 0:58 ` Greg Kroah-Hartman
2013-08-16 1:07 ` Guenter Roeck
2013-08-15 14:45 ` Guenter Roeck
2013-08-16 1:22 ` Greg Kroah-Hartman
2013-08-16 1:28 ` Guenter Roeck
2013-08-16 1:38 ` Greg Kroah-Hartman
2013-08-15 15:12 ` Guenter Roeck
2013-08-16 1:18 ` Greg Kroah-Hartman
2013-08-15 6:35 ` Greg Kroah-Hartman
2013-08-15 16:54 ` Luis Henriques
2013-08-15 6:34 ` Greg Kroah-Hartman
2013-08-15 6:31 ` Greg Kroah-Hartman
2013-08-15 7:43 ` Guenter Roeck
2013-08-15 7:55 ` Geert Uytterhoeven
2013-08-15 8:05 ` Guenter Roeck
2013-08-16 4:53 ` Guenter Roeck
2013-08-16 5:10 ` Greg Kroah-Hartman
2013-08-16 8:26 ` Guenter Roeck
2013-08-16 12:41 ` Greg Kroah-Hartman
2013-08-16 20:27 ` Guenter Roeck
2013-08-16 21:55 ` Geert Uytterhoeven
2013-08-16 22:39 ` Guenter Roeck
2013-08-16 23:08 ` Greg Kroah-Hartman
2013-08-16 0:53 ` Greg Kroah-Hartman
2013-08-13 20:33 ` Guenter Roeck
2013-08-13 17:19 ` Shuah Khan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130813063502.332957367@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=amit.shah@redhat.com \
--cc=anantyog@in.ibm.com \
--cc=chayang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=qzhang@redhat.com \
--cc=rusty@rustcorp.com.au \
--cc=sluo@redhat.com \
--cc=stable@vger.kernel.org \
--cc=xfu@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox