linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] VMCI: drop condition with no effect
@ 2016-11-06 12:53 Nicholas Mc Guire
  2016-11-06 12:53 ` [PATCH 2/2] VMCI: drop unused variable Nicholas Mc Guire
  0 siblings, 1 reply; 2+ messages in thread
From: Nicholas Mc Guire @ 2016-11-06 12:53 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Michal Hocko, George Zhang, linux-kernel, Nicholas Mc Guire

The condition calls the same code, further the code called does not 
differentiate between old/new-style VMX anyway so the if/else here 
is simply not needed.

Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

problem found by coccinelle

drivers/misc/vmw_vmci/vmci_queue_pair.c:vmci_qp_broker_detach()
2232        if (entry->vmci_page_files)
2233                qp_host_unregister_user_memory(entry->produce_q,
2234                                               entry->
2235                                               consume_q);
2236        else
2237                qp_host_unregister_user_memory(entry->produce_q,
2238                                               entry->
2239                                               consume_q);

The original driver code was calling:
               if (entry->vmciPageFiles) {
                       VMCIHost_ReleaseUserMemory(entry->produceQ,
                                                  entry->consumeQ);
               } else {
                       VMCIHost_UnregisterUserMemory(entry->produceQ,
                                                     entry->consumeQ);
               }
but the VMCIHost_ReleaseUserMemory equivalent function seems to not
have ever existed in the mainline Linus version of the driver so the 
differentiation no longer is needed.

The difference handled by setting vmci_page_files is old-style and
new-style VMX but the qp_host_unregister_user_memory and the there
called qp_release_pages() are not differentiating between old and
new style VMX so it does not seem to be a missing parameter and 
from the documentation it seems that the unregister function is to
be called after unmapping - so this if/else here seems to make no
sense - from code review I suspect it can be dropped but this would 
need confirmation by someone that understands the details of the 
driver.

The documentation at the top of the file also seems to be a bit out of
date/inconsistent with respect to old-style/new-style VMX (or Im just 
not understanding it).

Patch was compile checked with: x86_64_defconfig + CONFIG_VMWARE_VMCI=m

Patch is against 4.9.0-rc2 (localversion-next is next-20161028)

 drivers/misc/vmw_vmci/vmci_queue_pair.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index f84a427..6f6069c 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -2229,14 +2229,8 @@ int vmci_qp_broker_detach(struct vmci_handle handle, struct vmci_ctx *context)
 					handle.context, handle.resource,
 					result);
 
-			if (entry->vmci_page_files)
-				qp_host_unregister_user_memory(entry->produce_q,
-							       entry->
-							       consume_q);
-			else
-				qp_host_unregister_user_memory(entry->produce_q,
-							       entry->
-							       consume_q);
+			qp_host_unregister_user_memory(entry->produce_q,
+						       entry->consume_q);
 
 		}
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH 2/2] VMCI: drop unused variable
  2016-11-06 12:53 [PATCH 1/2] VMCI: drop condition with no effect Nicholas Mc Guire
@ 2016-11-06 12:53 ` Nicholas Mc Guire
  0 siblings, 0 replies; 2+ messages in thread
From: Nicholas Mc Guire @ 2016-11-06 12:53 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Michal Hocko, George Zhang, linux-kernel, Nicholas Mc Guire

The boolean vmci_page_files is used to differentiate between old style
VMX and new-style, but the code actually only used this variable
at one point in an if/else that had no effect so it can be removed. 

Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

drivers/misc/vmw_vmci/vmci_queue_pair.c:vmci_qp_broker_detach()
2232        if (entry->vmci_page_files)
2233                qp_host_unregister_user_memory(entry->produce_q,
2234                                               entry->
2235                                               consume_q);
2236        else
2237                qp_host_unregister_user_memory(entry->produce_q,
2238                                               entry->
2239                                               consume_q);

As this is the only place where vmci_page_files is being used this
could be dropped as well provided the dropped if/else is correct.

Patch was compile checked with: x86_64_defconfig + CONFIG_VMWARE_VMCI=m

Patch is against 4.9.0-rc2 (localversion-next is next-20161028)

 drivers/misc/vmw_vmci/vmci_queue_pair.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index 6f6069c..1b515ac 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -224,7 +224,6 @@ struct qp_broker_entry {
 	enum qp_broker_state state;
 	bool require_trusted_attach;
 	bool created_by_trusted;
-	bool vmci_page_files;	/* Created by VMX using VMCI page files */
 	struct vmci_queue *produce_q;
 	struct vmci_queue *consume_q;
 	struct vmci_queue_header saved_produce_q;
@@ -1435,7 +1434,6 @@ static int qp_broker_create(struct vmci_handle handle,
 	    !!(context->priv_flags & VMCI_PRIVILEGE_FLAG_RESTRICTED);
 	entry->created_by_trusted =
 	    !!(priv_flags & VMCI_PRIVILEGE_FLAG_TRUSTED);
-	entry->vmci_page_files = false;
 	entry->wakeup_cb = wakeup_cb;
 	entry->client_data = client_data;
 	entry->produce_q = qp_host_alloc_queue(guest_produce_size);
@@ -2112,8 +2110,6 @@ int vmci_qp_broker_set_page_store(struct vmci_handle handle,
 	else
 		entry->state = VMCIQPB_ATTACHED_MEM;
 
-	entry->vmci_page_files = true;
-
 	if (entry->state == VMCIQPB_ATTACHED_MEM) {
 		result =
 		    qp_notify_peer(true, handle, context_id, entry->create_id);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-11-06 12:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-06 12:53 [PATCH 1/2] VMCI: drop condition with no effect Nicholas Mc Guire
2016-11-06 12:53 ` [PATCH 2/2] VMCI: drop unused variable Nicholas Mc Guire

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).