From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67F21C433FE for ; Tue, 4 Jan 2022 13:52:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233846AbiADNwu (ORCPT ); Tue, 4 Jan 2022 08:52:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45526 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229535AbiADNwu (ORCPT ); Tue, 4 Jan 2022 08:52:50 -0500 Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::34]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C4295C061761; Tue, 4 Jan 2022 05:52:49 -0800 (PST) Received: by angie.orcam.me.uk (Postfix, from userid 500) id 0F67392009C; Tue, 4 Jan 2022 14:52:48 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 0BBB192009B; Tue, 4 Jan 2022 13:52:48 +0000 (GMT) Date: Tue, 4 Jan 2022 13:52:47 +0000 (GMT) From: "Maciej W. Rozycki" To: "Martin K. Petersen" cc: Douglas Gilbert , Khalid Aziz , "James E.J. Bottomley" , Christoph Hellwig , Nix , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 3/3] scsi: Set allocation length to 255 for ATA Information VPD page In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Martin, > > So I do have all the reasons to conclude this value has indeed been > > arbitrarily chosen, don't I? > > The SAT spec defines the contents of the first part of the page. The > trailing 512 bytes are defined in the ATA spec. I think that would best be reflected in code somehow as lone `64' doesn't say anything really. > > If you insist that the value of 64 stay, then please come up with a > > suitable macro name to define along with SCSI_VPD_PG_LEN that reflects > > the meaning of 64 in this context and I'll be happy to update 3/3 > > accordingly, but please explain why the value of 64 is any better than > > 255 here and the inconsistency with the buffer size justified. > > Can please you try the following patch? I have tried it and it's neutral, that is with 1/3 applied the HBA still works and with 1/3 removed it still breaks (2/3 and 3/3 obviously don't build anymore). Unsurprisingly, as it's the call to `scsi_get_vpd_page' rather than `scsi_get_vpd_buf' that causes an issue here. I think the latter function isn't called in my setup, and it's not clear to me anymore after so long why I didn't poke at it. It looks to me like the `retry_pg' code there can be gone now with your update in place as it duplicates buffer sizing, and with that included it'll be a nice clean-up. NB you'll need to adjust drivers/scsi/mpt3sas/mpt3sas_scsih.c accordingly if we are to move forward with this change, as it's another user of the SCSI_VPD_PG_LEN macro. Given what has been said in the discussion so far would you consider 2/3 and 3/3 unnecessary? In the course of verifying your change I have looked through our code again and found that inline magic numbers are there in several though not all places where `scsi_get_vpd_page' gets called, so I think it would make sense to get rid of them all at once with a single self-contained change. Thank you for your input and the extra fix. Maciej