* [PATCH] ata: libata-core: Enable context analysis
@ 2026-05-05 4:22 Bart Van Assche
2026-05-11 16:53 ` Niklas Cassel
0 siblings, 1 reply; 2+ messages in thread
From: Bart Van Assche @ 2026-05-05 4:22 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Niklas Cassel, Marco Elver, linux-ide, Bart Van Assche
Enable Clang's context analysis. This will cause the build to fail if
e.g. a locking bug would be introduced in an error path. This patch
should not affect the generated assembler code.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/ata/Makefile | 2 ++
drivers/ata/libata-core.c | 28 ++++++++++++++++++++++++++++
drivers/ata/libata-eh.c | 6 ++++++
drivers/ata/libata.h | 6 ++++--
4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 20e6645ab737..b96025abd45e 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_ATA) += libata.o
# non-SFF interface
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e76d15411e2a..ecdd6310236b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1537,6 +1537,7 @@ unsigned int ata_exec_internal(struct ata_device *dev, struct ata_taskfile *tf,
const u8 *cdb, enum dma_data_direction dma_dir,
void *buf, unsigned int buflen,
unsigned int timeout)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_link *link = dev->link;
struct ata_port *ap = link->ap;
@@ -1747,6 +1748,7 @@ static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)
*/
unsigned int ata_do_dev_read_id(struct ata_device *dev,
struct ata_taskfile *tf, __le16 *id)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
return ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
id, sizeof(id[0]) * ATA_ID_WORDS, 0);
@@ -1776,6 +1778,7 @@ EXPORT_SYMBOL_GPL(ata_do_dev_read_id);
*/
int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
unsigned int flags, u16 *id)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_port *ap = dev->link->ap;
unsigned int class = *p_class;
@@ -1990,6 +1993,7 @@ bool ata_dev_power_init_tf(struct ata_device *dev, struct ata_taskfile *tf,
}
static bool ata_dev_power_is_active(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_taskfile tf;
unsigned int err_mask;
@@ -2027,6 +2031,7 @@ static bool ata_dev_power_is_active(struct ata_device *dev)
* Kernel thread context (may sleep).
*/
void ata_dev_power_set_standby(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned long ap_flags = dev->link->ap->flags;
struct ata_taskfile tf;
@@ -2074,6 +2079,7 @@ void ata_dev_power_set_standby(struct ata_device *dev)
* Kernel thread context (may sleep).
*/
void ata_dev_power_set_active(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_taskfile tf;
unsigned int err_mask;
@@ -2118,6 +2124,7 @@ void ata_dev_power_set_active(struct ata_device *dev)
*/
unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
u8 page, void *buf, unsigned int sectors)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned long ap_flags = dev->link->ap->flags;
struct ata_taskfile tf;
@@ -2174,6 +2181,7 @@ static inline void ata_clear_log_directory(struct ata_device *dev)
}
static int ata_read_log_directory(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
u16 version;
@@ -2197,6 +2205,7 @@ static int ata_read_log_directory(struct ata_device *dev)
}
static int ata_log_supported(struct ata_device *dev, u8 log)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
if (dev->quirks & ATA_QUIRK_NO_LOG_DIR)
return 0;
@@ -2208,6 +2217,7 @@ static int ata_log_supported(struct ata_device *dev, u8 log)
}
static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err, i;
@@ -2288,6 +2298,7 @@ static inline bool ata_dev_knobble(struct ata_device *dev)
}
static void ata_dev_config_ncq_send_recv(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
@@ -2312,6 +2323,7 @@ static void ata_dev_config_ncq_send_recv(struct ata_device *dev)
}
static void ata_dev_config_ncq_non_data(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
@@ -2328,6 +2340,7 @@ static void ata_dev_config_ncq_non_data(struct ata_device *dev)
}
static void ata_dev_config_ncq_prio(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
@@ -2392,6 +2405,7 @@ bool ata_adapter_is_online(struct ata_port *ap)
static int ata_dev_config_ncq(struct ata_device *dev,
char *desc, size_t desc_sz)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_port *ap = dev->link->ap;
int hdepth = 0, ddepth = ata_id_queue_depth(dev->id);
@@ -2474,6 +2488,7 @@ static void ata_dev_config_sense_reporting(struct ata_device *dev)
}
static void ata_dev_config_zac(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
u8 *identify_buf = dev->sector_buf;
@@ -2516,6 +2531,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
}
static void ata_dev_config_trusted(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
u64 trusted_cap;
unsigned int err;
@@ -2552,6 +2568,7 @@ static void ata_dev_cleanup_cdl_resources(struct ata_device *dev)
}
static int ata_dev_init_cdl_resources(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_cdl *cdl = dev->cdl;
unsigned int err_mask;
@@ -2575,6 +2592,7 @@ static int ata_dev_init_cdl_resources(struct ata_device *dev)
}
static void ata_dev_config_cdl(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
bool cdl_enabled;
@@ -2691,6 +2709,7 @@ static void ata_dev_config_cdl(struct ata_device *dev)
}
static int ata_dev_config_lba(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
const u16 *id = dev->id;
const char *lba_desc;
@@ -2770,6 +2789,7 @@ static void ata_dev_config_fua(struct ata_device *dev)
}
static void ata_dev_config_devslp(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
u8 *sata_setting = dev->sector_buf;
unsigned int err_mask;
@@ -2798,6 +2818,7 @@ static void ata_dev_config_devslp(struct ata_device *dev)
}
static void ata_dev_config_cpr(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int err_mask;
size_t buf_len;
@@ -2936,6 +2957,7 @@ static void ata_dev_print_features(struct ata_device *dev)
* 0 on success, -errno otherwise
*/
int ata_dev_configure(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_port *ap = dev->link->ap;
bool print_info = ata_dev_print_info(dev);
@@ -3896,6 +3918,7 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,
* 0 on success, negative errno otherwise
*/
int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
unsigned int class = dev->class;
u16 *id = (void *)dev->sector_buf;
@@ -3931,6 +3954,7 @@ int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags)
*/
int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
unsigned int readid_flags)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
u64 n_sectors = dev->n_sectors;
u64 n_native_sectors = dev->n_native_sectors;
@@ -4631,6 +4655,7 @@ static void ata_dev_xfermask(struct ata_device *dev)
*/
static unsigned int ata_dev_set_xfermode(struct ata_device *dev)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_taskfile tf;
@@ -4676,6 +4701,7 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev)
* 0 on success, AC_ERR_* mask otherwise.
*/
unsigned int ata_dev_set_feature(struct ata_device *dev, u8 subcmd, u8 action)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_taskfile tf;
unsigned int timeout = 0;
@@ -4712,6 +4738,7 @@ EXPORT_SYMBOL_GPL(ata_dev_set_feature);
*/
static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors)
+ __must_hold(&dev->link->ap->host->eh_mutex)
{
struct ata_taskfile tf;
unsigned int err_mask;
@@ -6817,6 +6844,7 @@ EXPORT_SYMBOL_GPL(ata_ratelimit);
* Might sleep.
*/
void ata_msleep(struct ata_port *ap, unsigned int msecs)
+ __context_unsafe(conditional locking)
{
bool owns_eh = ap && ap->host->eh_owner == current;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 9a4b67b90b17..093af027c99b 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2833,6 +2833,7 @@ static bool ata_eh_followup_srst_needed(struct ata_link *link, int rc)
int ata_eh_reset(struct ata_link *link, int classify,
struct ata_reset_operations *reset_ops)
+ __must_hold(&link->ap->host->eh_mutex)
{
struct ata_port *ap = link->ap;
struct ata_link *slave = ap->slave_link;
@@ -3816,6 +3817,7 @@ static int ata_eh_handle_dev_fail(struct ata_device *dev, int err)
*/
int ata_eh_recover(struct ata_port *ap, struct ata_reset_operations *reset_ops,
struct ata_link **r_failed_link)
+ __must_hold(&ap->host->eh_mutex)
{
struct ata_link *link;
struct ata_device *dev;
@@ -3879,6 +3881,9 @@ int ata_eh_recover(struct ata_port *ap, struct ata_reset_operations *reset_ops,
ata_for_each_link(link, ap, EDGE) {
struct ata_eh_context *ehc = &link->eh_context;
+ /* Tell the compiler that link->ap == ap. */
+ __assume_ctx_lock(&link->ap->host->eh_mutex);
+
if (!(ehc->i.action & ATA_EH_RESET))
continue;
@@ -4112,6 +4117,7 @@ void ata_eh_finish(struct ata_port *ap)
* Kernel thread context (may sleep).
*/
void ata_std_error_handler(struct ata_port *ap)
+ __must_hold(&ap->host->eh_mutex)
{
struct ata_reset_operations *reset_ops = &ap->ops->reset;
struct ata_link *link = &ap->link;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index b5423b6e97de..f58bb591b3fa 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -173,8 +173,10 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap);
/* libata-eh.c */
extern unsigned int ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
extern void ata_internal_cmd_timed_out(struct ata_device *dev, u8 cmd);
-extern void ata_eh_acquire(struct ata_port *ap);
-extern void ata_eh_release(struct ata_port *ap);
+extern void ata_eh_acquire(struct ata_port *ap)
+ __acquires(ap->host->eh_mutex);
+extern void ata_eh_release(struct ata_port *ap)
+ __releases(ap->host->eh_mutex);
extern void ata_scsi_error(struct Scsi_Host *host);
extern void ata_eh_fastdrain_timerfn(struct timer_list *t);
extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ata: libata-core: Enable context analysis
2026-05-05 4:22 [PATCH] ata: libata-core: Enable context analysis Bart Van Assche
@ 2026-05-11 16:53 ` Niklas Cassel
0 siblings, 0 replies; 2+ messages in thread
From: Niklas Cassel @ 2026-05-11 16:53 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Damien Le Moal, Marco Elver, linux-ide
On Tue, May 05, 2026 at 06:22:27AM +0200, Bart Van Assche wrote:
> Enable Clang's context analysis. This will cause the build to fail if
> e.g. a locking bug would be introduced in an error path. This patch
> should not affect the generated assembler code.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/ata/Makefile | 2 ++
> drivers/ata/libata-core.c | 28 ++++++++++++++++++++++++++++
> drivers/ata/libata-eh.c | 6 ++++++
> drivers/ata/libata.h | 6 ++++--
> 4 files changed, 40 insertions(+), 2 deletions(-)
Hello Bart,
Thank you for doing this work!
It looks nice to me.
I did notice two things:
1) It seems that you are only checking for the EH mutex.
$ git grep -A 1 "LOCKING" drivers/ata/
Does have many functions with:
LOCKING:
spin_lock_irqsave(host lock)
Would it be possible to add __must_hold() annotations for these functions
too, but for ap->lock instead of EH mutex ?
2) There seems to be some files that did not get any annotations, e.g.
drivers/ata/libata-scsi.c, drivers/ata/libata-sata.c,
drivers/ata/libata-acpi.c, drivers/ata/libata-pmp.c.
Would it be possible to add annotations for these files too?
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-11 16:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 4:22 [PATCH] ata: libata-core: Enable context analysis Bart Van Assche
2026-05-11 16:53 ` Niklas Cassel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox