Received-SPF: pass (google.com: domain of linux-kernel+bounces-145387-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <7e6eb387-5a0e-460c-af08-eff070fa35ca@axis.com>
Date: Mon, 15 Apr 2024 16:49:46 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] ata: Add sdev attribute to lower link speed in runtime
Content-Language: en-US
To: Damien Le Moal <dlemoal@kernel.org>,
 Gustav Ekelund <gustav.ekelund@axis.com>, cassel@kernel.org, hare@suse.de,
 martin.petersen@oracle.com
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@axis.com
References: <20240412134838.788502-1-gustav.ekelund@axis.com>
 <4e5c88f1-1b24-4f6d-8c11-d7029329ba7a@kernel.org>
From: Gustav Ekelund <gustaek@axis.com>
In-Reply-To: <4e5c88f1-1b24-4f6d-8c11-d7029329ba7a@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?Ny9rOXhHNFhWd2Y4U0tjTUVCcmhiYlpUazF3d1Z4UncwZDJkRStncHIzMUx5?=
 =?utf-8?B?OXhvRmlRSDBhSTYwTHFpNXNwQXVxbmg4T1VyYkxIMU5QT1E3aGNCR04rL1R1?=
 =?utf-8?B?VWxUM2FWeEo5L2RrUjlVTGF5a0E0QnhpUUFyS3lLZUJRb0JybklydXBLbmlH?=
 =?utf-8?B?RUcybFRNbTY4cS9TbU1hak1EZGVTSVlyd0wvRG9nVFBwVEJHbklPUzBBV2RW?=
 =?utf-8?B?ZE91Q0h3VURSbVV3TGh0U1lPVW02MW5ENU5yWUM1L1dGSkRVeWtEUUxBRXM1?=
 =?utf-8?B?TGtrbk5YWVBiSFBjZFA4NFdtRTVzY3FuaG4xeENyb2I4V3dEZFFpMmNDVUdC?=
 =?utf-8?B?aThvNTgzYXIyZXF5TlZPL0hBdFUvYjB4RnJDMUc4MkppTGZDbytxS0ZXTUxu?=
 =?utf-8?B?RzFUaVlsMzJObmZHdEkzbzgrMkpqTER1Y3pVbC9mVjU1TkgwVXh0RzdBV3Fi?=
 =?utf-8?B?eER5Y1hpenlCTWg2RkdBeVB1Zk9FbkxYRHBQT3pyNWU0S2dvSjAwcGp3WVFV?=
 =?utf-8?B?VzV3aERPdS9qL1RlY1RMdlVnckg2Nk44a1hsT2QrUjZQNUlYRVRsczhyQWNL?=
 =?utf-8?B?Y2huNVN0Z3cvM3NJVlZ6cXc4Rlp3TXpHZGJVanE5YnVQa2ZpRnVFbFdNSkFp?=
 =?utf-8?B?dGdhemZ4TVppQUpiUXRkbTlhVng3OWRUL0xWZTAzQkE1eWlGQTNXSmo0MXJn?=
 =?utf-8?B?cU4veDJLTkRsT1NvL1ZNa3cxNDF4MDNZdzMvSFltUlFvb2VnWjhuQUhPWlI5?=
 =?utf-8?B?NHR4ZGJBUkpoUkpMeDJ6RU5Fa2V1UUloYUVSNTlEU2JvN1RoSzZiQUIrbXJ4?=
 =?utf-8?B?SWVUa2tHZ0U5MHVacm9IS0R5cVpURHp0M1VZNHVYMUNmL3Q2OXlUMmlzVkZS?=
 =?utf-8?B?Ynk0TysyNEhGWGo1V05PWktRT1laTWp0MVB5YVNWQkl6OWlTUGdsQmNOM3pS?=
 =?utf-8?B?b1E0U0VVR1YvRTJrM2o5R2RBRWxENmJ2RWozWFVIMzRLOWhzMG5rZzVoKzBO?=
 =?utf-8?B?UkJ3NUFldGVaeERXQ2hCaDBNOVhyZThCbUpKeWZhUHp1RnlyL0NaUzVjMW12?=
 =?utf-8?B?Q1Y3d2lSeTBrQjJ0MTc3VFJQVXp0MTIrdXFsYnFXcmJBNFVxTWdXWFRuV0dG?=
 =?utf-8?B?WnlKcEF3VFM5WVZlcW1oZ2RUSC9qenF5WDNLZENQcVllOWZOZWs2VkNYblht?=
 =?utf-8?B?UGE4MHVPa2s2a0pBUzZqdjBlMDJrMmdnL3JSKzhBN1ROUjg4UWRYdlhFdzYx?=
 =?utf-8?B?clJlNVZIcitzY2ZRL25sR09GbEkweFJzdURUQ241YmtjY1JnUXpZdFlrdDZF?=
 =?utf-8?B?TnZBZnZTK0JTTDVaSUFWZkthd3FZckFsbnd2bEowMWozTGRla0FNTVJzZkVQ?=
 =?utf-8?B?ZDgvdTFsSWlSTjdZTFV4NWZkTU00T0UwdFBHMyt6OXE4SmhSZUx4TVB4UTRB?=
 =?utf-8?B?eVZsZ01jTndPand4Vkl1VVhyTlJvOWFDeG5Ra3IvVTk5YS9nc1FicHhLVjgx?=
 =?utf-8?B?NzY1eEFkZ0xiY25WOEFHdnpwTm5yL2JQUzNlMkE0NnZSckMxSVh3UmNFa0hM?=
 =?utf-8?B?MVN0YjFKZDhtUGlyZUE4RkZad2dPYm5kR0crVzVwWVZJcndoRWVuQ0VPaHdk?=
 =?utf-8?B?bC9iV2hSQndaZDg1WFZXSzNzcDNEZ1VxMkgrV0kxNHFibjAzNWN1RnpSYUZk?=
 =?utf-8?B?eWlkbVN3RG43NVBRazBaVkplN3FaWHlYMlNaY0wxK3FMTjZkR0JFSDlnNmVK?=
 =?utf-8?B?eTVRKzZFUkU4blVIUmdna2hLZlNIOEMrcDE1WDN6dWd0ZHR4REZDcjQxTnNK?=
 =?utf-8?B?OGJuUXZQTTExc2Y3b2UycHl5WTFXOHRwV0tDalNwOWtEWlNyVUpBMEx3WUs0?=
 =?utf-8?B?cytURk9wcHMzRHUxY0FxSk1nbE1FVVl2TGkxanp4Y3Urb3R2b01ZRzdMUlpZ?=
 =?utf-8?B?b1FESUlnKzVXQmZMN1YzSG5nTGxQOVJiRGl0MmhGZDZaSzBFV1BoYUxwR01H?=
 =?utf-8?B?MGJzQ1l2TzBTQlNpMU5teDhEQlBvR21ITGtZMUViaW95bmwvR0tjNzZyTkdy?=
 =?utf-8?B?RzNCWWdwVks4YnhpNGh2WHkzRlFJbjhPVDJIUlRwSGlqR0UrK1JKSUtxV3lt?=
 =?utf-8?Q?nfrQ=3D?=
