Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bdb69723-269d-3c1f-2501-7db0e6246890@suse.com>
Date:   Sun, 2 Oct 2022 15:21:34 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH] scsi: core: Add BLIST_NO_ASK_VPD_SIZE for some VDASD
Content-Language: en-US
To:     Bart Van Assche <bvanassche@acm.org>,
        Lee Duncan <leeman.duncan@gmail.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Martin Wilck <mwilck@suse.com>, Hannes Reinecke <hare@suse.de>
References: <20220928181350.9948-1-leeman.duncan@gmail.com>
 <11a582f0-723c-95e1-0e44-0a19e1a8a9a8@acm.org>
From:   Lee Duncan <lduncan@suse.com>
In-Reply-To: <11a582f0-723c-95e1-0e44-0a19e1a8a9a8@acm.org>
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?ck1YWDV0Zk8xaHg5VGZyZWI0UFV3KzNlQW1rRkRIdldGaDRVSW5Td0hBZFpu?=
 =?utf-8?B?eEI5dUwrb0laenNRNzZBOWtzWWtlTjhCMjhsOHI1YTcwVTQ2YmpNVGVwbWN6?=
 =?utf-8?B?MW9RTENaZzVmNkJuQXN0UW9Bb1kzczhOcjI4OFMzaStUMHNlK2haVU1FZERu?=
 =?utf-8?B?R1h4eGdFWWNuK0xzZW05YVEwQzFGU0JnWDlySC9uOGNPRTd4SWtVT0FLd2ZR?=
 =?utf-8?B?L0NDSGVKbkhXUWJaTGlhRlM5dnVCWkR2REJMTjJ3UkNxVEVUWHN2RGoyR0JT?=
 =?utf-8?B?YlJBK08rdmVjdXZiTGZBNVZnTWJURHJWMzlka3MrSG1PS0FEZXBCdVJpV09V?=
 =?utf-8?B?Zy9RZnFsaWI5WUlxbWZ5L1lSV1lIcFBwQjNwb3JGOEFIT1RQSzB0WEJCWThR?=
 =?utf-8?B?NjFtV05RV0QxMThlY285MUUyb3JPTFFvRXl3NDBmeHZob3RkdS9ub2U0dW1N?=
 =?utf-8?B?QTBNbFp6NEY0SnNIU0hFRWR3SU9PM0tucEdFY2Y3YVJPYXlWMFZOek1kWGg2?=
 =?utf-8?B?cmJFNVZRRk1RN2FxanhoUGxydEFCQklBOVozcEllKzJzcUhFNlRwSjVlVm0y?=
 =?utf-8?B?b1FpVi9QZjM5dkJMc25iaVJaTTdKc2UvL0dScnlUL1ZZYTM1bk90WFkvaFow?=
 =?utf-8?B?akE5L0tSa25oTHc5VWIvb0hZb2ZUNGtHckFzVEh3ODVyZUk4RGhOdW03NGNS?=
 =?utf-8?B?NEtxTnFIbUZldFBsSitFVVhmYVNXeTYrU005ZjJrUERMc20yS245Z0kvU1l0?=
 =?utf-8?B?c1JCSTY5WDdPVVJWWE13L0FiOU1VNkJGbmZkeG9kUDl0RjRLRjB3ZXhiMGhS?=
 =?utf-8?B?eEUxdjlTbWNBMGk3a01aelNpZzNTZkdIUXpNd0dtU3FIWEV6ejgvdHVVaU12?=
 =?utf-8?B?TkV6dldnQTRRN3FLWnllbVBYYmNybkZDNVlvNEw5S05YdGJSRWh5UTlsQXV0?=
 =?utf-8?B?ZDNzTWRhTnA2anlId3hTWTY5alZzYnFld0J4OGpTc1N2NmNLMXczUUpIMVVy?=
 =?utf-8?B?a1Vwb3FnK05qK3hoWCtaYllFb3R6UnhLVW80R1FSc0lONklIWjViYTJoa3Zn?=
 =?utf-8?B?WjhORDhIVElzZjZxdGlJTjVpTnU0c1VITnNIL1hWWGRmcm9pamFQWnFhbFV3?=
 =?utf-8?B?TXV1WnRvdzZSSDJrMFA2YWZQWURPdjE4WVYzR2E5UXBsNHMyVkRHNDQ3c1Fz?=
 =?utf-8?B?a24wb3IwMTRsK1ZGQk5VTVVmUEk3aVg2RGJiOHNtWmJheGlkTTV5TUVnSjJs?=
 =?utf-8?B?T3N4dlZpODJOWjlqeEJVckJUb3pRNGFBOUNBNENLVi91QzlUWHNVNFZ3VHhY?=
 =?utf-8?B?RFh5Ym1pUEZVTlBDTUhoc0NMWWlyQW9ubU1PeWtkYVlLUlZLWFZRZVZmbnlE?=
 =?utf-8?B?eVAwVmN4ZmlwUkFYMGQ5VThIbng3djE2R1E3MVJjWDF1VWF4QVUrRHF4SVpQ?=
 =?utf-8?B?akJxc29FdFdrUmo4V0JBZ2ZJNUxHWDBzaTJSeFRETUNNZkY0YkRaSGM5S2Fs?=
 =?utf-8?B?V1EwRUx3c2ptWS9oRXZvZTc3TmdtNjFOSWdKYkVCaGlGYnB6eVBzSkMrK25h?=
 =?utf-8?B?UzVsQW1OY0FNZGI3SnVUa2VOQnZnZnpKems2NHJCU09VUlR3MHhtYjdQMCtG?=
 =?utf-8?B?YkNlblJicFM0NzBMRmYxcE9lRGRwSFVPenJ4REQxZXR0ejRHNXpRUUd6TWh2?=
 =?utf-8?B?S3BlSGdNak02c1l3b1RBNTNyY1lNMHZteWM2NlBoT3RPZy9EcldEV3FwOXcx?=
 =?utf-8?B?cE9rY09idEkxd1FOdm4yQ2VHMWxMMk0wNU5NTERYYnJMV0lHcjdOQWtzK1Fp?=
 =?utf-8?B?RlNxT0lWelQ4QU9WOW16WSs3RUYrQjZJQ0Vab1VTS2tsYkY0a3F5VnNReXg1?=
 =?utf-8?B?Y0p6bkVXRXVKMjJaZTdML0tkS3NBUVZvbkVtZkRVbTY3QmN5YWJybUtvL1No?=
 =?utf-8?B?OFRiVmF2VWtxTVFrc0R0OTJqUm1zTXExR1ZEOWxzbkFxajY4TXg4Z2tmMFh6?=
 =?utf-8?B?SVZ2N2dXMWZRZ01LNkNya3lNU0crUnRTMHZodHpaaGtKNjJhZFhnUHJTejcw?=
 =?utf-8?B?TGQxdk1iNU9LWEQvcHF3c2RzRlVnVW1lWC9EdU5yMFJVd0taV3laMFRoRHUv?=
 =?utf-8?Q?uab08MwlPoe8WOe46ebvObyoF?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4268e265-67c8-4f06-8c90-08daa4c47a6a
