Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <80d1bdb9-f037-24b4-647b-df982fa0c981@intel.com>
Date:   Thu, 19 Oct 2023 23:50:03 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH net-next v3 00/11] devlink: retain error in struct
 devlink_fmsg
Content-Language: en-US
To:     Jiri Pirko <jiri@resnulli.us>
CC:     <netdev@vger.kernel.org>, "David S . Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Shannon Nelson <shannon.nelson@amd.com>,
        Michael Chan <michael.chan@broadcom.com>,
        Cai Huoqing <cai.huoqing@linux.dev>,
        "George Cherian" <george.cherian@marvell.com>,
        Danielle Ratson <danieller@nvidia.com>,
        Moshe Shemesh <moshe@nvidia.com>,
        Saeed Mahameed <saeedm@nvidia.com>,
        Ariel Elior <aelior@marvell.com>,
        Manish Chopra <manishc@marvell.com>,
        Igor Russkikh <irusskikh@marvell.com>,
        Coiby Xu <coiby.xu@gmail.com>, Simon Horman <horms@kernel.org>,
        Brett Creeley <brett.creeley@amd.com>,
        Sunil Goutham <sgoutham@marvell.com>,
        Linu Cherian <lcherian@marvell.com>,
        Geetha sowjanya <gakula@marvell.com>,
        Jerin Jacob <jerinj@marvell.com>,
        hariprasad <hkelam@marvell.com>,
        Subbaraya Sundeep <sbhatta@marvell.com>,
        Ido Schimmel <idosch@nvidia.com>,
        Petr Machata <petrm@nvidia.com>,
        Eran Ben Elisha <eranbe@nvidia.com>,
        Aya Levin <ayal@mellanox.com>,
        Leon Romanovsky <leon@kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20231018202647.44769-1-przemyslaw.kitszel@intel.com>
 <ZTEyzpat4we6f4kE@nanopsycho>
From:   Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <ZTEyzpat4we6f4kE@nanopsycho>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WGlQbXFKNksySzNzUDhxRDNYeFh2UWZPdE0zaUdyQnhOOTJkenFFbG9DTUdC?=
 =?utf-8?B?V01HbTRLVlhOR3pxWk1Sek5TZ2tUREQ5SXdvSmNGakM4eFM2eThoMjdQaUp0?=
 =?utf-8?B?SWhTVWxDejdreGJnbys4VUJ6UmxPUWl2blAxNlJpVzAyNDBRQmsrMnNyU0Jz?=
 =?utf-8?B?TWtiTnpnSUR6c2NacUwzQUlXUzNNdnBQSzlhUTdyNFRLbmRqdTJYa1ViUHkv?=
 =?utf-8?B?czVtNzdHT1NwT3dRSUYxdFhSSDJZaDRvbGRTYzNiRmVidVFLTk5MT0tEWlZT?=
 =?utf-8?B?Qlc5aVNaQnBxSTNDekRUSDlrbkNtZExKbFQvMDBhbFlhTTJRMnlab2hrUmoz?=
 =?utf-8?B?ZGRmeWszd1F2TnBnSmpRN2k5Mm1NK3NZMU5hWHExNGNSbzJJcWNXUGMwZytW?=
 =?utf-8?B?NG1aaW9RdHhVT2NQT3Q1Rno5MnkwdmRXb0hGVnFwWm9yQ2xnNGNyRG9pSHRC?=
 =?utf-8?B?Rmh2U2dMREdINmpUUlM0UEJOVjhkWmVIaGZLRG0waVZtOGlROFVZNlZleDdK?=
 =?utf-8?B?RmFjcWxBK2JzUFl1Wi9Oc2hWemZxYkJndmp5MGtObHpiN1IvTTYzN0R5S0gr?=
 =?utf-8?B?anZSaEZjV3RHenMwSHN5c210L0tKbWlvcmplWE5oTGZncjlnaHNCaXFPa1hl?=
 =?utf-8?B?RkUyOCttNDlyRTFxZU1Fdy83V0ErRUlDajh0VHRiaDM3REJKU1JZZFVWb1Ex?=
 =?utf-8?B?V0t6QW5KaU5GRlROM2VUNmhmM1o5YnMrYXdKQlB2M04xZXNWSkxMZTA1OE1J?=
 =?utf-8?B?d3VHdzRCMzhkZUNVbnpGV2szSmhYN08zaVNlOFZvbk5icU80cEZGZ3BCZkxw?=
 =?utf-8?B?Ky9QYkdEUmNzeGdjd0N4V012MmZhaUI0bW8vOTRRdlU1dFJNcVE0VDNKWkFr?=
 =?utf-8?B?VlMvcDErSHA1cHBvQkpGRlozUnpmdmN4TkF4OUE2THVlWUlBbXpoZWRvY0JB?=
 =?utf-8?B?VkpZNnNPS2x1UFBLQmkxckowd1FOWnJRMm1iVDhxcFZTUkVONTV0Yks1VFNu?=
 =?utf-8?B?V3AxSlpKT1RmaStiZW9oUXV3ckpLWExPZ09lQmRHQVhINUlIekU1YVduVG1B?=
 =?utf-8?B?VS9WQ1JoRHM1MjdDYXVOMXE3cjRGNHZnUXF2a1ZCZjcrWVRpOHp6TGdZRTg1?=
 =?utf-8?B?eVltZWx4UkZyaEtHWDFPcFdXNk5BRC90VVA5L2paUU1Dd0VOSUNaczhzd2dE?=
 =?utf-8?B?VE1oSHkvU2pqYzJZYS8zVklVSCt0ZE1SWUZhT09LbjFnRGt5Q2Z4bjlDajVK?=
 =?utf-8?B?Tmx2YzFYM1F3ZXp2a1c5WmhEeUxlODRiOUpRVW9EeVBwVUhHa2xFT1Zlcy93?=
 =?utf-8?B?MXNMRnc5SDhYWGVpZ2FFMFlsRFBwL1k4djhxV3UrMlhnRzhVc2praldWZUhs?=
 =?utf-8?B?Ty9QNXdIMWlnSldDZGpMcnJRY2owQkNXVnBXaHZoTWpMWXgzMUdjM1NMZGY1?=
 =?utf-8?B?K2xNaEk1N2hVMExMQ3lYNWZlcjFjMDlNVExubnpxdWUxWGlvMXM4S3hmZURF?=
 =?utf-8?B?UmVnRTJSeTJNUy9ZSFdxZUdlVHZQTHhHaUYvb2xra2dKY3lhYVZkd01ZckQy?=
 =?utf-8?B?VmdtWjVWL1M3Ri9SWHdBY2lHWEhBNTRYVXg3MUZWZXBCN01ibEs3bnhYVmpZ?=
 =?utf-8?B?MkQreTNIVW5oc1dMKzMyUVRjU3hDUTFiZVBXeUxra2ZGU3puOFhkWVQ2Rmtw?=
 =?utf-8?B?RHg2Um9OVVlNOVFLS2lVdEVmbzJWT2NDeWV5OGdLZzg0UVlKa0ZyV2ZVbXE2?=
 =?utf-8?B?dkEyM2d6Wlllem5YWTdWc2VHWE5vM1pSTTNXSlh4VzNLYm5LZ2JoNGxyR2lK?=
 =?utf-8?B?VTUvL3B3Mmd4RzVEY1B5VHR0VGQvZTBPYU9HYlIxbnFiUzVCNk4xMCtCVENq?=
 =?utf-8?B?N0QvZXJ0N3cyMHRxRjM2Y2c0cE1MWXo5cGtWclhqampoRzV2NFd6dkFpcThU?=
 =?utf-8?B?U0ovQXFGck9Ob1ZxWkMxeHFSUmg0dDRBLzJCbG5WNWZCaU9heGNJQXV4RWJZ?=
 =?utf-8?B?YmhvZGVWK2dsajU3TzBZQWpGWEdQRzRCNkhRLzFNTEVxcENpeVJpa1BQYXc2?=
 =?utf-8?B?U3BVa3AzVDNZSWhKMUVyR0wrV21qb2dUdE0vZzFDKzlzOE14bTlYcDI3VjVJ?=
 =?utf-8?B?dmVFWVN3MTgxdXY0LzJWbklmdEc0Y1doQ0NhemtqOVFUaFVBeHdoUXBONTJT?=
 =?utf-8?B?Wmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dbc205d8-e1f2-4c04-cdd9-08dbd0ed618b
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3672.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2023 21:50:16.8242
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rLkEHFnLa9WBNsx6pBHzg/zD9kzj5qkWk4MDfMy1lenubcrUZ+rhbJijhA1g8Oe9rcAgLCOCDjG1Egu4E/YQZp3xJIkvyId/VtloZh5E9Sg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6058
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 19 Oct 2023 14:50:37 -0700 (PDT)

On 10/19/23 15:44, Jiri Pirko wrote:
> Wed, Oct 18, 2023 at 10:26:36PM CEST, przemyslaw.kitszel@intel.com wrote:
>> Extend devlink fmsg to retain error (patch 1),
>> so drivers could omit error checks after devlink_fmsg_*() (patches 2-10),
>> and finally enforce future uses to follow this practice by change to
>> return void (patch 11)
>>
>> Note that it was compile tested only.
>>
>> bloat-o-meter for whole series:
>> add/remove: 8/18 grow/shrink: 23/40 up/down: 2017/-5833 (-3816)
>>
>> changelog:
>> v3: set err to correct value, thanks to Simon and smatch
>>     (mlx5 patch, final patch);
> 
> 2 nits:
> - always better to have per-patch changelog so it is clear for the
>    reviewers what exactly did you change and where.

agree that adding changelog also to patches would be better

> - if you do any change in a patch, you should drop the
>    acked/reviewed/signedoff tags and get them again from people.

Here opinions differ widely, and my understanding was "it depends".
Noted to always drop yours.

On one side you have just rebased patches (different context of review),
then with trivial conflict fixed, then with minor change (as here,
0-init to retval, those are things that I believe most reviewers don't
want to look again at.
Then patches with some improvement that somebody suggested (as reviewer,
I want to see what could have been done better and I didn't notice).

In the above cases there is both time to react and no much harm keeping
RB. Things I think that most reviewers and maintainers would like to
drop RB start at "significant changes such as 'business' logic change",
which is of course a fuzzy line.

Always dropping is an easy solution, perhaps too easy ;)

> 
> that being said:
> set-
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> 

Thank you!