From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753681AbcEMVwK (ORCPT ); Fri, 13 May 2016 17:52:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40312 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbcEMVwI (ORCPT ); Fri, 13 May 2016 17:52:08 -0400 Subject: Re: [RESEND PATCH v7 17/21] IB/hns: Add QP operations support To: Lijun Ou , sean.hefty@intel.com, hal.rosenstock@gmail.com, davem@davemloft.net, jeffrey.t.kirsher@intel.com, jiri@mellanox.com, ogerlitz@mellanox.com References: <1462849483-67927-1-git-send-email-oulijun@huawei.com> <1462849483-67927-18-git-send-email-oulijun@huawei.com> Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, gongyangming@huawei.com, xiaokun@huawei.com, tangchaofei@huawei.com, haifeng.wei@huawei.com, yisen.zhuang@huawei.com, yankejian@huawei.com, charles.chenxin@huawei.com, linuxarm@huawei.com From: Doug Ledford Openpgp: id=AE6B1BDA122B23B4265B1274B826A3330E572FDD; url=pgp.mit.edu X-Enigmail-Draft-Status: N1110 Organization: Red Hat, Inc. Message-ID: Date: Fri, 13 May 2016 17:52:04 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1462849483-67927-18-git-send-email-oulijun@huawei.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="jveuGMgECj8uME3rkPil7mqHGxXbvOcqB" X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 13 May 2016 21:52:07 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --jveuGMgECj8uME3rkPil7mqHGxXbvOcqB Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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_par= am, > + 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. --=20 Doug Ledford GPG KeyID: 0E572FDD --jveuGMgECj8uME3rkPil7mqHGxXbvOcqB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBCAAGBQJXNkyEAAoJELgmozMOVy/dfGkP/1eSnxCzxbLrms/NZCBfALNB BHt5TYgcJF8qkQqirubFt6YS51zJdOTO5xMqyv3ySOLMggG6fVRVjWoC6WxRGZtT oFSNiv8JBgpYG0w7e5u49jdaDQT33hql6ndpfg+vWO3WXT7YAk+mS0+9NFARFbmC QqoZTCh1lb3Bg3/fwynPY7SdzUfOt98R+nLUGOpgqssNGG0pKEUXbZbC3T2bd/P+ p2E+FlmBaJy/5H2evRcQ3C4bC8nSQVhvhocEQTh2DCF80+wa0Yoc271yZgSSFXy3 /mUdD4tgOAyTUOmbrCNMwXzXIgbMiw+h8QB74bN2D1HqLfKTyStkPwrK/Nny9t44 8TcmiSL8RzTYc7X2jWuZKp6EE93fiNF1cGoT8fEY5Q0V08Ll9YaNiIX1beUncjGq iC0Zdbz9z17jY0TAPWQsbgRzhFnlnxC7xQ8ZYuWQnU04ZqSHnANznzFVEZxEPnu7 M1oFYL1GO/jPuNTv3XV12xOPkAuL4xrn2NGkf25FNSCkBr9vQRkwmPBeQ77vVkuU Xvk/j7Mv0reqvmbVLPKVuit1fZl1xfJ765cyjKVpaiDoAS3tesOZhwcc8h81ycdM Rk+iwxXI6U7mt6V897bfCHrwkdKbk2plXUqdGE32cgsGJrxJkLLnERHFhMd2lXGs 4icFUfNSvSaAckG60JYe =CRdL -----END PGP SIGNATURE----- --jveuGMgECj8uME3rkPil7mqHGxXbvOcqB--