[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VcEquDVRA4yNSh4pxczc60=+fdFkeG4Cji1patZYUk6xw@mail.gmail.com>
Date: Sat, 7 Jan 2017 01:20:48 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Sainath Grandhi <sainath.grandhi@...el.com>
Cc: netdev <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>, mahesh@...dewar.net,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv1 5/7] TAP: Extending tap device create/destroy APIs
On Sat, Jan 7, 2017 at 12:33 AM, Sainath Grandhi
<sainath.grandhi@...el.com> wrote:
> Extending tap APIs get/free_minor and create/destroy_cdev to handle more than one
> type of virtual interface.
>
> Signed-off-by: Sainath Grandhi <sainath.grandhi@...el.com>
> Tested-by: Sainath Grandhi <sainath.grandhi@...el.com>
Usually it implies that commiter has tested the stuff.
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -99,12 +99,16 @@ static struct proto tap_proto = {
> };
>
> #define TAP_NUM_DEVS (1U << MINORBITS)
> +
> +LIST_HEAD(major_list);
> +
static ?
> -int tap_get_minor(struct tap_dev *tap)
> +int tap_get_minor(dev_t major, struct tap_dev *tap)
> {
> int retval = -ENOMEM;
> + struct major_info *tap_major, *tmp;
> + bool found = false;
>
> - mutex_lock(&macvtap_major.minor_lock);
> - retval = idr_alloc(&macvtap_major.minor_idr, tap, 1, TAP_NUM_DEVS, GFP_KERNEL);
> + list_for_each_entry_safe(tap_major, tmp, &major_list, next) {
> + if (tap_major->major == MAJOR(major)) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + return -EINVAL;
This is candidate to be a separate helper function. See also below.
> -void tap_free_minor(struct tap_dev *tap)
> +void tap_free_minor(dev_t major, struct tap_dev *tap)
> {
> - mutex_lock(&macvtap_major.minor_lock);
> + struct major_info *tap_major, *tmp;
> + bool found = false;
> +
> + list_for_each_entry_safe(tap_major, tmp, &major_list, next) {
> + if (tap_major->major == MAJOR(major)) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + return;
Here is quite the same code (as above).
> -static struct tap_dev *dev_get_by_tap_minor(int minor)
> +static struct tap_dev *dev_get_by_tap_file(int major, int minor)
> {
> struct net_device *dev = NULL;
> struct tap_dev *tap;
> + struct major_info *tap_major, *tmp;
> + bool found = false;
>
> - mutex_lock(&macvtap_major.minor_lock);
> - tap = idr_find(&macvtap_major.minor_idr, minor);
> + list_for_each_entry_safe(tap_major, tmp, &major_list, next) {
> + if (tap_major->major == major) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + return NULL;
And here.
> +static int tap_list_add(dev_t major, const char *device_name)
> +{
> + int err = 0;
> + struct major_info *tap_major;
Perhaps
+ struct major_info *tap_major;
+ int err = 0;
> +
> + tap_major = kzalloc(sizeof(*tap_major), GFP_ATOMIC);
> +
> + tap_major->major = MAJOR(major);
> +
> + idr_init(&tap_major->minor_idr);
> + mutex_init(&tap_major->minor_lock);
> +
> + tap_major->device_name = device_name;
> +
> + list_add_tail(&tap_major->next, &major_list);
> + return err;
> + err = tap_list_add(*tap_major, device_name);
>
> return err;
return tap_list_add();
> void tap_destroy_cdev(dev_t major, struct cdev *tap_cdev)
> {
> + struct major_info *tap_major, *tmp;
> + bool found = false;
> +
> + list_for_each_entry_safe(tap_major, tmp, &major_list, next) {
> + if (tap_major->major == MAJOR(major)) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + return;
And here.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists