* [Patch net] tap: convert a mutex to a spinlock
@ 2017-07-05 20:50 Cong Wang
2017-07-06 7:14 ` Christian Borntraeger
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-05 20:50 UTC (permalink / raw)
To: netdev; +Cc: borntraeger, Cong Wang, Sainath Grandhi
We are not allowed to block on the RCU reader side, so can't
just hold the mutex as before. As a quick fix, convert it to
a spinlock.
Fixes: d9f1f61c0801 ("tap: Extending tap device create/destroy APIs")
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Sainath Grandhi <sainath.grandhi@intel.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
drivers/net/tap.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 4d4173d..d88ae3c 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -106,7 +106,7 @@ struct major_info {
struct rcu_head rcu;
dev_t major;
struct idr minor_idr;
- struct mutex minor_lock;
+ spinlock_t minor_lock;
const char *device_name;
struct list_head next;
};
@@ -416,15 +416,15 @@ int tap_get_minor(dev_t major, struct tap_dev *tap)
goto unlock;
}
- mutex_lock(&tap_major->minor_lock);
- retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_KERNEL);
+ spin_lock(&tap_major->minor_lock);
+ retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_ATOMIC);
if (retval >= 0) {
tap->minor = retval;
} else if (retval == -ENOSPC) {
netdev_err(tap->dev, "Too many tap devices\n");
retval = -EINVAL;
}
- mutex_unlock(&tap_major->minor_lock);
+ spin_unlock(&tap_major->minor_lock);
unlock:
rcu_read_unlock();
@@ -442,12 +442,12 @@ void tap_free_minor(dev_t major, struct tap_dev *tap)
goto unlock;
}
- mutex_lock(&tap_major->minor_lock);
+ spin_lock(&tap_major->minor_lock);
if (tap->minor) {
idr_remove(&tap_major->minor_idr, tap->minor);
tap->minor = 0;
}
- mutex_unlock(&tap_major->minor_lock);
+ spin_unlock(&tap_major->minor_lock);
unlock:
rcu_read_unlock();
@@ -467,13 +467,13 @@ static struct tap_dev *dev_get_by_tap_file(int major, int minor)
goto unlock;
}
- mutex_lock(&tap_major->minor_lock);
+ spin_lock(&tap_major->minor_lock);
tap = idr_find(&tap_major->minor_idr, minor);
if (tap) {
dev = tap->dev;
dev_hold(dev);
}
- mutex_unlock(&tap_major->minor_lock);
+ spin_unlock(&tap_major->minor_lock);
unlock:
rcu_read_unlock();
@@ -1227,7 +1227,7 @@ static int tap_list_add(dev_t major, const char *device_name)
tap_major->major = MAJOR(major);
idr_init(&tap_major->minor_idr);
- mutex_init(&tap_major->minor_lock);
+ spin_lock_init(&tap_major->minor_lock);
tap_major->device_name = device_name;
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch net] tap: convert a mutex to a spinlock
2017-07-05 20:50 [Patch net] tap: convert a mutex to a spinlock Cong Wang
@ 2017-07-06 7:14 ` Christian Borntraeger
2017-07-06 8:24 ` Eric Dumazet
2017-07-06 9:52 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Christian Borntraeger @ 2017-07-06 7:14 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: Sainath Grandhi
On 07/05/2017 10:50 PM, Cong Wang wrote:
> We are not allowed to block on the RCU reader side, so can't
> just hold the mutex as before. As a quick fix, convert it to
> a spinlock.
>
> Fixes: d9f1f61c0801 ("tap: Extending tap device create/destroy APIs")
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Sainath Grandhi <sainath.grandhi@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> drivers/net/tap.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 4d4173d..d88ae3c 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -106,7 +106,7 @@ struct major_info {
> struct rcu_head rcu;
> dev_t major;
> struct idr minor_idr;
> - struct mutex minor_lock;
> + spinlock_t minor_lock;
> const char *device_name;
> struct list_head next;
> };
> @@ -416,15 +416,15 @@ int tap_get_minor(dev_t major, struct tap_dev *tap)
> goto unlock;
> }
>
> - mutex_lock(&tap_major->minor_lock);
> - retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_KERNEL);
> + spin_lock(&tap_major->minor_lock);
> + retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_ATOMIC);
> if (retval >= 0) {
> tap->minor = retval;
> } else if (retval == -ENOSPC) {
> netdev_err(tap->dev, "Too many tap devices\n");
> retval = -EINVAL;
> }
> - mutex_unlock(&tap_major->minor_lock);
> + spin_unlock(&tap_major->minor_lock);
>
> unlock:
> rcu_read_unlock();
> @@ -442,12 +442,12 @@ void tap_free_minor(dev_t major, struct tap_dev *tap)
> goto unlock;
> }
>
> - mutex_lock(&tap_major->minor_lock);
> + spin_lock(&tap_major->minor_lock);
> if (tap->minor) {
> idr_remove(&tap_major->minor_idr, tap->minor);
> tap->minor = 0;
> }
> - mutex_unlock(&tap_major->minor_lock);
> + spin_unlock(&tap_major->minor_lock);
>
> unlock:
> rcu_read_unlock();
> @@ -467,13 +467,13 @@ static struct tap_dev *dev_get_by_tap_file(int major, int minor)
> goto unlock;
> }
>
> - mutex_lock(&tap_major->minor_lock);
> + spin_lock(&tap_major->minor_lock);
> tap = idr_find(&tap_major->minor_idr, minor);
> if (tap) {
> dev = tap->dev;
> dev_hold(dev);
> }
> - mutex_unlock(&tap_major->minor_lock);
> + spin_unlock(&tap_major->minor_lock);
>
> unlock:
> rcu_read_unlock();
> @@ -1227,7 +1227,7 @@ static int tap_list_add(dev_t major, const char *device_name)
> tap_major->major = MAJOR(major);
>
> idr_init(&tap_major->minor_idr);
> - mutex_init(&tap_major->minor_lock);
> + spin_lock_init(&tap_major->minor_lock);
>
> tap_major->device_name = device_name;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch net] tap: convert a mutex to a spinlock
2017-07-05 20:50 [Patch net] tap: convert a mutex to a spinlock Cong Wang
2017-07-06 7:14 ` Christian Borntraeger
@ 2017-07-06 8:24 ` Eric Dumazet
2017-07-06 17:56 ` Cong Wang
2017-07-06 9:52 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2017-07-06 8:24 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, borntraeger, Sainath Grandhi
On Wed, 2017-07-05 at 13:50 -0700, Cong Wang wrote:
> We are not allowed to block on the RCU reader side, so can't
> just hold the mutex as before. As a quick fix, convert it to
> a spinlock.
>
> Fixes: d9f1f61c0801 ("tap: Extending tap device create/destroy APIs")
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Sainath Grandhi <sainath.grandhi@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> drivers/net/tap.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 4d4173d..d88ae3c 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -106,7 +106,7 @@ struct major_info {
> struct rcu_head rcu;
> dev_t major;
> struct idr minor_idr;
> - struct mutex minor_lock;
> + spinlock_t minor_lock;
> const char *device_name;
> struct list_head next;
> };
> @@ -416,15 +416,15 @@ int tap_get_minor(dev_t major, struct tap_dev *tap)
> goto unlock;
> }
>
> - mutex_lock(&tap_major->minor_lock);
> - retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_KERNEL);
idr_preload(GFP_KERNEL);
> + spin_lock(&tap_major->minor_lock);
> + retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_ATOMIC);
> if (retval >= 0) {
> tap->minor = retval;
> } else if (retval == -ENOSPC) {
> netdev_err(tap->dev, "Too many tap devices\n");
> retval = -EINVAL;
> }
> - mutex_unlock(&tap_major->minor_lock);
> + spin_unlock(&tap_major->minor_lock);
idr_preload_end();
>
> unlock:
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch net] tap: convert a mutex to a spinlock
2017-07-05 20:50 [Patch net] tap: convert a mutex to a spinlock Cong Wang
2017-07-06 7:14 ` Christian Borntraeger
2017-07-06 8:24 ` Eric Dumazet
@ 2017-07-06 9:52 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-07-06 9:52 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, borntraeger, sainath.grandhi
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 5 Jul 2017 13:50:00 -0700
> We are not allowed to block on the RCU reader side, so can't
> just hold the mutex as before. As a quick fix, convert it to
> a spinlock.
>
> Fixes: d9f1f61c0801 ("tap: Extending tap device create/destroy APIs")
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Sainath Grandhi <sainath.grandhi@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
I agree with Eric that we should use idr_preload() and
idr_preload_end() here so that we can still use GFP_KERNEL even though
we're now using a spinlock instead of a mutex.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch net] tap: convert a mutex to a spinlock
2017-07-06 8:24 ` Eric Dumazet
@ 2017-07-06 17:56 ` Cong Wang
0 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2017-07-06 17:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, borntraeger, Sainath Grandhi
On Thu, Jul 6, 2017 at 1:24 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-07-05 at 13:50 -0700, Cong Wang wrote:
>> We are not allowed to block on the RCU reader side, so can't
>> just hold the mutex as before. As a quick fix, convert it to
>> a spinlock.
>>
>> Fixes: d9f1f61c0801 ("tap: Extending tap device create/destroy APIs")
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Sainath Grandhi <sainath.grandhi@intel.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> ---
>> drivers/net/tap.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 4d4173d..d88ae3c 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -106,7 +106,7 @@ struct major_info {
>> struct rcu_head rcu;
>> dev_t major;
>> struct idr minor_idr;
>> - struct mutex minor_lock;
>> + spinlock_t minor_lock;
>> const char *device_name;
>> struct list_head next;
>> };
>> @@ -416,15 +416,15 @@ int tap_get_minor(dev_t major, struct tap_dev *tap)
>> goto unlock;
>> }
>>
>> - mutex_lock(&tap_major->minor_lock);
>> - retval = idr_alloc(&tap_major->minor_idr, tap, 1, TAP_NUM_DEVS, GFP_KERNEL);
>
> idr_preload(GFP_KERNEL);
Are you sure we can use GFP_KERNEL? RCU read lock is already taken
at this point, so I am afraid we can't do preload here.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-06 17:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-05 20:50 [Patch net] tap: convert a mutex to a spinlock Cong Wang
2017-07-06 7:14 ` Christian Borntraeger
2017-07-06 8:24 ` Eric Dumazet
2017-07-06 17:56 ` Cong Wang
2017-07-06 9:52 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).