From: Leon Romanovsky <l...@kernel.org>
Sent: Tuesday, October 3, 2017 4:26 PM
>On Tue, Oct 03, 2017 at 11:54:56AM +0300, Michal Kalderon wrote:
>> For iWARP unaligned MPA flow, a slowpath event of flushing an
>> MPA connection that entered an unaligned state is required.
>> The flush ramrod is received on the ll2 queue, and a pre-registered
>> callback function is called to handle the flush event.
>>
>> Signed-off-by: Michal Kalderon <michal.kalde...@cavium.com>
>> Signed-off-by: Ariel Elior <ariel.el...@cavium.com>
>> ---
>> drivers/net/ethernet/qlogic/qed/qed_ll2.c | 40
>> +++++++++++++++++++++++++++++--
>> include/linux/qed/qed_ll2_if.h | 5 ++++
>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
>> b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
>> index 8eb9645..047f556 100644
>> --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
>> +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
>> @@ -423,6 +423,41 @@ static void qed_ll2_rxq_parse_reg(struct qed_hwfn
>> *p_hwfn,
>> }
>>
>> static int
>> +qed_ll2_handle_slowpath(struct qed_hwfn *p_hwfn,
>> + struct qed_ll2_info *p_ll2_conn,
>> + union core_rx_cqe_union *p_cqe,
>> + unsigned long *p_lock_flags)
>> +{
>> + struct qed_ll2_rx_queue *p_rx = &p_ll2_conn->rx_queue;
>> + struct core_rx_slow_path_cqe *sp_cqe;
>> +
>> + sp_cqe = &p_cqe->rx_cqe_sp;
>> + if (sp_cqe->ramrod_cmd_id != CORE_RAMROD_RX_QUEUE_FLUSH) {
>> + DP_NOTICE(p_hwfn,
>> + "LL2 - unexpected Rx CQE slowpath
>> ramrod_cmd_id:%d\n",
>> + sp_cqe->ramrod_cmd_id);
>> + return -EINVAL;
>> + }
>> +
>> + if (!p_ll2_conn->cbs.slowpath_cb) {
>> + DP_NOTICE(p_hwfn,
>> + "LL2 - received RX_QUEUE_FLUSH but no callback was
>> provided\n");
>> + return -EINVAL;
>> + }
>> +
>> + spin_unlock_irqrestore(&p_rx->lock, *p_lock_flags);
>
>Interesting, you are unlock the lock which was taken in upper layer.
>It is not actual error, but chances to have such error are pretty high
>(for example, after refactoring).
Thanks. Ensuring that the lock will only be unlocked inside the calling
function would make
the calling function long and less readable.
The risk exists, but I think the fact that p_lock_flags is passed as parameter
should
give a strong indication in the future that lock should be handled delicately.
