Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 49ee87d5 authored by Skylar Chang's avatar Skylar Chang
Browse files

msm: ipa: fix delete dependency race condition 



IPA RM dependencies are added both by kernel drivers and by
userspace application (IPACM), depending on the use case.
On rare condition, a race is possible between adding the dependency
and deleting it, which results in a bad state of the dependency graph.
This change makes sure that dependency is deleted only if it was added
by the same entity.

CRs-Fixed: 1027773
Change-Id: I9253469887b8913f6f2c513a6c7043ed60400b8a
Acked-by Ady Abraham <adya@qti.qualcomm.com>
Signed-off-by: default avatarSkylar Chang <chiaweic@codeaurora.org>
parent 48f705c3

drivers/platform/msm/ipa/ipa_rm.c

+114 −30
Original line number Diff line number Diff line
@@ -172,19 +172,9 @@ bail: 
}

EXPORT_SYMBOL(ipa_rm_delete_resource);



/**

 * ipa_rm_add_dependency() - create dependency

 *					between 2 resources

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 */

int ipa_rm_add_dependency(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name)

static int _ipa_rm_add_dependency(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name,

			bool userspace_dep)

{

	unsigned long flags;

	int result;
@@ -200,29 +190,53 @@ int ipa_rm_add_dependency(enum ipa_rm_resource_name resource_name, 
	result = ipa_rm_dep_graph_add_dependency(

						ipa_rm_ctx->dep_graph,

						resource_name,

						depends_on_name);

						depends_on_name,

						userspace_dep);

	spin_unlock_irqrestore(&ipa_rm_ctx->ipa_rm_lock, flags);

	IPA_RM_DBG("EXIT with %d\n", result);



	return result;

}



/**

 * ipa_rm_add_dependency() - create dependency between 2 resources

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 */

int ipa_rm_add_dependency(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_add_dependency(resource_name, depends_on_name, false);

}

EXPORT_SYMBOL(ipa_rm_add_dependency);



/**

 * ipa_rm_add_dependency_sync() - Create a dependency between 2 resources

 * in a synchronized fashion. In case a producer resource is in GRANTED state

 * and the newly added consumer resource is in RELEASED state, the consumer

 * entity will be requested and the function will block until the consumer

 * is granted.

 * ipa_rm_add_dependency_from_ioctl() - create dependency between 2 resources

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * This function is expected to be called from IOCTL and the dependency will be

 * marked as is was added by the userspace.

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: May block. See documentation above.

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 */