X-OriginatorOrg: axis.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a5a03d7e-caa6-43c9-23ef-08dc5d5b4c03
X-MS-Exchange-CrossTenant-AuthSource: GV1PR02MB10909.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2024 14:49:48.0201
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 78703d3c-b907-432f-b066-88f7af9ca3af
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lpwvP8eiODH2FwEz2BkizWUCfNREciO25qZbl7aPHtXzN8OTZYCQpFNS8kHJD9yIOPUT3HllLgt0zYgs/XLMEA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR02MB9020

On 4/13/24 02:29, Damien Le Moal wrote:
> On 4/12/24 22:48, Gustav Ekelund wrote:
>> Expose a new sysfs attribute to userspace that gives root the ability to
>> lower the link speed in a scsi_device at runtime. The handle enables
>> programs to, based on external circumstances that may be unbeknownst to
>> the kernel, determine if a link should slow down to perhaps achieve a
>> stabler signal. External circumstances could include the mission time
>> of the connected hardware or observations to temperature trends.
> 
> may, perhaps, could... This does not sound very deterministic. Do you have an
> actual practical use case where this patch is useful and solve a real problem ?
> 
> Strictly speaking, if you are seeing link stability issues due to temperature or
> other environmental factors (humidity, altitude), then either you are operating
> your hardware (board and/or HDD) outside of their environmental specifications,
> or you have some serious hardware issues (which can be a simple as a bad SATA
> cable or an inappropriate power supply). In both cases, I do not think that this
> patch will be of any help.
> 
> Furthermore, libata already lowers a link speed automatically at runtime if it
> sees too many NCQ errors. Isn't that enough ? And we also have the horkage flags
> to force a maximum link speed for a device/adapter, which can also be specified
> as a libata module argument (libata.force).
> 
>> Writing 1 to /sys/block/*/device/down_link_spd signals the kernel to
>> first lower the link speed one step with sata_down_spd_limit and then
>> finish off with sata_link_hardreset.
> 
> We already have "/sys/class/ata_link/*/hw_sata_spd_limit", which is read-only
> for now. So if you can really justify this manual link speed tuning for an
> actual use case (not a hypothetical one), then the way to go would be to make
> that attribute RW and implement its store() method to lower the link speed at
> runtime.
> 
> And by the way, looking at what that attribute says, I always get:
> <unknown>
> 
> So it looks like there is an issue with it that went unnoticed (because no one
> is using it...). This needs some fixing.
> 
Hello Damien and Niklas,

Thank you for the feedback.

I have a hotplug system, where the links behave differently depending
on the disk model connected. For some models the kernel emits a lot of
bus errors, but mostly not enough errors for it to automatically lower
the link speed, except during high workloads. I have not observed any
data-loss regarding the errors, but the excessive logging becomes a problem.

So I want to adapt the link, depending on the connected model, in a
running system because I know that some particular models in this case
will operate better in SATA2 in this system.

Can I use the libata.force module to make changes to a particular link
in runtime?

Best regards
Gustav