-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
crypto: fix an issue of multiple tasks using the same qp #6529
Conversation
@@ -606,19 +606,25 @@ static enum hisi_drv_status qm_cqc_cfg(struct hisi_qp *qp) | |||
struct hisi_qp *hisi_qm_create_qp(struct hisi_qm *qm, uint8_t sq_type) | |||
{ | |||
struct hisi_qp *qp = NULL; | |||
int cur_idx = 0; | |||
uint32_t i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
= 0;
else | ||
qm->qp_idx++; | ||
for (i = 0; i < qm->qp_num; i++) { | ||
cur_idx = (qm->qp_dix + i) % qm->qp_num; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't even compile. Do you build and test properly before submitting code for review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, there are significant differences between our local code and the code in the library, so there are certain compilation issues. It is difficult to test now. After patches of qm, we will modify all the driver code according to the style of comments. During this, I will check my patch more carefully. Thanks!
|
@jforissier I have modified my commit according to the comments. |
qm->qp_idx = cur_idx + 1; | ||
break; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
panic if all used? or return NULL? This makes last tested index being used.
unlock/wait/relock, cond_var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit header could metion hisilicon:
drivers: crypto: hisilicon: fix multiple tasks using the same qp
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
with 2 suggestions.
qm->qp_idx = cur_idx + 1; | ||
break; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, sorry.
qm->qp_idx++; | ||
for (i = 0; i < qm->qp_num; i++) { | ||
cur_idx = (qm->qp_idx + i) % qm->qp_num; | ||
if (qm->qp_array[cur_idx].used == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer if (!qm->qp_array[cur_idx].used)
|
||
qp = &qm->qp_array[qm->qp_idx]; | ||
qp = &qm->qp_array[cur_idx]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use qp = qm->qp_array + cur_idx;
…ame qp Flag in the qp structure is used to indicate whether the qp is occupied.The new task can find an unused qp and use it. Fixes: c7f9abc ("drivers: implement HiSilicon Queue Management (QM) module") Signed-off-by: Zexi Yu <yuzexi@hisilicon.com> Acked-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
@jforissier @etienne-lms All comments has been solved. |
Flag in the qp structure is used to indicate whether the qp is occupied.The new task can find an unused qp and use it.
Fixes: c7f9abc ("drivers: implement HiSilicon Queue Management (QM) module")