int ipa_rm_add_dependency_sync(enum ipa_rm_resource_name resource_name,

int ipa_rm_add_dependency_from_ioctl(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_add_dependency(resource_name, depends_on_name, true);

}



static int _ipa_rm_add_dependency_sync(enum ipa_rm_resource_name resource_name,

		enum ipa_rm_resource_name depends_on_name,

		bool userspsace_dep)

{

	int result;

	struct ipa_rm_resource *consumer;
@@ -240,7 +254,8 @@ int ipa_rm_add_dependency_sync(enum ipa_rm_resource_name resource_name, 
	result = ipa_rm_dep_graph_add_dependency(

						ipa_rm_ctx->dep_graph,

						resource_name,

						depends_on_name);

						depends_on_name,

						userspsace_dep);

	spin_unlock_irqrestore(&ipa_rm_ctx->ipa_rm_lock, flags);

	if (result == -EINPROGRESS) {

		ipa_rm_dep_graph_get_resource(ipa_rm_ctx->dep_graph,
@@ -268,21 +283,54 @@ int ipa_rm_add_dependency_sync(enum ipa_rm_resource_name resource_name, 


	return result;

}

/**

 * ipa_rm_add_dependency_sync() - Create a dependency between 2 resources

 * in a synchronized fashion. In case a producer resource is in GRANTED state

 * and the newly added consumer resource is in RELEASED state, the consumer

 * entity will be requested and the function will block until the consumer

 * is granted.

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * This function is expected to be called from IOCTL and the dependency will be

 * marked as is was added by the userspace.

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: May block. See documentation above.

 */

int ipa_rm_add_dependency_sync(enum ipa_rm_resource_name resource_name,

		enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_add_dependency_sync(resource_name, depends_on_name,

		false);

}

EXPORT_SYMBOL(ipa_rm_add_dependency_sync);



/**

 * ipa_rm_delete_dependency() - create dependency

 *					between 2 resources

 * ipa_rm_add_dependency_sync_from_ioctl() - Create a dependency between 2

 * resources in a synchronized fashion. In case a producer resource is in

 * GRANTED state and the newly added consumer resource is in RELEASED state,

 * the consumer entity will be requested and the function will block until

 * the consumer is granted.

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 * Side effects: May block. See documentation above.

 */

int ipa_rm_delete_dependency(enum ipa_rm_resource_name resource_name,

int ipa_rm_add_dependency_sync_from_ioctl(

	enum ipa_rm_resource_name resource_name,

	enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_add_dependency_sync(resource_name, depends_on_name,

		true);

}



static int _ipa_rm_delete_dependency(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name,

			bool userspace_dep)

{

	unsigned long flags;

	int result;
@@ -298,14 +346,50 @@ int ipa_rm_delete_dependency(enum ipa_rm_resource_name resource_name, 
	result = ipa_rm_dep_graph_delete_dependency(

			  ipa_rm_ctx->dep_graph,

			  resource_name,

			  depends_on_name);

			  depends_on_name,

			  userspace_dep);

	spin_unlock_irqrestore(&ipa_rm_ctx->ipa_rm_lock, flags);

	IPA_RM_DBG("EXIT with %d\n", result);



	return result;

}



/**

 * ipa_rm_delete_dependency() - delete dependency between 2 resources

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 */

int ipa_rm_delete_dependency(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_delete_dependency(resource_name, depends_on_name, false);

}

EXPORT_SYMBOL(ipa_rm_delete_dependency);



/**

 * ipa_rm_delete_dependency_fron_ioctl() - delete dependency between 2 resources

 * @resource_name: name of dependent resource

 * @depends_on_name: name of its dependency

 *

 * This function is expected to be called from IOCTL and the dependency will be

 * marked as is was added by the userspace.

 *

 * Returns: 0 on success, negative on failure

 *

 * Side effects: IPA_RM_RESORCE_GRANTED could be generated

 * in case client registered with IPA RM

 */

int ipa_rm_delete_dependency_from_ioctl(enum ipa_rm_resource_name resource_name,

			enum ipa_rm_resource_name depends_on_name)

{

	return _ipa_rm_delete_dependency(resource_name, depends_on_name, true);

}



/**

 * ipa_rm_request_resource() - request resource

 * @resource_name: [in] name of the requested resource

drivers/platform/msm/ipa/ipa_rm_dependency_graph.c

+11 −5
Original line number Diff line number Diff line 
/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.

/* Copyright (c) 2013-2016, The Linux Foundation. All rights reserved.

 *

 * This program is free software; you can redistribute it and/or modify

 * it under the terms of the GNU General Public License version 2 and
@@ -152,12 +152,14 @@ int ipa_rm_dep_graph_remove(struct ipa_rm_dep_graph *graph, 
 * @graph: [in] dependency graph

 * @resource_name: [in] resource to add

 * @depends_on_name: [in] resource to add

 * @userspace_dep: [in] operation requested by userspace ?

 *

 * Returns: 0 on success, negative on failure

 */

int ipa_rm_dep_graph_add_dependency(struct ipa_rm_dep_graph *graph,

				    enum ipa_rm_resource_name resource_name,

				    enum ipa_rm_resource_name depends_on_name)

				    enum ipa_rm_resource_name depends_on_name,

				    bool userspace_dep)

{

	struct ipa_rm_resource *dependent = NULL;

	struct ipa_rm_resource *dependency = NULL;
@@ -186,7 +188,8 @@ int ipa_rm_dep_graph_add_dependency(struct ipa_rm_dep_graph *graph, 
		result = -EINVAL;

		goto bail;

	}

	result = ipa_rm_resource_add_dependency(dependent, dependency);

	result = ipa_rm_resource_add_dependency(dependent, dependency,

		userspace_dep);

bail:

	IPA_RM_DBG("EXIT with %d\n", result);
@@ -199,13 +202,15 @@ bail: 
 * @graph: [in] dependency graph

 * @resource_name: [in] resource to delete

 * @depends_on_name: [in] resource to delete

 * @userspace_dep: [in] operation requested by userspace ?

 *

 * Returns: 0 on success, negative on failure

 *

 */

int ipa_rm_dep_graph_delete_dependency(struct ipa_rm_dep_graph *graph,

				enum ipa_rm_resource_name resource_name,

				enum ipa_rm_resource_name depends_on_name)

				enum ipa_rm_resource_name depends_on_name,

				bool userspace_dep)

{

	struct ipa_rm_resource *dependent = NULL;

	struct ipa_rm_resource *dependency = NULL;
@@ -237,7 +242,8 @@ int ipa_rm_dep_graph_delete_dependency(struct ipa_rm_dep_graph *graph, 
		goto bail;

	}



	result = ipa_rm_resource_delete_dependency(dependent, dependency);

	result = ipa_rm_resource_delete_dependency(dependent, dependency,

		userspace_dep);

bail:

	IPA_RM_DBG("EXIT with %d\n", result);

drivers/platform/msm/ipa/ipa_rm_dependency_graph.h

+5 −3
Original line number Diff line number Diff line 
/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.

/* Copyright (c) 2013-2016, The Linux Foundation. All rights reserved.

 *

 * This program is free software; you can redistribute it and/or modify

 * it under the terms of the GNU General Public License version 2 and
@@ -38,10 +38,12 @@ int ipa_rm_dep_graph_remove(struct ipa_rm_dep_graph *graph, 


int ipa_rm_dep_graph_add_dependency(struct ipa_rm_dep_graph *graph,

				enum ipa_rm_resource_name resource_name,

				enum ipa_rm_resource_name depends_on_name);

				enum ipa_rm_resource_name depends_on_name,

				bool userspsace_dep);



int ipa_rm_dep_graph_delete_dependency(struct ipa_rm_dep_graph *graph,

				enum ipa_rm_resource_name resource_name,

				enum ipa_rm_resource_name depends_on_name);

				enum ipa_rm_resource_name depends_on_name,

				bool userspsace_dep);



#endif /* _IPA_RM_DEPENDENCY_GRAPH_H_ */

drivers/platform/msm/ipa/ipa_rm_i.h

+6 −0
Original line number Diff line number Diff line
@@ -146,6 +146,12 @@ int ipa_rm_request_resource_with_timer(enum ipa_rm_resource_name resource_name); 


void delayed_release_work_func(struct work_struct *work);



int ipa_rm_add_dependency_from_ioctl(enum ipa_rm_resource_name resource_name,

	enum ipa_rm_resource_name depends_on_name);



int ipa_rm_delete_dependency_from_ioctl(enum ipa_rm_resource_name resource_name,

	enum ipa_rm_resource_name depends_on_name);



void ipa_rm_exit(void);



#endif /* _IPA_RM_I_H_ */

drivers/platform/msm/ipa/ipa_rm_peers_list.c

+46 −12
Original line number Diff line number Diff line
@@ -68,7 +68,7 @@ int ipa_rm_peers_list_create(int max_peers, 


	(*peers_list)->max_peers = max_peers;

	(*peers_list)->peers = kzalloc((*peers_list)->max_peers *

				sizeof(struct ipa_rm_resource *), GFP_ATOMIC);

			sizeof(*((*peers_list)->peers)), GFP_ATOMIC);

	if (!((*peers_list)->peers)) {

		IPA_RM_ERR("no mem\n");

		result = -ENOMEM;
@@ -112,7 +112,9 @@ void ipa_rm_peers_list_remove_peer( 
		return;



	peers_list->peers[ipa_rm_peers_list_get_resource_index(

			resource_name)] = NULL;

			resource_name)].resource = NULL;

	peers_list->peers[ipa_rm_peers_list_get_resource_index(

			resource_name)].userspace_dep = false;

	peers_list->peers_count--;

}
@@ -125,14 +127,16 @@ void ipa_rm_peers_list_remove_peer( 
 */

void ipa_rm_peers_list_add_peer(

		struct ipa_rm_peers_list *peers_list,

		struct ipa_rm_resource *resource)

		struct ipa_rm_resource *resource,

		bool userspace_dep)

{

	if (!peers_list || !resource)

		return;



	peers_list->peers[ipa_rm_peers_list_get_resource_index(

			resource->name)] =

			resource;

			resource->name)].resource = resource;

	peers_list->peers[ipa_rm_peers_list_get_resource_index(

		resource->name)].userspace_dep = userspace_dep;

	peers_list->peers_count++;

}
@@ -186,6 +190,7 @@ bail: 
 * @resource_name: first peers list resource name

 * @depends_on_peers: second peers list

 * @depends_on_name: second peers list resource name

 * @userspace_dep: [out] dependency was created by userspace

 *

 * Returns: true if there is dependency, false otherwise

 *
@@ -194,20 +199,28 @@ bool ipa_rm_peers_list_check_dependency( 
		struct ipa_rm_peers_list *resource_peers,

		enum ipa_rm_resource_name resource_name,

		struct ipa_rm_peers_list *depends_on_peers,

		enum ipa_rm_resource_name depends_on_name)

		enum ipa_rm_resource_name depends_on_name,

		bool *userspace_dep)

{

	bool result = false;

	int resource_index;



	if (!resource_peers || !depends_on_peers)

	if (!resource_peers || !depends_on_peers || !userspace_dep)

		return result;



	if (resource_peers->peers[ipa_rm_peers_list_get_resource_index(

			depends_on_name)] != NULL)

	resource_index = ipa_rm_peers_list_get_resource_index(depends_on_name);

	if (resource_peers->peers[resource_index].resource != NULL) {

		result = true;

		*userspace_dep = resource_peers->peers[resource_index].

			userspace_dep;

	}



	if (depends_on_peers->peers[ipa_rm_peers_list_get_resource_index(

						resource_name)] != NULL)

	resource_index = ipa_rm_peers_list_get_resource_index(resource_name);

	if (depends_on_peers->peers[resource_index].resource != NULL) {

		result = true;

		*userspace_dep = depends_on_peers->peers[resource_index].

			userspace_dep;

	}



	return result;

}
@@ -228,7 +241,28 @@ struct ipa_rm_resource *ipa_rm_peers_list_get_resource(int resource_index, 
	if (!ipa_rm_peers_list_check_index(resource_index, resource_peers))

		goto bail;



	result = resource_peers->peers[resource_index];

	result = resource_peers->peers[resource_index].resource;

bail:

	return result;

}



/**

 * ipa_rm_peers_list_get_userspace_dep() - returns whether resource dependency

 * was added by userspace

 * @resource_index: resource index

 * @resource_peers: peers list

 *

 * Returns: true if dependency was added by userspace, false by kernel

 */

bool ipa_rm_peers_list_get_userspace_dep(int resource_index,

		struct ipa_rm_peers_list *resource_peers)

{

	bool result = false;



	if (!ipa_rm_peers_list_check_index(resource_index, resource_peers))

		goto bail;



	result = resource_peers->peers[resource_index].userspace_dep;

bail:

	return result;

}