* [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl
@ 2016-05-09 4:07 Tomas Winkler
2016-05-09 4:07 ` [char-misc-next 2/2] mei: drop wr_msg from the mei_dev structure Tomas Winkler
2016-05-23 13:07 ` [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Winkler, Tomas
0 siblings, 2 replies; 5+ messages in thread
From: Tomas Winkler @ 2016-05-09 4:07 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
wr_ctrl waiters are none interruptible, so should be waken up
with call to wake_up and not to wake_up_interruptible.
This fixes commit:
7ff4bdd ("mei: fix waiting for wr_ctrl for corner cases.")
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/misc/mei/client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index eed254da63a8..641c1a566687 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -730,7 +730,7 @@ static void mei_cl_wake_all(struct mei_cl *cl)
/* synchronized under device mutex */
if (waitqueue_active(&cl->wait)) {
cl_dbg(dev, cl, "Waking up ctrl write clients!\n");
- wake_up_interruptible(&cl->wait);
+ wake_up(&cl->wait);
}
}
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [char-misc-next 2/2] mei: drop wr_msg from the mei_dev structure
2016-05-09 4:07 [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Tomas Winkler
@ 2016-05-09 4:07 ` Tomas Winkler
2016-05-23 13:07 ` [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Winkler, Tomas
1 sibling, 0 replies; 5+ messages in thread
From: Tomas Winkler @ 2016-05-09 4:07 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
The control messages are usually small, around 8 bytes, and can be
allocated on the stack.
Using on stack allocation allows us to drop 'wr_msg' a rather large
buffer reserved in the mei_dev structure and relax contention
of this device global buffer.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/misc/mei/hbm.c | 137 ++++++++++++++++++++++-----------------------
drivers/misc/mei/mei_dev.h | 10 +---
2 files changed, 68 insertions(+), 79 deletions(-)
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 5aa606c8a827..085f3aafe6fa 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -132,6 +132,7 @@ static inline void mei_hbm_hdr(struct mei_msg_hdr *hdr, size_t length)
hdr->length = length;
hdr->msg_complete = 1;
hdr->reserved = 0;
+ hdr->internal = 0;
}
/**
@@ -165,15 +166,15 @@ void mei_hbm_cl_hdr(struct mei_cl *cl, u8 hbm_cmd, void *buf, size_t len)
* Return: 0 on success, <0 on failure.
*/
static inline
-int mei_hbm_cl_write(struct mei_device *dev,
- struct mei_cl *cl, u8 hbm_cmd, size_t len)
+int mei_hbm_cl_write(struct mei_device *dev, struct mei_cl *cl,
+ u8 hbm_cmd, u8 *buf, size_t len)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
+ struct mei_msg_hdr mei_hdr;
- mei_hbm_hdr(mei_hdr, len);
- mei_hbm_cl_hdr(cl, hbm_cmd, dev->wr_msg.data, len);
+ mei_hbm_hdr(&mei_hdr, len);
+ mei_hbm_cl_hdr(cl, hbm_cmd, buf, len);
- return mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ return mei_write_message(dev, &mei_hdr, buf);
}
/**
@@ -250,24 +251,23 @@ int mei_hbm_start_wait(struct mei_device *dev)
*/
int mei_hbm_start_req(struct mei_device *dev)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_host_version_request *start_req;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_host_version_request start_req;
const size_t len = sizeof(struct hbm_host_version_request);
int ret;
mei_hbm_reset(dev);
- mei_hbm_hdr(mei_hdr, len);
+ mei_hbm_hdr(&mei_hdr, len);
/* host start message */
- start_req = (struct hbm_host_version_request *)dev->wr_msg.data;
- memset(start_req, 0, len);
- start_req->hbm_cmd = HOST_START_REQ_CMD;
- start_req->host_version.major_version = HBM_MAJOR_VERSION;
- start_req->host_version.minor_version = HBM_MINOR_VERSION;
+ memset(&start_req, 0, len);
+ start_req.hbm_cmd = HOST_START_REQ_CMD;
+ start_req.host_version.major_version = HBM_MAJOR_VERSION;
+ start_req.host_version.minor_version = HBM_MINOR_VERSION;
dev->hbm_state = MEI_HBM_IDLE;
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &start_req);
if (ret) {
dev_err(dev->dev, "version message write failed: ret = %d\n",
ret);
@@ -288,23 +288,22 @@ int mei_hbm_start_req(struct mei_device *dev)
*/
static int mei_hbm_enum_clients_req(struct mei_device *dev)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_host_enum_request *enum_req;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_host_enum_request enum_req;
const size_t len = sizeof(struct hbm_host_enum_request);
int ret;
/* enumerate clients */
- mei_hbm_hdr(mei_hdr, len);
+ mei_hbm_hdr(&mei_hdr, len);
- enum_req = (struct hbm_host_enum_request *)dev->wr_msg.data;
- memset(enum_req, 0, len);
- enum_req->hbm_cmd = HOST_ENUM_REQ_CMD;
- enum_req->flags |= dev->hbm_f_dc_supported ?
- MEI_HBM_ENUM_F_ALLOW_ADD : 0;
- enum_req->flags |= dev->hbm_f_ie_supported ?
- MEI_HBM_ENUM_F_IMMEDIATE_ENUM : 0;
+ memset(&enum_req, 0, len);
+ enum_req.hbm_cmd = HOST_ENUM_REQ_CMD;
+ enum_req.flags |= dev->hbm_f_dc_supported ?
+ MEI_HBM_ENUM_F_ALLOW_ADD : 0;
+ enum_req.flags |= dev->hbm_f_ie_supported ?
+ MEI_HBM_ENUM_F_IMMEDIATE_ENUM : 0;
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &enum_req);
if (ret) {
dev_err(dev->dev, "enumeration request write failed: ret = %d.\n",
ret);
@@ -358,23 +357,21 @@ static int mei_hbm_me_cl_add(struct mei_device *dev,
*/
static int mei_hbm_add_cl_resp(struct mei_device *dev, u8 addr, u8 status)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_add_client_response *resp;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_add_client_response resp;
const size_t len = sizeof(struct hbm_add_client_response);
int ret;
dev_dbg(dev->dev, "adding client response\n");
- resp = (struct hbm_add_client_response *)dev->wr_msg.data;
+ mei_hbm_hdr(&mei_hdr, len);
- mei_hbm_hdr(mei_hdr, len);
- memset(resp, 0, sizeof(struct hbm_add_client_response));
+ memset(&resp, 0, sizeof(struct hbm_add_client_response));
+ resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD;
+ resp.me_addr = addr;
+ resp.status = status;
- resp->hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD;
- resp->me_addr = addr;
- resp->status = status;
-
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &resp);
if (ret)
dev_err(dev->dev, "add client response write failed: ret = %d\n",
ret);
@@ -421,18 +418,17 @@ int mei_hbm_cl_notify_req(struct mei_device *dev,
struct mei_cl *cl, u8 start)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_notification_request *req;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_notification_request req;
const size_t len = sizeof(struct hbm_notification_request);
int ret;
- mei_hbm_hdr(mei_hdr, len);
- mei_hbm_cl_hdr(cl, MEI_HBM_NOTIFY_REQ_CMD, dev->wr_msg.data, len);
+ mei_hbm_hdr(&mei_hdr, len);
+ mei_hbm_cl_hdr(cl, MEI_HBM_NOTIFY_REQ_CMD, &req, len);
- req = (struct hbm_notification_request *)dev->wr_msg.data;
- req->start = start;
+ req.start = start;
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &req);
if (ret)
dev_err(dev->dev, "notify request failed: ret = %d\n", ret);
@@ -534,8 +530,8 @@ static void mei_hbm_cl_notify(struct mei_device *dev,
*/
static int mei_hbm_prop_req(struct mei_device *dev, unsigned long start_idx)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_props_request *prop_req;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_props_request prop_req;
const size_t len = sizeof(struct hbm_props_request);
unsigned long addr;
int ret;
@@ -550,15 +546,14 @@ static int mei_hbm_prop_req(struct mei_device *dev, unsigned long start_idx)
return 0;
}
- mei_hbm_hdr(mei_hdr, len);
- prop_req = (struct hbm_props_request *)dev->wr_msg.data;
+ mei_hbm_hdr(&mei_hdr, len);
- memset(prop_req, 0, sizeof(struct hbm_props_request));
+ memset(&prop_req, 0, sizeof(struct hbm_props_request));
- prop_req->hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD;
- prop_req->me_addr = addr;
+ prop_req.hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD;
+ prop_req.me_addr = addr;
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &prop_req);
if (ret) {
dev_err(dev->dev, "properties request write failed: ret = %d\n",
ret);
@@ -581,21 +576,20 @@ static int mei_hbm_prop_req(struct mei_device *dev, unsigned long start_idx)
*/
int mei_hbm_pg(struct mei_device *dev, u8 pg_cmd)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_power_gate *req;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_power_gate req;
const size_t len = sizeof(struct hbm_power_gate);
int ret;
if (!dev->hbm_f_pg_supported)
return -EOPNOTSUPP;
- mei_hbm_hdr(mei_hdr, len);
+ mei_hbm_hdr(&mei_hdr, len);
- req = (struct hbm_power_gate *)dev->wr_msg.data;
- memset(req, 0, len);
- req->hbm_cmd = pg_cmd;
+ memset(&req, 0, len);
+ req.hbm_cmd = pg_cmd;
- ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ ret = mei_write_message(dev, &mei_hdr, &req);
if (ret)
dev_err(dev->dev, "power gate command write failed.\n");
return ret;
@@ -611,18 +605,17 @@ EXPORT_SYMBOL_GPL(mei_hbm_pg);
*/
static int mei_hbm_stop_req(struct mei_device *dev)
{
- struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
- struct hbm_host_stop_request *req =
- (struct hbm_host_stop_request *)dev->wr_msg.data;
+ struct mei_msg_hdr mei_hdr;
+ struct hbm_host_stop_request req;
const size_t len = sizeof(struct hbm_host_stop_request);
- mei_hbm_hdr(mei_hdr, len);
+ mei_hbm_hdr(&mei_hdr, len);
- memset(req, 0, len);
- req->hbm_cmd = HOST_STOP_REQ_CMD;
- req->reason = DRIVER_STOP_REQUEST;
+ memset(&req, 0, len);
+ req.hbm_cmd = HOST_STOP_REQ_CMD;
+ req.reason = DRIVER_STOP_REQUEST;
- return mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+ return mei_write_message(dev, &mei_hdr, &req);
}
/**
@@ -636,9 +629,10 @@ static int mei_hbm_stop_req(struct mei_device *dev)
int mei_hbm_cl_flow_control_req(struct mei_device *dev, struct mei_cl *cl)
{
const size_t len = sizeof(struct hbm_flow_control);
+ u8 buf[len];
cl_dbg(dev, cl, "sending flow control\n");
- return mei_hbm_cl_write(dev, cl, MEI_FLOW_CONTROL_CMD, len);
+ return mei_hbm_cl_write(dev, cl, MEI_FLOW_CONTROL_CMD, buf, len);
}
/**
@@ -714,8 +708,9 @@ static void mei_hbm_cl_flow_control_res(struct mei_device *dev,
int mei_hbm_cl_disconnect_req(struct mei_device *dev, struct mei_cl *cl)
{
const size_t len = sizeof(struct hbm_client_connect_request);
+ u8 buf[len];
- return mei_hbm_cl_write(dev, cl, CLIENT_DISCONNECT_REQ_CMD, len);
+ return mei_hbm_cl_write(dev, cl, CLIENT_DISCONNECT_REQ_CMD, buf, len);
}
/**
@@ -729,8 +724,9 @@ int mei_hbm_cl_disconnect_req(struct mei_device *dev, struct mei_cl *cl)
int mei_hbm_cl_disconnect_rsp(struct mei_device *dev, struct mei_cl *cl)
{
const size_t len = sizeof(struct hbm_client_connect_response);
+ u8 buf[len];
- return mei_hbm_cl_write(dev, cl, CLIENT_DISCONNECT_RES_CMD, len);
+ return mei_hbm_cl_write(dev, cl, CLIENT_DISCONNECT_RES_CMD, buf, len);
}
/**
@@ -765,8 +761,9 @@ static void mei_hbm_cl_disconnect_res(struct mei_device *dev, struct mei_cl *cl,
int mei_hbm_cl_connect_req(struct mei_device *dev, struct mei_cl *cl)
{
const size_t len = sizeof(struct hbm_client_connect_request);
+ u8 buf[len];
- return mei_hbm_cl_write(dev, cl, CLIENT_CONNECT_REQ_CMD, len);
+ return mei_hbm_cl_write(dev, cl, CLIENT_CONNECT_REQ_CMD, buf, len);
}
/**
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index c9e01021eadf..e5e32503d4bc 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -382,7 +382,6 @@ const char *mei_pg_state_str(enum mei_pg_state state);
*
* @hbuf_depth : depth of hardware host/write buffer is slots
* @hbuf_is_ready : query if the host host/write buffer is ready
- * @wr_msg : the buffer for hbm control messages
*
* @version : HBM protocol version in use
* @hbm_f_pg_supported : hbm feature pgi protocol
@@ -467,12 +466,6 @@ struct mei_device {
u8 hbuf_depth;
bool hbuf_is_ready;
- /* used for control messages */
- struct {
- struct mei_msg_hdr hdr;
- unsigned char data[128];
- } wr_msg;
-
struct hbm_version version;
unsigned int hbm_f_pg_supported:1;
unsigned int hbm_f_dc_supported:1;
@@ -670,8 +663,7 @@ static inline size_t mei_hbuf_max_len(const struct mei_device *dev)
}
static inline int mei_write_message(struct mei_device *dev,
- struct mei_msg_hdr *hdr,
- unsigned char *buf)
+ struct mei_msg_hdr *hdr, void *buf)
{
return dev->ops->write(dev, hdr, buf);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl
2016-05-09 4:07 [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Tomas Winkler
2016-05-09 4:07 ` [char-misc-next 2/2] mei: drop wr_msg from the mei_dev structure Tomas Winkler
@ 2016-05-23 13:07 ` Winkler, Tomas
2016-05-23 21:28 ` Greg Kroah-Hartman
1 sibling, 1 reply; 5+ messages in thread
From: Winkler, Tomas @ 2016-05-23 13:07 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Usyskin, Alexander, linux-kernel
>
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> wr_ctrl waiters are none interruptible, so should be waken up with call to
> wake_up and not to wake_up_interruptible.
>
> This fixes commit:
> 7ff4bdd ("mei: fix waiting for wr_ctrl for corner cases.")
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Hi Greg,
I see this fix didn't make it to rc1, so currently the driver is somehow broken.
Can you please make an effort and include the fix in next rc?
Thanks
Tomas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl
2016-05-23 13:07 ` [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Winkler, Tomas
@ 2016-05-23 21:28 ` Greg Kroah-Hartman
2016-05-24 8:50 ` Winkler, Tomas
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2016-05-23 21:28 UTC (permalink / raw)
To: Winkler, Tomas; +Cc: Usyskin, Alexander, linux-kernel
On Mon, May 23, 2016 at 01:07:53PM +0000, Winkler, Tomas wrote:
> >
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > wr_ctrl waiters are none interruptible, so should be waken up with call to
> > wake_up and not to wake_up_interruptible.
> >
> > This fixes commit:
> > 7ff4bdd ("mei: fix waiting for wr_ctrl for corner cases.")
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>
> Hi Greg,
> I see this fix didn't make it to rc1, so currently the driver is somehow broken.
> Can you please make an effort and include the fix in next rc?
Ugh, didn't realize it was totally broken, the patch didn't really say
that, sorry. I'll queue it up once 4.7-rc1 is out.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl
2016-05-23 21:28 ` Greg Kroah-Hartman
@ 2016-05-24 8:50 ` Winkler, Tomas
0 siblings, 0 replies; 5+ messages in thread
From: Winkler, Tomas @ 2016-05-24 8:50 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Usyskin, Alexander, linux-kernel
>
> On Mon, May 23, 2016 at 01:07:53PM +0000, Winkler, Tomas wrote:
> > >
> > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > >
> > > wr_ctrl waiters are none interruptible, so should be waken up with
> > > call to wake_up and not to wake_up_interruptible.
> > >
> > > This fixes commit:
> > > 7ff4bdd ("mei: fix waiting for wr_ctrl for corner cases.")
> > >
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >
> > Hi Greg,
> > I see this fix didn't make it to rc1, so currently the driver is somehow
> broken.
> > Can you please make an effort and include the fix in next rc?
>
> Ugh, didn't realize it was totally broken, the patch didn't really say that, sorry.
> I'll queue it up once 4.7-rc1 is out.
>
There's a big difference between somhow broken and totally broken.
Just couldn't resist to parphrase Miracle Max :There's a big difference between mostly dead and all dead :)
Thanks
Tomas
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-24 8:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-09 4:07 [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Tomas Winkler
2016-05-09 4:07 ` [char-misc-next 2/2] mei: drop wr_msg from the mei_dev structure Tomas Winkler
2016-05-23 13:07 ` [char-misc-next 1/2] mei: don't use wake_up_interruptible for wr_ctrl Winkler, Tomas
2016-05-23 21:28 ` Greg Kroah-Hartman
2016-05-24 8:50 ` Winkler, Tomas
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).