linux-kernel - Re: [PATCH 1/1] vfio: put off the allocation of "minor" in vfio_create

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 20 Nov 2014 08:37:17 -0700
From:	Alex Williamson <alex.williamson@...hat.com>
To:	Zhen Lei <thunder.leizhen@...wei.com>
Cc:	KVM <kvm@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Zefan Li <lizefan@...wei.com>, Xinwei Hu <huxinwei@...wei.com>,
	Tianhong Ding <dingtianhong@...wei.com>,
	Kefeng Wang <wangkefeng.wang@...wei.com>
Subject: Re: [PATCH 1/1] vfio: put off the allocation of "minor" in
 vfio_create_group

On Thu, 2014-11-20 at 19:25 +0800, Zhen Lei wrote:
> The next code fragment "list_for_each_entry" is not depend on "minor". With this
> patch, the free of "minor" in "list_for_each_entry" can be reduced, and there is
> no functional change.

A reasonable micro-optimization, but I'm curious if you're actually
seeing some measurable overhead from this.  It seems like we'd need to
have multiple devices, all within the same IOMMU group, all probed by
vfio-pci at the same time to exercise the race condition.  Thanks,

Alex

> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
> ---
>  drivers/vfio/vfio.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index f018d8d..737eb468 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -225,22 +225,21 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
> 
>  	mutex_lock(&vfio.group_lock);
> 
> -	minor = vfio_alloc_group_minor(group);
> -	if (minor < 0) {
> -		vfio_group_unlock_and_free(group);
> -		return ERR_PTR(minor);
> -	}
> -
>  	/* Did we race creating this group? */
>  	list_for_each_entry(tmp, &vfio.group_list, vfio_next) {
>  		if (tmp->iommu_group == iommu_group) {
>  			vfio_group_get(tmp);
> -			vfio_free_group_minor(minor);
>  			vfio_group_unlock_and_free(group);
>  			return tmp;
>  		}
>  	}
> 
> +	minor = vfio_alloc_group_minor(group);
> +	if (minor < 0) {
> +		vfio_group_unlock_and_free(group);
> +		return ERR_PTR(minor);
> +	}
> +
>  	dev = device_create(vfio.class, NULL,
>  			    MKDEV(MAJOR(vfio.group_devt), minor),
>  			    group, "%d", iommu_group_id(iommu_group));
> --
> 1.8.0
> 
> 
> --
> 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/



--
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/