On 2022/02/02 21:25, Anton Lundin wrote:
> On 02 February, 2022 - Damien Le Moal wrote:
>
>> On 2/2/22 19:05, Anton Lundin wrote:
>>> Back in 06f6c4c6c3e8 ("ata: libata: add missing ata_identify_page_supported() calls")
>>> a read of ATA_LOG_DIRECTORY page was added. This caused the
>>> SATADOM-ML 3ME to lock up.
>>>
>>> In 636f6e2af4fb ("libata: add horkage for missing Identify Device log")
>>> a flag was added to cache if a device supports this or not.
>>>
>>> This adds a blacklist entry which flags that these devices doesn't
>>> support that call and shouldn't be issued that call.
>>>
>>> Cc: stable@xxxxxxxxxxxxxxx # v5.10+
>>> Signed-off-by: Anton Lundin <glance@xxxxxxxxxx>
>>> Depends-on: 636f6e2af4fb ("libata: add horkage for missing Identify Device log")
>>
>> I do not think so. See below.
>>
>>> ---
>>> drivers/ata/libata-core.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>>> index 87d36b29ca5f..e024af9f33d0 100644
>>> --- a/drivers/ata/libata-core.c
>>> +++ b/drivers/ata/libata-core.c
>>> @@ -4070,6 +4070,13 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
>>> { "WDC WD3000JD-*", NULL, ATA_HORKAGE_WD_BROKEN_LPM },
>>> { "WDC WD3200JD-*", NULL, ATA_HORKAGE_WD_BROKEN_LPM },
>>>
>>> + /*
>>> + * This sata dom goes on a walkabout when it sees the
>>> + * ATA_LOG_DIRECTORY read request so ensure we don't issue such a
>>> + * request to these devices.
>>> + */
>>> + { "SATADOM-ML 3ME", NULL, ATA_HORKAGE_NO_ID_DEV_LOG },
>>
>> This flag only disables trying to access the identify device log page,
>> it does *not* avoid access to the log directory log page in general. The
>> log directory will still be consulted for other log pages beside the
>> identify device log page, from any function that calls
>> ata_log_supported() (e.g. ata_dev_config_ncq_send_recv() and
>> ata_dev_config_ncq_non_data())
>
> Non of those code paths are called for this device, probably due to some
> other flag disqualifying them.
>
>> So it will be a lot more solid to define a ATA_HORKAGE_NO_LOG_DIR flag
>> and test for it in ata_log_supported(), completely preventing any access
>> to the log directory page for this drive type.
>
> That was my first thought but then I found ATA_HORKAGE_NO_ID_DEV_LOG
> which was in the calling path that actually triggered this issue.
>
> But, yes, I totally agree that's a more solid solution preventing this
> kind of issue to crop up again.
>
>>> +
>>> /* End Marker */
>>> { }
>>> };
>>
>> Note: if you need this fix sent to linux-stable, add "Cc: stable@..."
>> and a Fixes tag.
>
> I'd think it's fitting to send it to linux-stable, because it prevents
> those DOM's from working in v5.15.5+.
>
> Ok. I must have missed that part when I read submitting-patches doc.
>
> I'll rework and re-submit the patch.
I sent you a draft patch. Please try it.
Also, to confirm if the log directory log page is indeed the page that locks up
the drive, can you try this command:
sg_sat_read_gplog --dma --log=0 --page=0 --readonly
and
sg_sat_read_gplog --log=0 --page=0 --readonly
Beware that if the log drectory is the problem, this will lockup your drive !
If that is OK and works, then please try:
sg_sat_read_gplog --dma --log=48 --page=0 --readonly
and
sg_sat_read_gplog --log=48 --page=0 --readonly
log=48 (0x30) is the identify device log page. log=0 is the log directory log page.
>
>
> //Anton
--
Damien Le Moal
Western Digital Research
