From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5A5ED355F41 for ; Tue, 17 Mar 2026 22:16:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773785780; cv=none; b=UdtSv89vpVowi3DTNafsVgSHtO8xtsliubc6aKTTvfjMs/pGJWxugSzPZvI9auKsWRXk+erwEfH71TKkKI3rIcjLP72pUVEOpHI+9dfPLLYNnGpHvyQOTilGctAbxSGlp/T2OmwoWlIXLXGduiJu0D4sED22ntI0SDD8i4uygW0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773785780; c=relaxed/simple; bh=cbk0KmpSBN0gRdMOOzwzobkvq9/63Vw2F/mwMGza5vg=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GujurC26XjCr8qUR3yhTy9RTA03Y3JWF3koaoWkVFg5xvXIsSGoSfsBdx16mWz599DfLxP0plwQLqiAWDq0Jm1zGcz1h/GzL8gJYpyQUw/HgczqvNn5DOH5yb+lvQLbfKVJSNkhMGTXPIuD9i+H7XFuJZ+QG3ozp6SpGpIE9lww= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.com; spf=pass smtp.mailfrom=gmail.com; arc=none smtp.client-ip=209.85.167.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-5a13d40c760so6254548e87.0 for ; Tue, 17 Mar 2026 15:16:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773785777; x=1774390577; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:reply-to:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=c9EaTAS3TCBEoNvybqZAkYdlegFkdiUQjyghCpDGEHk=; b=EoHqDabMUoyqBWMLMugBNOQLnvV3uqpGGI6vf/Uo0pEJrcYDAgvOkiE+VRpUeuzdrH 7eDcC2l3iMzsAkcdJZm3cfstlkXKTg4xLVfAoCbW4qtqRHgXi6UOfs0OnvMJn9ZibWT2 9PJ6GVrP6ysDJoA6yc5M/mQCgRoe7zGS40eaqZDhAMjtKGfPpA81etVA0FpD09vcCwWC bgXgM8nExFVCsBeMyZQC3mQCCz73lrzxmE/RAsrMhHdPYpc0COwp/nA1Z3ghwTucTCCc ifUSonaKYfRYyXYMUxYgkejvMczsM9Es88tgiHc40dpFDi7AkUoxzk9eJrwP8shw5Brh 7YiQ== X-Forwarded-Encrypted: i=1; AJvYcCW2zNLoTziQn/vUGfoInqH8Yu1CWMBT7NpssaH5uU0tDjNf6Kc49IA/HvRCWX1Sk7iEgTODk97hEGQgo0E=@vger.kernel.org X-Gm-Message-State: AOJu0Yw9htSWF4A1FFx6aJ/oR5+HcUPQPeavrjfm938Bl1tCYBtN/Qm3 uxrv7SVU2kM9qezETabv2b6bWOFndPvxLa/8yATrNGwl0FtLClcXn027 X-Gm-Gg: ATEYQzxXzLMCKcw+3ACjOh/EXQ3hgkAY5LStqM09Dy6Ri/AT4EPuXY5p3ukkvAikBdx bRSEvd7G5yS1YBOQZIZr4HKQCe4ZCnL0HisK66VDfbO3hJU3P3md82Q86eNaTrp5oJ3e6k1+ht3 J3UnkaPmaRNstsoqyH8R7Ns+HxlaHVestJDJpzKlMfvYvWsqsu/sYois6c9vrMajz76U1rJub5j 67/XU3/c7euYu4Px0igBlK03/V/8R2E6iBHl7PiwxJ1nz5jjibqmBkim1SFaHPFXbg8cgWHMpsJ GZbw1L8WJgmlfF6xuhzGmd2d24qoP+ESJCnqmwZr/BrjEeY28SZUoD8MlTQUJwFgmWd/SQtN6MZ kLOglzU1vtBbkDSPkiXwRSZuamBSGS6BfQ2FA4CEFcVQadin4xbluqLeXuxqtNxMXb0SfxYPLPP 76UjnZeT2V7nlz1Gs4LRpr X-Received: by 2002:ac2:43ca:0:b0:5a1:7434:6b32 with SMTP id 2adb3069b0e04-5a2796c3442mr349033e87.23.1773785777151; Tue, 17 Mar 2026 15:16:17 -0700 (PDT) Received: from [10.68.32.41] ([5.194.237.34]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5a279c742c5sm136304e87.57.2026.03.17.15.16.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Mar 2026 15:16:16 -0700 (PDT) Message-ID: <810f2e9c-c8c9-47ad-9ea0-b3a15d647453@linux.com> Date: Wed, 18 Mar 2026 02:16:13 +0400 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: efremov@linux.com Subject: Re: [PATCH] floppy: annotate data-races around command_status and floppy_work_fn To: Cen Zhang , axboe@kernel.dk Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, baijiaju1990@gmail.com, r33s3n6@gmail.com, gality369@gmail.com, zhenghaoran154@gmail.com, hanguidong02@gmail.com, ziyuzhang201@gmail.com References: <20260316090826.2878527-1-zzzccc427@gmail.com> Content-Language: en-US, ru-RU From: "Denis Efremov (Oracle)" In-Reply-To: <20260316090826.2878527-1-zzzccc427@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hello, thank you for the patch. Some comments below. On 16/03/2026 13:08, Cen Zhang wrote: > command_status is accessed by multiple contexts: > - generic_done() and do_wakeup() write it from the floppy > workqueue context. > - wait_til_done() reads it in wait_event conditions without > holding fdc_busy. > - is_alive() reads it locklessly to check driver liveness. > - floppy_queue_rq(), lock_fdc(), and unlock_fdc() write it > during FDC ownership transitions. > > There is currently no LKMM annotation for these concurrent accesses > since command_status relies on the deprecated volatile qualifier. > Remove volatile and use READ_ONCE()/WRITE_ONCE() on every access > to command_status to provide proper LKMM data-race annotations. > > Also annotate floppy_work_fn with WRITE_ONCE() in schedule_bh() > and READ_ONCE() in floppy_work_workfn(), and current_type[drive] > with READ_ONCE() in drive_no_geom() where it can be read without > holding the FDC lock. Please, inline a KCSAN report if you used it. > > Signed-off-by: Cen Zhang > --- > drivers/block/floppy.c | 38 +++++++++++++++++++++----------------- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c > index 92e446a64371..1da7947fe042 100644 > --- a/drivers/block/floppy.c > +++ b/drivers/block/floppy.c > @@ -502,7 +502,7 @@ static int probing; > #define FD_COMMAND_ERROR 2 > #define FD_COMMAND_OKAY 3 > > -static volatile int command_status = FD_COMMAND_NONE; > +static int command_status = FD_COMMAND_NONE; > static unsigned long fdc_busy; > static DECLARE_WAIT_QUEUE_HEAD(fdc_wait); > static DECLARE_WAIT_QUEUE_HEAD(command_done); > @@ -601,7 +601,8 @@ static inline void fdc_outb(unsigned char value, int fdc, int reg) > > static inline bool drive_no_geom(int drive) > { > - return !current_type[drive] && !ITYPE(drive_state[drive].fd_device); > + return !READ_ONCE(current_type[drive]) && > + !ITYPE(drive_state[drive].fd_device); > } > > #ifndef fd_eject > @@ -640,7 +641,7 @@ static const char *timeout_message; > static void is_alive(const char *func, const char *message) > { > /* this routine checks whether the floppy driver is "alive" */ > - if (test_bit(0, &fdc_busy) && command_status < 2 && > + if (test_bit(0, &fdc_busy) && READ_ONCE(command_status) < 2 && > !delayed_work_pending(&fd_timeout)) { > DPRINT("%s: timeout handler died. %s\n", func, message); > } > @@ -892,7 +893,7 @@ static int lock_fdc(int drive) > if (wait_event_interruptible(fdc_wait, !test_and_set_bit(0, &fdc_busy))) > return -EINTR; > > - command_status = FD_COMMAND_NONE; > + WRITE_ONCE(command_status, FD_COMMAND_NONE); > > reschedule_timeout(drive, "lock fdc"); > set_fdc(drive); > @@ -906,7 +907,7 @@ static void unlock_fdc(void) > DPRINT("FDC access conflict!\n"); > > raw_cmd = NULL; > - command_status = FD_COMMAND_NONE; > + WRITE_ONCE(command_status, FD_COMMAND_NONE); > cancel_delayed_work(&fd_timeout); > do_floppy = NULL; > cont = NULL; > @@ -990,7 +991,9 @@ static void (*floppy_work_fn)(void); > > static void floppy_work_workfn(struct work_struct *work) > { > - floppy_work_fn(); > + void (*fn)(void) = READ_ONCE(floppy_work_fn); > + > + fn(); > } > > static DECLARE_WORK(floppy_work, floppy_work_workfn); > @@ -999,7 +1002,7 @@ static void schedule_bh(void (*handler)(void)) > { > WARN_ON(work_pending(&floppy_work)); > > - floppy_work_fn = handler; > + WRITE_ONCE(floppy_work_fn, handler); > queue_work(floppy_wq, &floppy_work); > } > > @@ -1864,7 +1867,7 @@ static void show_floppy(int fdc) > > pr_info("cont=%p\n", cont); > pr_info("current_req=%p\n", current_req); > - pr_info("command_status=%d\n", command_status); > + pr_info("command_status=%d\n", READ_ONCE(command_status)); > pr_info("\n"); > } > > @@ -1991,7 +1994,7 @@ static void do_wakeup(void) > { > reschedule_timeout(MAXTIMEOUT, "do wakeup"); > cont = NULL; > - command_status += 2; > + WRITE_ONCE(command_status, command_status + 2); WRITE_ONCE(command_status, READ_ONCE(command_status) + 2); ? or with a temporary variable. > wake_up(&command_done); > } > > @@ -2019,11 +2022,12 @@ static int wait_til_done(void (*handler)(void), bool interruptible) > schedule_bh(handler); > > if (interruptible) > - wait_event_interruptible(command_done, command_status >= 2); > + wait_event_interruptible(command_done, > + READ_ONCE(command_status) >= 2); > else > - wait_event(command_done, command_status >= 2); > + wait_event(command_done, READ_ONCE(command_status) >= 2); > > - if (command_status < 2) { > + if (READ_ONCE(command_status) < 2) { > cancel_activity(); > cont = &intr_cont; > reset_fdc(); > @@ -2031,18 +2035,18 @@ static int wait_til_done(void (*handler)(void), bool interruptible) > } > > if (fdc_state[current_fdc].reset) > - command_status = FD_COMMAND_ERROR; > - if (command_status == FD_COMMAND_OKAY) > + WRITE_ONCE(command_status, FD_COMMAND_ERROR); > + if (READ_ONCE(command_status) == FD_COMMAND_OKAY) > ret = 0; > else > ret = -EIO; > - command_status = FD_COMMAND_NONE; > + WRITE_ONCE(command_status, FD_COMMAND_NONE); > return ret; > } > > static void generic_done(int result) > { > - command_status = result; > + WRITE_ONCE(command_status, result); > cont = &wakeup_cont; > } > > @@ -2873,7 +2877,7 @@ static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx, > list_add_tail(&bd->rq->queuelist, &floppy_reqs); > spin_unlock_irq(&floppy_lock); > > - command_status = FD_COMMAND_NONE; > + WRITE_ONCE(command_status, FD_COMMAND_NONE); > __reschedule_timeout(MAXTIMEOUT, "fd_request"); > set_fdc(0); > process_fd_request();