lkml.org 
[lkml]   [2016]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RESEND PATCH v7 17/21] IB/hns: Add QP operations support
From
Date
Hi, Doug Ledford
In hns_roce_cmd_wait and hns_roce_cmd_poll, there are down&up
seminal operations,
So, there are not deadlock and conflict, right?

static int hns_roce_cmd_poll(struct hns_roce_dev *hr_dev, u64 in_param,
u64 *out_param, unsigned long in_modifier,
u8 op_modifier, u16 op, unsigned long timeout)
{
struct device *dev = &hr_dev->pdev->dev;
u8 __iomem *hcr = hr_dev->cmd.hcr;
unsigned long end = 0;
u32 status = 0;
int ret;

down(&hr_dev->cmd.poll_sem);

..// <snip>

up (&hr_dev->cmd.poll_sem);
return ret;
}

static int hns_roce_cmd_wait(struct hns_roce_dev *hr_dev, u64 in_param,
u64 *out_param, unsigned long in_modifier,
u8 op_modifier, u16 op, unsigned long timeout)
{
struct hns_roce_cmdq *cmd = &hr_dev->cmd;
struct device *dev = &hr_dev->pdev->dev;
struct hns_roce_cmd_context *context;
int ret = 0;

down(&cmd->event_sem);

..// <snip>

up (&cmd->event_sem);
return ret;
}

int __hns_roce_cmd(struct hns_roce_dev *hr_dev, u64 in_param, u64 *out_param,
unsigned long in_modifier, u8 op_modifier, u16 op,
unsigned long timeout)
{
if (hr_dev->cmd.use_events)
return hns_roce_cmd_wait(hr_dev, in_param, out_param,
in_modifier, op_modifier, op, timeout);
else
return hns_roce_cmd_poll(hr_dev, in_param, out_param,
in_modifier, op_modifier, op, timeout);
}



Thanks.

Regards
Wei Hu

On 2016/5/14 5:52, Doug Ledford wrote:
> On 05/09/2016 11:04 PM, Lijun Ou wrote:
>> +int __hns_roce_cmd(struct hns_roce_dev *hr_dev, u64 in_param, u64 *out_param,
>> + unsigned long in_modifier, u8 op_modifier, u16 op,
>> + unsigned long timeout);
>> +
>> +/* Invoke a command with no output parameter */
>> +static inline int hns_roce_cmd(struct hns_roce_dev *hr_dev, u64 in_param,
>> + unsigned long in_modifier, u8 op_modifier,
>> + u16 op, unsigned long timeout)
>> +{
>> + return __hns_roce_cmd(hr_dev, in_param, NULL, in_modifier,
>> + op_modifier, op, timeout);
>> +}
>> +
>> +/* Invoke a command with an output mailbox */
>> +static inline int hns_roce_cmd_box(struct hns_roce_dev *hr_dev, u64 in_param,
>> + u64 out_param, unsigned long in_modifier,
>> + u8 op_modifier, u16 op,
>> + unsigned long timeout)
>> +{
>> + return __hns_roce_cmd(hr_dev, in_param, &out_param, in_modifier,
>> + op_modifier, op, timeout);
>> +}
> This will make people scratch their head in the future. You are using
> two commands to map to one command without there being any locking
> involved. The typical convention for routine_1() -> __routine_1() is
> that the __ version requires that it be called while locked, and the
> version without a __ does the locking before calling it. That way a
> used can always know if they aren't currently holding the appropriate
> lock, then they6 call routine_1() and if they are, they call
> __routine_1() to avoid a deadlock. I would suggest changing the name of
> __hns_roce_cmd to hns_roce_cmd_box and completely remove the existing
> hns_roce_cmd_box inline, and then change the hns_roce_cmd() inline to
> directly call hns_roce_cmd_box() which will then select between
> event/poll command sends.
>


\
 
 \ /
  Last update: 2016-05-23 05:41    [W:0.077 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site