Skip to content

Commit

Permalink
Fix crash in signal code after forking.
Browse files Browse the repository at this point in the history
Thanks to user i10a for reporting this issue.
After a fork the engine is not initialised in the child until the
first call of get_next_inst(). qat_use_signals() also relies on the
engine being intialised in order for the application to know it can
send a signal to the polling thread. This change ensures that
qat_use_signals() will also initialise the engine after a fork, if
necessary.
get_next_inst() and qat_use_signals() have also been optimised so
that they do not use locking in the case where the engine has already
been initialised, which is most of the time.

Change-Id: I5a1d088f7f6cbea8cef240a15a05ae2df263fa54
Signed-off-by: Steve Linsell <stevenx.linsell@intel.com>
  • Loading branch information
stevelinsell committed Dec 20, 2017
1 parent ed4989b commit 871a728
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 27 deletions.
79 changes: 53 additions & 26 deletions e_qat.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@
#include "icp_sal_poll.h"
#include "qat_parseconf.h"

/* Macro used to handle errors in qat_engine_ctrl() */
#define BREAK_IF(cond, mesg) \
if(unlikely(cond)) { retVal = 0; WARN(mesg); break; }


/* Qat engine id declaration */
const char *engine_qat_id = "qat";
const char *engine_qat_name =
Expand Down Expand Up @@ -149,10 +144,38 @@ sigset_t set = {{0}};
pthread_t timer_poll_func_thread = 0;
int cleared_to_start = 0;

static inline int qat_use_signals_no_engine_start(void)
{
return (int)timer_poll_func_thread;
}

int qat_use_signals(void)
{
return (int)timer_poll_func_thread;
/* We check engine_inited outside of a mutex here because it is more
efficient and we are only interested in the state if it hasn't been
initialised. The usual case is that the engine will have been
initialised and we can carry on without locking. If the engine hasn't
been initialised then there will be a further check within
qat_engine_init inside a mutex to prevent a race condition. */

if (unlikely(!engine_inited)) {
ENGINE* e = ENGINE_by_id(engine_qat_id);

if (e == NULL) {
WARN("Function ENGINE_by_id returned NULL\n");
return 0;
}

if (!qat_engine_init(e)) {
WARN("Failure in qat_engine_init function\n");
ENGINE_free(e);
return 0;
}

ENGINE_free(e);
}

return qat_use_signals_no_engine_start();
}


Expand All @@ -174,7 +197,6 @@ static inline void incr_curr_inst(void)
CpaInstanceHandle get_next_inst(void)
{
CpaInstanceHandle instance_handle = NULL;
ENGINE* e = NULL;

if (1 == enable_instance_for_thread) {
instance_handle = pthread_getspecific(qatInstanceForThread);
Expand All @@ -186,22 +208,27 @@ CpaInstanceHandle get_next_inst(void)
}
}

e = ENGINE_by_id(engine_qat_id);
if(e == NULL) {
WARN("Function ENGINE_by_id returned NULL\n");
instance_handle = NULL;
return instance_handle;
}
/* See qat_use_signals() above for more info on why it is safe to
check engine_inited outside of a mutex in this case. */
if (unlikely(!engine_inited)) {
ENGINE* e = ENGINE_by_id(engine_qat_id);

if (e == NULL) {
WARN("Function ENGINE_by_id returned NULL\n");
instance_handle = NULL;
return instance_handle;
}

if (!qat_engine_init(e)) {
WARN("Failure in qat_engine_init function\n");
instance_handle = NULL;
ENGINE_free(e);
return instance_handle;
}

if(!qat_engine_init(e)){
WARN("Failure in qat_engine_init function\n");
instance_handle = NULL;
ENGINE_free(e);
return instance_handle;
}

ENGINE_free(e);

/* Each call to get_next_inst will iterate through the array of instances
if an instance was not retrieved already from thread specific data. */
if (instance_handle == NULL)
Expand Down Expand Up @@ -246,7 +273,7 @@ int qat_engine_init(ENGINE *e)
int ret_pthread_sigmask;

pthread_mutex_lock(&qat_engine_mutex);
if(engine_inited) {
if (engine_inited) {
pthread_mutex_unlock(&qat_engine_mutex);
return 1;
}
Expand Down Expand Up @@ -676,15 +703,15 @@ qat_engine_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void))

case QAT_CMD_SET_CRYPTO_SMALL_PACKET_OFFLOAD_THRESHOLD:
#ifndef OPENSSL_ENABLE_QAT_SMALL_PACKET_CIPHER_OFFLOADS
if(p) {
if (p) {
char *token;
char str_p[1024];
char *itr = str_p;
strncpy(str_p, (const char *)p, 1024);
while((token = strsep(&itr, ","))) {
while ((token = strsep(&itr, ","))) {
char *name_token = strsep(&token,":");
char *value_token = strsep(&token,":");
if(name_token && value_token) {
if (name_token && value_token) {
retVal = qat_pkt_threshold_table_set_threshold(
name_token, atoi(value_token));
} else {
Expand All @@ -707,7 +734,7 @@ qat_engine_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void))
retVal = 0;
break;
}
if(!retVal) {
if (!retVal) {
QATerr(QAT_F_QAT_ENGINE_CTRL, QAT_R_ENGINE_CTRL_CMD_FAILURE);
}
return retVal;
Expand All @@ -725,7 +752,7 @@ int qat_engine_finish_int(ENGINE *e, int reset_globals)

pthread_mutex_lock(&qat_engine_mutex);
keep_polling = 0;
if (qat_use_signals()) {
if (qat_use_signals_no_engine_start()) {
if (pthread_kill(timer_poll_func_thread, SIGUSR1) != 0) {
WARN("pthread_kill error\n");
QATerr(QAT_F_QAT_ENGINE_FINISH_INT, QAT_R_PTHREAD_KILL_FAILURE);
Expand All @@ -735,7 +762,7 @@ int qat_engine_finish_int(ENGINE *e, int reset_globals)

if (qat_instance_handles) {
for (i = 0; i < qat_num_instances; i++) {
if(instance_started[i]) {
if (instance_started[i]) {
status = cpaCyStopInstance(qat_instance_handles[i]);

if (CPA_STATUS_SUCCESS != status) {
Expand Down
2 changes: 1 addition & 1 deletion e_qat.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@

/* Macro used to handle errors in qat_engine_ctrl() */
#define BREAK_IF(cond, mesg) \
if(unlikely(cond)) { retVal = 0; WARN(mesg); break; }
if (unlikely(cond)) { retVal = 0; WARN(mesg); break; }

#define QAT_QMEMFREE_BUFF(b) \
do { \
Expand Down

0 comments on commit 871a728

Please sign in to comment.