| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Aug 2015 23:14:29 -0500
From: Bjorn Helgaas <bhelgaas@...gle.com>
To: Yinghai Lu <yinghai@...nel.org>
Cc: David Miller <davem@...emloft.net>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Wei Yang <weiyang@...ux.vnet.ibm.com>, TJ <linux@....tj>,
Yijing Wang <wangyijing@...wei.com>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 32/51] resources: Split out __allocate_resource()
On Mon, Jul 27, 2015 at 04:29:50PM -0700, Yinghai Lu wrote:
> It will not hold lock, so we could use it in other functions that
> hold the resource lock already.
What's the benefit of this patch? Does it fix something? Does it add some
functionality we need?
>
> -v2: according to Linus, using "bool lock" as parameter
> aka "conditionally take lock" is *wrong*.
>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> Acked-by: Linus Torvalds <torvalds@...ux-foundation.org>
> ---
> kernel/resource.c | 70 +++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 50 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index fed052a..67b58a5 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -619,7 +619,7 @@ static int find_resource(struct resource *root, struct resource *new,
> }
>
> /**
> - * reallocate_resource - allocate a slot in the resource tree given range & alignment.
> + * __reallocate_resource - allocate a slot in the resource tree given range & alignment.
> * The resource will be relocated if the new size cannot be reallocated in the
> * current location.
> *
> @@ -628,7 +628,7 @@ static int find_resource(struct resource *root, struct resource *new,
> * @newsize: new size of the resource descriptor
> * @constraint: the size and alignment constraints to be met.
> */
> -static int reallocate_resource(struct resource *root, struct resource *old,
> +static int __reallocate_resource(struct resource *root, struct resource *old,
> resource_size_t newsize,
> struct resource_constraint *constraint)
> {
> @@ -636,8 +636,6 @@ static int reallocate_resource(struct resource *root, struct resource *old,
> struct resource new = *old;
> struct resource *conflict;
>
> - write_lock(&resource_lock);
> -
> if ((err = __find_resource(root, old, &new, newsize, constraint)))
> goto out;
>
> @@ -662,14 +660,13 @@ static int reallocate_resource(struct resource *root, struct resource *old,
> BUG_ON(conflict);
> }
> out:
> - write_unlock(&resource_lock);
> return err;
> }
>
> -
> /**
> - * allocate_resource - allocate empty slot in the resource tree given range & alignment.
> - * The resource will be reallocated with a new size if it was already allocated
> + * __allocate_resource - allocate empty slot in the resource tree given range & alignment.
> + * The resource will be reallocated with a new size if it was already
> + * allocated
> * @root: root resource descriptor
> * @new: resource descriptor desired by caller
> * @size: requested resource region size
> @@ -678,15 +675,17 @@ out:
> * @align: alignment requested, in bytes
> * @alignf: alignment function, optional, called if not NULL
> * @alignf_data: arbitrary data to pass to the @alignf function
> + *
> + * Caller need to hold resource_lock if needed.
> */
> -int allocate_resource(struct resource *root, struct resource *new,
> - resource_size_t size, resource_size_t min,
> - resource_size_t max, resource_size_t align,
> - resource_size_t (*alignf)(void *,
> - const struct resource *,
> - resource_size_t,
> - resource_size_t),
> - void *alignf_data)
> +static int __allocate_resource(struct resource *root, struct resource *new,
> + resource_size_t size, resource_size_t min,
> + resource_size_t max, resource_size_t align,
> + resource_size_t (*alignf)(void *,
> + const struct resource *,
> + resource_size_t,
> + resource_size_t),
> + void *alignf_data)
> {
> int err;
> struct resource_constraint constraint;
> @@ -700,20 +699,51 @@ int allocate_resource(struct resource *root, struct resource *new,
> constraint.alignf = alignf;
> constraint.alignf_data = alignf_data;
>
> - if ( new->parent ) {
> + if (new->parent) {
> /* resource is already allocated, try reallocating with
> the new constraints */
> - return reallocate_resource(root, new, size, &constraint);
> + return __reallocate_resource(root, new, size, &constraint);
> }
>
> - write_lock(&resource_lock);
> err = find_resource(root, new, size, &constraint);
> if (err >= 0 && __request_resource(root, new))
> err = -EBUSY;
> - write_unlock(&resource_lock);
> +
> return err;
> }
>
> +/**
> + * allocate_resource - allocate empty slot in the resource tree given range & alignment.
> + * The resource will be reallocated with a new size if it was already
> + * allocated
> + * @root: root resource descriptor
> + * @new: resource descriptor desired by caller
> + * @size: requested resource region size
> + * @min: minimum boundary to allocate
> + * @max: maximum boundary to allocate
> + * @align: alignment requested, in bytes
> + * @alignf: alignment function, optional, called if not NULL
> + * @alignf_data: arbitrary data to pass to the @alignf function
> + */
> +int allocate_resource(struct resource *root, struct resource *new,
> + resource_size_t size, resource_size_t min,
> + resource_size_t max, resource_size_t align,
> + resource_size_t (*alignf)(void *,
> + const struct resource *,
> + resource_size_t,
> + resource_size_t),
> + void *alignf_data)
> +{
> + int ret;
> +
> + write_lock(&resource_lock);
> + ret = __allocate_resource(root, new, size, min, max, align,
> + alignf, alignf_data);
> + write_unlock(&resource_lock);
> +
> + return ret;
> +}
> +
> EXPORT_SYMBOL(allocate_resource);
>
> /**
> --
> 1.8.4.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists