[PATCH 137/141] staging: unisys: Hide contents of pending_msg_hdr
Benjamin Romer
benjamin.romer at unisys.com
Tue May 5 22:37:54 UTC 2015
From: Don Zickus <dzickus at redhat.com>
The pending_msg_hdr element of struct visor_device should really be
private and not public. Currently there is no easy way to do that
without allocating memory and passing it along from the command
notifier all the way to the command responder.
I tried to minimize the change and use NULL as the equivalent to
INVALID. I also added a check for when the pending_msg_hdr is not
NULL during the call to _epilog. This indicates a previous command
is inflight for the device/bus. Not good.
I also guessed on an appropriate memory failure response to return
in case the kzalloc failed to allocate memory for the message header.
Signed-off-by: Don Zickus <dzickus at redhat.com>
Signed-off-by: Benjamin Romer <benjamin.romer at unisys.com>
---
drivers/staging/unisys/include/visorbus.h | 5 +-
drivers/staging/unisys/visorbus/visorchipset.c | 67 +++++++++++++++++---------
2 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index cc0724a..17c15f5 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -37,7 +37,6 @@
#include "periodic_work.h"
#include "channel.h"
-#include "controlvmchannel.h"
struct visor_driver;
struct visor_device;
@@ -118,6 +117,8 @@ struct visor_driver {
/** A device type for things "plugged" into the visorbus bus */
+struct controlvm_message_header;
+
struct visor_device {
/** visor driver can use the visorchannel member with the functions
* defined in visorchannel.h to access the channel
@@ -154,7 +155,7 @@ struct visor_device {
uuid_le inst;
u8 *name;
u8 *description;
- struct controlvm_message_header pending_msg_hdr;
+ struct controlvm_message_header *pending_msg_hdr;
void *vbus_hdr_info;
u32 switch_no;
u32 internal_port_no;
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index b1e059a..4a895e8 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -885,12 +885,13 @@ bus_responder(enum controlvm_id cmd_id, struct visor_device *p, int response)
if (cmd_id == CONTROLVM_BUS_CREATE)
p->state.created = 1;
- if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+ if (p->pending_msg_hdr == NULL)
return; /* no controlvm response needed */
- if (p->pending_msg_hdr.id != (u32)cmd_id)
+ if (p->pending_msg_hdr->id != (u32)cmd_id)
return;
- controlvm_respond(&p->pending_msg_hdr, response);
- p->pending_msg_hdr.id = CONTROLVM_INVALID;
+ controlvm_respond(p->pending_msg_hdr, response);
+ kfree(p->pending_msg_hdr);
+ p->pending_msg_hdr = NULL;
}
static void
@@ -900,12 +901,12 @@ device_changestate_responder(enum controlvm_id cmd_id,
{
struct controlvm_message outmsg;
- if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+ if (p->pending_msg_hdr == NULL)
return; /* no controlvm response needed */
- if (p->pending_msg_hdr.id != cmd_id)
+ if (p->pending_msg_hdr->id != cmd_id)
return;
- controlvm_init_response(&outmsg, &p->pending_msg_hdr, response);
+ controlvm_init_response(&outmsg, p->pending_msg_hdr, response);
outmsg.cmd.device_change_state.bus_no = p->chipset_bus_no;
outmsg.cmd.device_change_state.dev_no = p->chipset_dev_no;
@@ -915,7 +916,8 @@ device_changestate_responder(enum controlvm_id cmd_id,
CONTROLVM_QUEUE_REQUEST, &outmsg))
return;
- p->pending_msg_hdr.id = CONTROLVM_INVALID;
+ kfree(p->pending_msg_hdr);
+ p->pending_msg_hdr = NULL;
}
static void
@@ -926,14 +928,15 @@ device_responder(enum controlvm_id cmd_id, struct visor_device *p, int response)
p->state.created = 1;
}
- if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+ if (p->pending_msg_hdr == NULL)
return; /* no controlvm response needed */
- if (p->pending_msg_hdr.id != (u32)cmd_id)
+ if (p->pending_msg_hdr->id != (u32)cmd_id)
return;
- controlvm_respond(&p->pending_msg_hdr, response);
- p->pending_msg_hdr.id = CONTROLVM_INVALID;
+ controlvm_respond(p->pending_msg_hdr, response);
+ kfree(p->pending_msg_hdr);
+ p->pending_msg_hdr = NULL;
}
static void
@@ -942,12 +945,21 @@ bus_epilog(struct visor_device *bus_info,
int response, bool need_response)
{
bool notified = false;
-
- if (need_response) {
- memcpy(&bus_info->pending_msg_hdr, msg_hdr,
+ struct controlvm_message_header *pmsg_hdr;
+
+ if (bus_info->pending_msg_hdr) {
+ /* only non-NULL if bus is still waiting on a response */
+ response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
+ goto away;
+ } else if (need_response) {
+ pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
+ if (!pmsg_hdr) {
+ response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
+ goto away;
+ }
+ memcpy(pmsg_hdr, msg_hdr,
sizeof(struct controlvm_message_header));
- } else {
- bus_info->pending_msg_hdr.id = CONTROLVM_INVALID;
+ bus_info->pending_msg_hdr = pmsg_hdr;
}
down(¬ifier_lock);
@@ -967,6 +979,7 @@ bus_epilog(struct visor_device *bus_info,
break;
}
}
+away:
if (notified)
/* The callback function just called above is responsible
* for calling the appropriate visorchipset_busdev_responders
@@ -987,6 +1000,7 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
bool notified = false;
u32 bus_no = dev_info->chipset_bus_no;
u32 dev_no = dev_info->chipset_dev_no;
+ struct controlvm_message_header *pmsg_hdr;
char *envp[] = {
"SPARSP_DIAGPOOL_PAUSED_STATE = 1",
@@ -995,11 +1009,19 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
notifiers = &busdev_notifiers;
- if (need_response) {
- memcpy(&dev_info->pending_msg_hdr, msg_hdr,
+ if (dev_info->pending_msg_hdr) {
+ /* only non-NULL if dev is still waiting on a response */
+ response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
+ goto away;
+ } else if (need_response) {
+ pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
+ if (!pmsg_hdr) {
+ response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
+ goto away;
+ }
+ memcpy(pmsg_hdr, msg_hdr,
sizeof(struct controlvm_message_header));
- } else {
- dev_info->pending_msg_hdr.id = CONTROLVM_INVALID;
+ dev_info->pending_msg_hdr = pmsg_hdr;
}
down(¬ifier_lock);
@@ -1057,6 +1079,7 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
break;
}
}
+away:
if (notified)
/* The callback function just called above is responsible
* for calling the appropriate visorchipset_busdev_responders
@@ -1156,7 +1179,7 @@ bus_configure(struct controlvm_message *inmsg,
POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
POSTCODE_SEVERITY_ERR);
rc = -CONTROLVM_RESP_ERROR_BUS_INVALID;
- } else if (bus_info->pending_msg_hdr.id != CONTROLVM_INVALID) {
+ } else if (bus_info->pending_msg_hdr != NULL) {
POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
POSTCODE_SEVERITY_ERR);
rc = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
--
2.1.4
More information about the devel
mailing list