[ovs-dev] [PATCH 18/19] datapath: Hold mutex for DP while userspace is blocking on read.
Jesse Gross
jesse at nicira.com
Thu Dec 9 06:14:16 UTC 2010
Currently we get a pointer to the DP in openvswitch_read() and
openvswitch_poll() and use it without any synchronization. This means
that the DP could disappear from underneath us while we are using it.
Currently, this isn't a problem because userspace is single threaded but
it's better for the locking to be correct.
With this change we hold the mutex while doing a blocking wait, which
means that no changes can be made, including adding/removing flows. It's
possible to make this finer grained but for the time being that isn't done,
since current userspace doesn't care.
Found with lockdep.
Signed-off-by: Jesse Gross <jesse at nicira.com>
---
datapath/datapath.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 08e7450..49dcec5 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1841,10 +1841,9 @@ fault:
static ssize_t openvswitch_read(struct file *f, char __user *buf,
size_t nbytes, loff_t *ppos)
{
- /* XXX is there sufficient synchronization here? */
int listeners = get_listen_mask(f);
int dp_idx = iminor(f->f_dentry->d_inode);
- struct datapath *dp = get_dp(dp_idx);
+ struct datapath *dp = get_dp_locked(dp_idx);
struct sk_buff *skb;
size_t copy_bytes, tot_copy_bytes;
int retval;
@@ -1881,6 +1880,8 @@ static ssize_t openvswitch_read(struct file *f, char __user *buf,
}
}
success:
+ mutex_unlock(&dp->mutex);
+
copy_bytes = tot_copy_bytes = min_t(size_t, skb->len, nbytes);
retval = 0;
@@ -1918,16 +1919,17 @@ success:
retval = tot_copy_bytes;
kfree_skb(skb);
+ return retval;
error:
+ mutex_unlock(&dp->mutex);
return retval;
}
static unsigned int openvswitch_poll(struct file *file, poll_table *wait)
{
- /* XXX is there sufficient synchronization here? */
int dp_idx = iminor(file->f_dentry->d_inode);
- struct datapath *dp = get_dp(dp_idx);
+ struct datapath *dp = get_dp_locked(dp_idx);
unsigned int mask;
if (dp) {
@@ -1935,6 +1937,7 @@ static unsigned int openvswitch_poll(struct file *file, poll_table *wait)
poll_wait(file, &dp->waitqueue, wait);
if (dp_has_packet_of_interest(dp, get_listen_mask(file)))
mask |= POLLIN | POLLRDNORM;
+ mutex_unlock(&dp->mutex);
} else {
mask = POLLIN | POLLRDNORM | POLLHUP;
}
--
1.7.1
More information about the dev
mailing list