X-MS-Exchange-CrossTenant-AuthSource: AM5PR04MB3089.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Oct 2022 22:21:40.1944
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bOEui0IhI1/GDa8GvzGhyaHShQ95p38Izl2SIHudSynsxM7ogVXr66IUJlm/FriMSELvp2/8E3NgWms0qBjeYw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB7360
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/2/22 14:16, Bart Van Assche wrote:
> On 9/28/22 11:13, Lee Duncan wrote:
>> From: Lee Duncan <lduncan@suse.com>
>>
>> Some storage, such as AIX VDASD (virtual storage) and IBM 2076
>> (front end) do not like the recent commit:
>>
>> commit c92a6b5d6335 ("scsi: core: Query VPD size before getting full 
>> page")
>>
>> That commit changed getting SCSI VPD pages so that we now read
>> just enough of the page to get the actual page size, then read
>> the whole page in a second read. The problem is that the above
>> mentioned hardware returns zero for the page size, because of
>> a firmware error. In such cases, until the firmware is fixed,
>> this new black flag says to revert to the original method of
>> reading the VPD pages, i.e. try to read as a whole buffer's
>> worth on the first try.
>>
>> Fixes: c92a6b5d6335 ("scsi: core: Query VPD size before getting full 
>> page")
> 
> Hi Lee,
> 
> If we introduce a blacklist flag to skip querying the VPD page size then 
> we will have to find all SCSI devices that do not handle querying the 
> VPD page size correctly. Has it been considered instead of introducing a 
> blacklist flag to not use the reported VPD page size if the device 
> reports that the VPD page size is zero? I am not aware of any VPD pages 
> for which zero is a valid size.
> 
> Thanks,
> 
> Bart.

Hi Bart:

The problem with the broken firmware in my case is that it reports a 
size of zero, but it actually has the data! So the "size" returned for 
this one VPD page is just wrong. And I haven't researched it yet, but I 
assume that this hardware returned the failing page in question as a 
page it supported. In other words, you can't count on this hardware to 
report correctly. [I will check and update this email thread if this is 
wrong.]

This broken firmware was never an issue before commit c92a6b5d6335, 
since we used to just try to read 255 bytes, expecting that we would get 
back 255 or less. This worked almost all the time -- except for buggy 
hardware!

I suspect there isn't many pieces of hardware that return zero length 
incorrectly, and that if such hardware shoes up then they'll be able to 
use this flag to work around it.

So, for my hardware use case, if I add my commit, the VPD page shows up 
in sysfs, and before my commit no VPD page showed up. [Also, reverting 
commit c92a6b5d6335 made the VPD page show up, as a side note.]

Lastly, as for pages that might validly return size zero, Hannes seems 
to think some of the older hardware (under the older standards) returned 
zero as a valid page size for some VPD pages. For this reason I decided 
to not use a simpler approach of just trying to read the VPD page with a 
size of 255 if the "read length" returned zero (as in this case), i.e. 
since Hannes thinks some hardware might legitimately do this.

-- 
Lee