Thread: [f2fs-dev] [PATCH V2 3/5] nvmet: ignore discard return value
Brought to you by:
kjgkr
Menu
▾
▴
linux-f2fs-devel
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:57:53
|
__blkdev_issue_discard() always returns 0, making the error checking
in nvmet_bdev_discard_range() dead code.
Kill the function nvmet_bdev_discard_range() and call
__blkdev_issue_discard() directly from nvmet_bdev_execute_discard(),
since no error handling is needed anymore for __blkdev_issue_discard()
call.
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
drivers/nvme/target/io-cmd-bdev.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 8d246b8ca604..97d868d6e01a 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -362,29 +362,14 @@ u16 nvmet_bdev_flush(struct nvmet_req *req)
return 0;
}
-static u16 nvmet_bdev_discard_range(struct nvmet_req *req,
- struct nvme_dsm_range *range, struct bio **bio)
-{
- struct nvmet_ns *ns = req->ns;
- int ret;
-
- ret = __blkdev_issue_discard(ns->bdev,
- nvmet_lba_to_sect(ns, range->slba),
- le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
- GFP_KERNEL, bio);
- if (ret && ret != -EOPNOTSUPP) {
- req->error_slba = le64_to_cpu(range->slba);
- return errno_to_nvme_status(req, ret);
- }
- return NVME_SC_SUCCESS;
-}
-
static void nvmet_bdev_execute_discard(struct nvmet_req *req)
{
+ struct nvmet_ns *ns = req->ns;
struct nvme_dsm_range range;
struct bio *bio = NULL;
+ sector_t nr_sects;
int i;
- u16 status;
+ u16 status = NVME_SC_SUCCESS;
for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
status = nvmet_copy_from_sgl(req, i * sizeof(range), &range,
@@ -392,9 +377,11 @@ static void nvmet_bdev_execute_discard(struct nvmet_req *req)
if (status)
break;
- status = nvmet_bdev_discard_range(req, &range, &bio);
- if (status)
- break;
+ nr_sects = le32_to_cpu(range.nlb) << (ns->blksize_shift - 9);
+ __blkdev_issue_discard(ns->bdev,
+ nvmet_lba_to_sect(ns, range.slba),
+ nr_sects,
+ GFP_KERNEL, &bio);
}
if (bio) {
--
2.40.0
|
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:57:58
|
Hi,
__blkdev_issue_discard() only returns value 0, that makes post call
error checking code dead. This patch series revmoes this dead code at
all the call sites and adjust the callers.
Please note that it doesn't change the return type of the function from
int to void in this series, it will be done once this series gets merged
smoothly.
For f2fs and xfs I've ran following test which includes discard
they produce same PASS and FAIL output with and without this patch
series [1].
for-next (lblk-fnext) discard-ret (lblk-discard)
--------------------- --------------------------
FAIL f2fs/008 FAIL f2fs/008
FAIL f2fs/014 FAIL f2fs/014
FAIL f2fs/015 FAIL f2fs/015
PASS f2fs/017 PASS f2fs/017
PASS xfs/016 PASS xfs/016
PASS xfs/288 PASS xfs/288
PASS xfs/432 PASS xfs/432
PASS xfs/449 PASS xfs/449
PASS xfs/513 PASS xfs/513
PASS generic/033 PASS generic/033
PASS generic/038 PASS generic/038
PASS generic/098 PASS generic/098
PASS generic/224 PASS generic/224
PASS generic/251 PASS generic/251
PASS generic/260 PASS generic/260
PASS generic/288 PASS generic/288
PASS generic/351 PASS generic/351
PASS generic/455 PASS generic/455
PASS generic/457 PASS generic/457
PASS generic/470 PASS generic/470
PASS generic/482 PASS generic/482
PASS generic/500 PASS generic/500
PASS generic/537 PASS generic/537
PASS generic/608 PASS generic/608
PASS generic/619 PASS generic/619
PASS generic/746 PASS generic/746
PASS generic/757 PASS generic/757
For NVMeOF taret I've testing blktest with nvme_trtype=nvme_loop
and all the testcases are passing [2].
-ck
Chaitanya Kulkarni (5):
block: ignore discard return value
dm: ignore discard return value
nvmet: ignore discard return value
f2fs: ignore discard return value
xfs: ignore discard return value
block/blk-lib.c | 6 +++---
drivers/md/dm-thin.c | 12 +++++-------
drivers/md/md.c | 4 ++--
drivers/nvme/target/io-cmd-bdev.c | 29 ++++++++---------------------
fs/f2fs/segment.c | 12 +++++-------
fs/xfs/xfs_discard.c | 27 +++++----------------------
fs/xfs/xfs_discard.h | 2 +-
7 files changed, 29 insertions(+), 63 deletions(-)
######################XFS TEST dircard return value ignore ##################
linux-block (discard-ret-value) # gitlog -6
a6b2c13bf2b4 (HEAD -> discard-ret-value) xfs: ignore discard return value
ccb8da1a96b1 f2fs: ignore discard return value
e53455d2d61f nvmet: ignore discard return value
1e16fecedfd6 dm: ignore discard return value
e0f69471a2ad block: ignore discard return value
linux (discard-ret-value) # ./kernel_compile.sh
+ make -j 48
SYNC include/config/auto.conf.cmd
HOSTCC scripts/kconfig/conf.o
HOSTLD scripts/kconfig/conf
GEN arch/x86/include/generated/asm/orc_hash.h
WRAP arch/x86/include/generated/uapi/asm/bpf_perf_event.h
WRAP arch/x86/include/generated/uapi/asm/errno.h
WRAP arch/x86/include/generated/uapi/asm/fcntl.h
WRAP arch/x86/include/generated/uapi/asm/ioctl.h
WRAP arch/x86/include/generated/uapi/asm/ipcbuf.h
WRAP arch/x86/include/generated/uapi/asm/ioctls.h
WRAP arch/x86/include/generated/uapi/asm/param.h
[...]
LD arch/x86/boot/setup.elf
OBJCOPY arch/x86/boot/setup.bin
BUILD arch/x86/boot/bzImage
Kernel: arch/x86/boot/bzImage is ready (#1)
++ nproc
+ make -j 48 modules
DESCEND objtool
CALL scripts/checksyscalls.sh
INSTALL libsubcmd_headers
+ make -j 48 modules_install
SYMLINK /lib/modules/6.18.0-rc6lblk-discard-ret-val+/build
INSTALL /lib/modules/6.18.0-rc6lblk-discard-ret-val+/modules.order
INSTALL /lib/modules/6.18.0-rc6lblk-discard-ret-val+/modules.builtin.modinfo
INSTALL /lib/modules/6.18.0-rc6lblk-discard-ret-val+/modules.builtin
INSTALL /lib/modules/6.18.0-rc6lblk-discard-ret-val+/kernel/arch/x86/events/intel/intel-uncore.ko
[...]
DEPMOD /lib/modules/6.18.0-rc6lblk-discard-ret-val+
INSTALL /boot
Generating grub configuration file ...
Adding boot menu entry for UEFI Firmware Settings ...
done
real 5m30.328s
user 109m9.089s
sys 43m14.254s
linux-block (discard-ret-value) #
linux-block (discard-ret-value) #
linux-block (discard-ret-value) #
linux-block (discard-ret-value) #
linux-block (discard-ret-value) # reboot
Connection to 192.168.122.185 closed by remote host.
Connection to 192.168.122.185 closed.
ssh: connect to host 192.168.122.185 port 22: Connection refused
ssh: connect to host 192.168.122.185 port 22: Connection refused
ssh: connect to host 192.168.122.185 port 22: Connection refused
~ #
~ #
~ #
~ #
~ #
~ #
~ # cdxfstest
xfstests-dev (master) # ./setup-xfstest-loop.sh ; ./run-my-discard-tests.sh
xfstests-dev (master) # ./run-my-discard-tests.sh
=== XFSTest null_blk Setup (Memory-backed) ===
[1/6] Setting up null_blk devices (20GB each, memory-backed)...
Created: /dev/nullb0 (20GB, memory-backed)
Created: /dev/nullb1 (20GB, memory-backed)
nullb0 251:0 0 20G 0 disk
nullb1 251:1 0 20G 0 disk
[2/6] Creating mount directories...
Created: /mnt/test
Created: /mnt/scratch
[3/6] Creating test users and groups for xfstests...
Group fsgqa already exists
User fsgqa already exists
User 123456-fsgqa already exists
User fsgqa2 already exists
[4/6] Formatting TEST_DEV (/dev/nullb0) with XFS...
Formatted /dev/nullb0 with XFS filesystem
Note: SCRATCH_DEV will be formatted by xfstests as needed
[5/6] Creating local.config for xfstests...
Created: /mnt/data100G/xfs-linux/xfstests-dev/local.config
[6/6] Verifying test users...
â fsgqa user configured
â 123456-fsgqa user configured
â fsgqa2 user configured
=== Setup Complete ===
Configuration (null_blk - memory-backed):
TEST_DEV: /dev/nullb0 (20GB, RAM)
TEST_DIR: /mnt/test
SCRATCH_DEV: /dev/nullb1 (20GB, RAM)
SCRATCH_MNT: /mnt/scratch
Config file: /mnt/data100G/xfs-linux/xfstests-dev/local.config
null_blk devices:
nullb0 251:0 0 20G 0 disk
nullb1 251:1 0 20G 0 disk
Device info:
/dev/nullb0: 20.00 GB
/dev/nullb1: 20.00 GB
Advantages of null_blk (memory-backed):
â Much faster than loop devices (pure RAM)
â No disk I/O overhead
â Perfect for kernel testing
You can now run xfstests:
cd /mnt/data100G/xfs-linux/xfstests-dev
./check -g auto # Run all auto tests
./check generic/001 # Run single test
./check -g quick # Run quick test group
To cleanup later, run:
umount /mnt/test /mnt/scratch 2>/dev/null || true
rmmod null_blk
===============================================================================
Discard-Related Tests
===============================================================================
Total tests: 27
Dry run: NO
Interactive: NO
Press ENTER to run all tests, Ctrl+C to cancel
===============================================================================
Test [1/27]: f2fs/008
===============================================================================
Running: ./check f2fs/008
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
f2fs/008 [failed, exit status 1]- output mismatch (see /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/008.out.bad)
--- tests/f2fs/008.out 2025-11-17 22:53:15.022496507 -0800
+++ /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/008.out.bad 2025-11-23 16:05:11.860492472 -0800
@@ -1,2 +1 @@
QA output created by 008
-Silence is golden
...
(Run 'diff -u /mnt/data100G/xfs-linux/xfstests-dev/tests/f2fs/008.out /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/008.out.bad' to see the entire diff)
HINT: You _MAY_ be missing kernel fix:
bc8aeb04fd80 f2fs: fix to drop all discards after creating snapshot on lvm device
Ran: f2fs/008
Failures: f2fs/008
Failed 1 of 1 tests
â Test f2fs/008 FAILED (exit code: 1)
===============================================================================
Test [2/27]: f2fs/014
===============================================================================
Running: ./check f2fs/014
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
f2fs/014 - output mismatch (see /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/014.out.bad)
--- tests/f2fs/014.out 2025-11-17 22:53:15.023496535 -0800
+++ /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/014.out.bad 2025-11-23 16:05:17.174275720 -0800
@@ -1,2 +1 @@
QA output created by 014
-trimmed
...
(Run 'diff -u /mnt/data100G/xfs-linux/xfstests-dev/tests/f2fs/014.out /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/014.out.bad' to see the entire diff)
HINT: You _MAY_ be missing kernel fix:
21263d035ff2 f2fs: fix missing discard for active segments
Ran: f2fs/014
Failures: f2fs/014
Failed 1 of 1 tests
â Test f2fs/014 FAILED (exit code: 1)
===============================================================================
Test [3/27]: f2fs/015
===============================================================================
Running: ./check f2fs/015
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
f2fs/015
[failed, exit status 1]- output mismatch (see /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/015.out.bad)
--- tests/f2fs/015.out 2025-11-17 22:53:15.023496535 -0800
+++ /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/015.out.bad 2025-11-23 16:06:11.016487059 -0800
@@ -1,12 +1,12 @@
QA output created by 015
Option#0: background_gc=on :
-0
+32
Option#2: background_gc=off :
-0
+32
...
(Run 'diff -u /mnt/data100G/xfs-linux/xfstests-dev/tests/f2fs/015.out /mnt/data100G/xfs-linux/xfstests-dev/results//f2fs/015.out.bad' to see the entire diff)
Ran: f2fs/015
Failures: f2fs/015
Failed 1 of 1 tests
â Test f2fs/015 FAILED (exit code: 1)
===============================================================================
Test [4/27]: f2fs/017
===============================================================================
Running: ./check f2fs/017
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
f2fs/017 [not run] this test requires a zoned block device
Ran: f2fs/017
Not run: f2fs/017
Passed all 1 tests
â Test f2fs/017 PASSED
===============================================================================
Test [5/27]: xfs/016
===============================================================================
Running: ./check xfs/016
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
xfs/016 62s ... 62s
Ran: xfs/016
Passed all 1 tests
â Test xfs/016 PASSED
===============================================================================
Test [6/27]: xfs/288
===============================================================================
Running: ./check xfs/288
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
xfs/288 2s ... 1s
Ran: xfs/288
Passed all 1 tests
â Test xfs/288 PASSED
===============================================================================
Test [7/27]: xfs/432
===============================================================================
Running: ./check xfs/432
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
xfs/432 28s ... 27s
Ran: xfs/432
Passed all 1 tests
â Test xfs/432 PASSED
===============================================================================
Test [8/27]: xfs/449
===============================================================================
Running: ./check xfs/449
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
xfs/449 1s ... 2s
Ran: xfs/449
Passed all 1 tests
â Test xfs/449 PASSED
===============================================================================
Test [9/27]: xfs/513
===============================================================================
Running: ./check xfs/513
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
xfs/513 12s ... 11s
Ran: xfs/513
Passed all 1 tests
â Test xfs/513 PASSED
===============================================================================
Test [10/27]: generic/033
===============================================================================
Running: ./check generic/033
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/033 3s ... 3s
Ran: generic/033
Passed all 1 tests
â Test generic/033 PASSED
===============================================================================
Test [11/27]: generic/038
===============================================================================
Running: ./check generic/038
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/038 37s ... 37s
Ran: generic/038
Passed all 1 tests
â Test generic/038 PASSED
===============================================================================
Test [12/27]: generic/098
===============================================================================
Running: ./check generic/098
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/098 3s ... 3s
Ran: generic/098
Passed all 1 tests
â Test generic/098 PASSED
===============================================================================
Test [13/27]: generic/224
===============================================================================
Running: ./check generic/224
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/224 9s ... 8s
Ran: generic/224
Passed all 1 tests
â Test generic/224 PASSED
===============================================================================
Test [14/27]: generic/251
===============================================================================
Running: ./check generic/251
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/251 63s ... 43s
Ran: generic/251
Passed all 1 tests
â Test generic/251 PASSED
===============================================================================
Test [15/27]: generic/260
===============================================================================
Running: ./check generic/260
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/260 9s ... 8s
Ran: generic/260
Passed all 1 tests
â Test generic/260 PASSED
===============================================================================
Test [16/27]: generic/288
===============================================================================
Running: ./check generic/288
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/288 4s ... 4s
Ran: generic/288
Passed all 1 tests
â Test generic/288 PASSED
===============================================================================
Test [17/27]: generic/351
===============================================================================
Running: ./check generic/351
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/351 2s ... 3s
Ran: generic/351
Passed all 1 tests
â Test generic/351 PASSED
===============================================================================
Test [18/27]: generic/455
===============================================================================
Running: ./check generic/455
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/455 [not run] This test requires a valid $LOGWRITES_DEV
Ran: generic/455
Not run: generic/455
Passed all 1 tests
â Test generic/455 PASSED
===============================================================================
Test [19/27]: generic/457
===============================================================================
Running: ./check generic/457
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/457 [not run] This test requires a valid $LOGWRITES_DEV
Ran: generic/457
Not run: generic/457
Passed all 1 tests
â Test generic/457 PASSED
===============================================================================
Test [20/27]: generic/470
===============================================================================
Running: ./check generic/470
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/470 [not run] This test requires a valid $LOGWRITES_DEV
Ran: generic/470
Not run: generic/470
Passed all 1 tests
â Test generic/470 PASSED
===============================================================================
Test [21/27]: generic/482
===============================================================================
Running: ./check generic/482
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/482 [not run] This test requires a valid $LOGWRITES_DEV
Ran: generic/482
Not run: generic/482
Passed all 1 tests
â Test generic/482 PASSED
===============================================================================
Test [22/27]: generic/500
===============================================================================
Running: ./check generic/500
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/500 5s ... 5s
Ran: generic/500
Passed all 1 tests
â Test generic/500 PASSED
===============================================================================
Test [23/27]: generic/537
===============================================================================
Running: ./check generic/537
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/537 3s ... 3s
Ran: generic/537
Passed all 1 tests
â Test generic/537 PASSED
===============================================================================
Test [24/27]: generic/608
===============================================================================
Running: ./check generic/608
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/608 [not run] /dev/nullb1 xfs does not support -o dax=always
Ran: generic/608
Not run: generic/608
Passed all 1 tests
â Test generic/608 PASSED
===============================================================================
Test [25/27]: generic/619
===============================================================================
Running: ./check generic/619
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/619 16s ... 17s
Ran: generic/619
Passed all 1 tests
â Test generic/619 PASSED
===============================================================================
Test [26/27]: generic/746
===============================================================================
Running: ./check generic/746
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/746 9s ... 9s
Ran: generic/746
Passed all 1 tests
â Test generic/746 PASSED
===============================================================================
Test [27/27]: generic/757
===============================================================================
Running: ./check generic/757
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 dev 6.18.0-rc6lblk-discard-ret-val+ #1 SMP PREEMPT_DYNAMIC Sun Nov 23 15:58:42 PST 2025
MKFS_OPTIONS -- -f /dev/nullb1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/nullb1 /mnt/scratch
generic/757 [not run] This test requires a valid $LOGWRITES_DEV
Ran: generic/757
Not run: generic/757
Passed all 1 tests
â Test generic/757 PASSED
===============================================================================
TEST SUMMARY
===============================================================================
â FAIL f2fs/008
â FAIL f2fs/014
â FAIL f2fs/015
â PASS f2fs/017
â PASS xfs/016
â PASS xfs/288
â PASS xfs/432
â PASS xfs/449
â PASS xfs/513
â PASS generic/033
â PASS generic/038
â PASS generic/098
â PASS generic/224
â PASS generic/251
â PASS generic/260
â PASS generic/288
â PASS generic/351
â PASS generic/455
â PASS generic/457
â PASS generic/470
â PASS generic/482
â PASS generic/500
â PASS generic/537
â PASS generic/608
â PASS generic/619
â PASS generic/746
â PASS generic/757
===============================================================================
xfstests-dev (master) #
blktests (master) # ./test-nvme.sh
++ for t in loop tcp
++ echo '################nvme_trtype=loop############'
################nvme_trtype=loop############
++ nvme_img_size=900M
++ nvme_num_iter=1
++ nvme_trtype=loop
++ ./check nvme
nvme/002 (tr=loop) (create many subsystems and test discovery) [passed]
runtime 42.094s ... 42.504s
nvme/003 (tr=loop) (test if we're sending keep-alives to a discovery controller) [passed]
runtime 10.255s ... 10.271s
nvme/004 (tr=loop) (test nvme and nvmet UUID NS descriptors) [passed]
runtime 0.900s ... 0.899s
nvme/005 (tr=loop) (reset local loopback target) [passed]
runtime 1.476s ... 1.456s
nvme/006 (tr=loop bd=device) (create an NVMeOF target) [passed]
runtime 0.107s ... 0.114s
nvme/006 (tr=loop bd=file) (create an NVMeOF target) [passed]
runtime 0.083s ... 0.078s
nvme/008 (tr=loop bd=device) (create an NVMeOF host) [passed]
runtime 0.886s ... 0.913s
nvme/008 (tr=loop bd=file) (create an NVMeOF host) [passed]
runtime 0.901s ... 0.887s
nvme/010 (tr=loop bd=device) (run data verification fio job) [passed]
runtime 77.012s ... 98.682s
nvme/010 (tr=loop bd=file) (run data verification fio job) [passed]
runtime 174.211s ... 280.958s
nvme/012 (tr=loop bd=device) (run mkfs and data verification fio) [passed]
runtime 91.545s ... 84.399s
nvme/012 (tr=loop bd=file) (run mkfs and data verification fio)
runtime 156.857s ...
nvme/012 (tr=loop bd=file) (run mkfs and data verification fio) [passed]
runtime 156.857s ... 239.848s
nvme/014 (tr=loop bd=device) (flush a command from host) [passed]
runtime 13.794s ... 13.422s
nvme/014 (tr=loop bd=file) (flush a command from host) [passed]
runtime 10.749s ... 11.194s
nvme/016 (tr=loop) (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 0.151s ... 0.170s
nvme/017 (tr=loop) (create/delete many file-ns and test discovery) [passed]
runtime 0.163s ... 0.167s
nvme/018 (tr=loop) (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 0.871s ... 0.873s
nvme/019 (tr=loop bd=device) (test NVMe DSM Discard command) [passed]
runtime 0.885s ... 0.902s
nvme/019 (tr=loop bd=file) (test NVMe DSM Discard command) [passed]
runtime 0.876s ... 0.872s
nvme/021 (tr=loop bd=device) (test NVMe list command) [passed]
runtime 0.887s ... 0.919s
nvme/021 (tr=loop bd=file) (test NVMe list command) [passed]
runtime 0.875s ... 0.878s
nvme/022 (tr=loop bd=device) (test NVMe reset command) [passed]
runtime 1.489s ... 1.494s
nvme/022 (tr=loop bd=file) (test NVMe reset command) [passed]
runtime 1.453s ... 1.646s
nvme/023 (tr=loop bd=device) (test NVMe smart-log command) [passed]
runtime 0.899s ... 0.904s
nvme/023 (tr=loop bd=file) (test NVMe smart-log command) [passed]
runtime 0.858s ... 0.881s
nvme/025 (tr=loop bd=device) (test NVMe effects-log) [passed]
runtime 0.907s ... 0.910s
nvme/025 (tr=loop bd=file) (test NVMe effects-log) [passed]
runtime 0.862s ... 0.908s
nvme/026 (tr=loop bd=device) (test NVMe ns-descs) [passed]
runtime 0.892s ... 0.909s
nvme/026 (tr=loop bd=file) (test NVMe ns-descs) [passed]
runtime 0.863s ... 0.893s
nvme/027 (tr=loop bd=device) (test NVMe ns-rescan command) [passed]
runtime 0.916s ... 0.931s
nvme/027 (tr=loop bd=file) (test NVMe ns-rescan command) [passed]
runtime 0.890s ... 0.884s
nvme/028 (tr=loop bd=device) (test NVMe list-subsys) [passed]
runtime 0.902s ... 0.882s
nvme/028 (tr=loop bd=file) (test NVMe list-subsys) [passed]
runtime 0.854s ... 0.842s
nvme/029 (tr=loop) (test userspace IO via nvme-cli read/write interface) [passed]
runtime 1.114s ... 1.126s
nvme/030 (tr=loop) (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.535s ... 0.527s
nvme/031 (tr=loop) (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 8.354s ... 8.276s
nvme/038 (tr=loop) (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.028s ... 0.029s
nvme/040 (tr=loop) (test nvme fabrics controller reset/disconnect operation during I/O) [passed]
runtime 7.411s ... 7.527s
nvme/041 (tr=loop) (Create authenticated connections) [passed]
runtime ... 0.923s
nvme/042 (tr=loop) (Test dhchap key types for authenticated connections)
WARNING: Test did not clean up port: 0
WARNING: Test did not clean up subsystem: blktests-subsystem-1
rmdir: failed to remove '/sys/kernel/config/nvmet//subsystems/blktests-subsystem-1': Direc...
[truncated message content] |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:57:55
|
__blkdev_issue_discard() always returns 0, making the error check
in blkdev_issue_discard() dead code.
In function blkdev_issue_discard() initialize ret = 0, remove ret
assignment from __blkdev_issue_discard(), rely on bio == NULL check to
call submit_bio_wait(), preserve submit_bio_wait() error handling, and
preserve -EOPNOTSUPP to 0 mapping.
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
block/blk-lib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 3030a772d3aa..19e0203cc18a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -87,11 +87,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
{
struct bio *bio = NULL;
struct blk_plug plug;
- int ret;
+ int ret = 0;
blk_start_plug(&plug);
- ret = __blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, &bio);
- if (!ret && bio) {
+ __blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, &bio);
+ if (bio) {
ret = submit_bio_wait(bio);
if (ret == -EOPNOTSUPP)
ret = 0;
--
2.40.0
|
|
From: Christoph H. <hc...@ls...> - 2025-11-24 06:26:44
|
Looks good: Reviewed-by: Christoph Hellwig <hc...@ls...> |
|
From: Johannes T. <Joh...@wd...> - 2025-11-24 08:07:02
|
Looks good, Reviewed-by: Johannes Thumshirn <joh...@wd...> |
|
From: Martin K. P. <mar...@or...> - 2025-11-24 12:59:04
|
Chaitanya, > __blkdev_issue_discard() always returns 0, making the error check > in blkdev_issue_discard() dead code. Reviewed-by: Martin K. Petersen <mar...@or...> -- Martin K. Petersen |
|
From: Wilfred M. <wil...@gm...> - 2025-11-25 23:33:47
|
Reviewed-by: Wilfred Mallawa <wil...@wd...> Regards, Wilfred |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:57:57
|
__blkdev_issue_discard() always returns 0, making all error checking
at call sites dead code.
For dm-thin change issue_discard() return type to void, in
passdown_double_checking_shared_status() remove the r assignment from
return value of the issue_discard(), for end_discard() hardcod value
of r to 0 that matches only value returned from
__blkdev_issue_discard().
md part is simplified to only check !discard_bio by ignoring the
__blkdev_issue_discard() value.
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
drivers/md/dm-thin.c | 12 +++++-------
drivers/md/md.c | 4 ++--
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index c84149ba4e38..77c76f75c85f 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -395,13 +395,13 @@ static void begin_discard(struct discard_op *op, struct thin_c *tc, struct bio *
op->bio = NULL;
}
-static int issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
+static void issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
{
struct thin_c *tc = op->tc;
sector_t s = block_to_sectors(tc->pool, data_b);
sector_t len = block_to_sectors(tc->pool, data_e - data_b);
- return __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
+ __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
}
static void end_discard(struct discard_op *op, int r)
@@ -1113,9 +1113,7 @@ static void passdown_double_checking_shared_status(struct dm_thin_new_mapping *m
break;
}
- r = issue_discard(&op, b, e);
- if (r)
- goto out;
+ issue_discard(&op, b, e);
b = e;
}
@@ -1188,8 +1186,8 @@ static void process_prepared_discard_passdown_pt1(struct dm_thin_new_mapping *m)
struct discard_op op;
begin_discard(&op, tc, discard_parent);
- r = issue_discard(&op, m->data_block, data_end);
- end_discard(&op, r);
+ issue_discard(&op, m->data_block, data_end);
+ end_discard(&op, 0);
}
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 7b5c5967568f..aeb62df39828 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9132,8 +9132,8 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
{
struct bio *discard_bio = NULL;
- if (__blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO,
- &discard_bio) || !discard_bio)
+ __blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO, &discard_bio);
+ if (!discard_bio)
return;
bio_chain(discard_bio, bio);
--
2.40.0
|
|
From: Yu K. <yu...@fn...> - 2025-11-24 06:38:25
|
Hi,
在 2025/11/24 10:57, Chaitanya Kulkarni 写道:
> __blkdev_issue_discard() always returns 0, making all error checking
> at call sites dead code.
>
> For dm-thin change issue_discard() return type to void, in
> passdown_double_checking_shared_status() remove the r assignment from
> return value of the issue_discard(), for end_discard() hardcod value
> of r to 0 that matches only value returned from
> __blkdev_issue_discard().
>
> md part is simplified to only check !discard_bio by ignoring the
> __blkdev_issue_discard() value.
>
> Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
> ---
> drivers/md/dm-thin.c | 12 +++++-------
> drivers/md/md.c | 4 ++--
> 2 files changed, 7 insertions(+), 9 deletions(-)
mdraid and dm are different drivers, please split them.
>
> diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
> index c84149ba4e38..77c76f75c85f 100644
> --- a/drivers/md/dm-thin.c
> +++ b/drivers/md/dm-thin.c
> @@ -395,13 +395,13 @@ static void begin_discard(struct discard_op *op, struct thin_c *tc, struct bio *
> op->bio = NULL;
> }
>
> -static int issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
> +static void issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
> {
> struct thin_c *tc = op->tc;
> sector_t s = block_to_sectors(tc->pool, data_b);
> sector_t len = block_to_sectors(tc->pool, data_e - data_b);
>
> - return __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
> + __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
> }
>
> static void end_discard(struct discard_op *op, int r)
> @@ -1113,9 +1113,7 @@ static void passdown_double_checking_shared_status(struct dm_thin_new_mapping *m
> break;
> }
>
> - r = issue_discard(&op, b, e);
> - if (r)
> - goto out;
> + issue_discard(&op, b, e);
>
> b = e;
> }
> @@ -1188,8 +1186,8 @@ static void process_prepared_discard_passdown_pt1(struct dm_thin_new_mapping *m)
> struct discard_op op;
>
> begin_discard(&op, tc, discard_parent);
> - r = issue_discard(&op, m->data_block, data_end);
> - end_discard(&op, r);
> + issue_discard(&op, m->data_block, data_end);
> + end_discard(&op, 0);
> }
> }
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 7b5c5967568f..aeb62df39828 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9132,8 +9132,8 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
> {
> struct bio *discard_bio = NULL;
>
> - if (__blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO,
> - &discard_bio) || !discard_bio)
> + __blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO, &discard_bio);
> + if (!discard_bio)
> return;
>
> bio_chain(discard_bio, bio);
--
Thanks
Kuai
|
|
From: Christoph H. <hc...@ls...> - 2025-11-24 06:28:08
|
On Mon, Nov 24, 2025 at 02:24:05PM +0800, Yu Kuai wrote: > mdraid and dm are different drivers, please split them. Yes. Both parts looks fine to me, though. So: Reviewed-by: Christoph Hellwig <hc...@ls...> for the next round. |
|
From: Johannes T. <Joh...@wd...> - 2025-11-24 08:09:16
|
With the two patches being split Reviewed-by: Johannes Thumshirn <joh...@wd...> |
|
From: Martin K. P. <mar...@or...> - 2025-11-24 13:00:45
|
Chaitanya, > __blkdev_issue_discard() always returns 0, making all error checking > at call sites dead code. > > For dm-thin change issue_discard() return type to void, in > passdown_double_checking_shared_status() remove the r assignment from > return value of the issue_discard(), for end_discard() hardcod value > of r to 0 that matches only value returned from > __blkdev_issue_discard(). > > md part is simplified to only check !discard_bio by ignoring the > __blkdev_issue_discard() value. With md and dm split: Reviewed-by: Martin K. Petersen <mar...@or...> -- Martin K. Petersen |
|
From: Wilfred M. <wil...@gm...> - 2025-11-25 23:41:38
|
On Sun, 2025-11-23 at 18:57 -0800, Chaitanya Kulkarni wrote: > __blkdev_issue_discard() always returns 0, making all error checking > at call sites dead code. > > For dm-thin change issue_discard() return type to void, in > passdown_double_checking_shared_status() remove the r assignment from > return value of the issue_discard(), for end_discard() hardcod value Hey Chaitanya, Typo here s/hardcod/hardcode. Otherwise, with the split as other have suggested: Reviewed-by: Wilfred Mallawa <wil...@wd...> Regards, Wilfred |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:58:00
|
__blkdev_issue_discard() always returns 0, making the error assignment
in __submit_discard_cmd() dead code.
Initialize err to 0 and remove the error assignment from the
__blkdev_issue_discard() call tp err. Move fault injection code into
already present if branch where err is set to -EIO.
This preserves the fault injection behavior while removing dead error
handling.
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
fs/f2fs/segment.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b45eace879d7..3dbcfb9067e9 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1343,15 +1343,9 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
dc->di.len += len;
+ err = 0;
if (time_to_inject(sbi, FAULT_DISCARD)) {
err = -EIO;
- } else {
- err = __blkdev_issue_discard(bdev,
- SECTOR_FROM_BLOCK(start),
- SECTOR_FROM_BLOCK(len),
- GFP_NOFS, &bio);
- }
- if (err) {
spin_lock_irqsave(&dc->lock, flags);
if (dc->state == D_PARTIAL)
dc->state = D_SUBMIT;
@@ -1360,6 +1354,10 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
break;
}
+ __blkdev_issue_discard(bdev,
+ SECTOR_FROM_BLOCK(start),
+ SECTOR_FROM_BLOCK(len),
+ GFP_NOFS, &bio);
f2fs_bug_on(sbi, !bio);
/*
--
2.40.0
|
|
From: Christoph H. <hc...@ls...> - 2025-11-24 06:30:26
|
On Sun, Nov 23, 2025 at 06:57:36PM -0800, Chaitanya Kulkarni wrote: > + __blkdev_issue_discard(bdev, > + SECTOR_FROM_BLOCK(start), > + SECTOR_FROM_BLOCK(len), > + GFP_NOFS, &bio); This can be shortened a bit as well: __blkdev_issue_discard(bdev, SECTOR_FROM_BLOCK(start), SECTOR_FROM_BLOCK(len), GFP_NOFS, &bio); Otherwise looks good: Reviewed-by: Christoph Hellwig <hc...@ls...> |
|
From: Johannes T. <Joh...@wd...> - 2025-11-24 08:03:46
|
On 11/24/25 3:58 AM, Chaitanya Kulkarni wrote: > __blkdev_issue_discard() always returns 0, making the error assignment > in __submit_discard_cmd() dead code. > > Initialize err to 0 and remove the error assignment from the > __blkdev_issue_discard() call tp err. Move fault injection code into s/tp/to/ > already present if branch where err is set to -EIO. > > This preserves the fault injection behavior while removing dead error > handling. Otherwise Reviewed-by: Johannes Thumshirn <joh...@wd...> |
|
From: Martin K. P. <mar...@or...> - 2025-11-24 12:59:35
|
Chaitanya, > __blkdev_issue_discard() always returns 0, making the error assignment > in __submit_discard_cmd() dead code. With Christoph's and Johannes' comments addressed. Reviewed-by: Martin K. Petersen <mar...@or...> -- Martin K. Petersen |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 02:58:01
|
__blkdev_issue_discard() always returns 0, making all error checking
in XFS discard functions dead code.
Change xfs_discard_extents() return type to void, remove error variable,
error checking, and error logging for the __blkdev_issue_discard() call
in same function.
Update xfs_trim_perag_extents() and xfs_trim_rtgroup_extents() to
ignore the xfs_discard_extents() return value and error checking
code.
Update xfs_discard_rtdev_extents() to ignore __blkdev_issue_discard()
return value and error checking code.
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
fs/xfs/xfs_discard.c | 27 +++++----------------------
fs/xfs/xfs_discard.h | 2 +-
2 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index 6917de832191..b6ffe4807a11 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -108,7 +108,7 @@ xfs_discard_endio(
* list. We plug and chain the bios so that we only need a single completion
* call to clear all the busy extents once the discards are complete.
*/
-int
+void
xfs_discard_extents(
struct xfs_mount *mp,
struct xfs_busy_extents *extents)
@@ -116,7 +116,6 @@ xfs_discard_extents(
struct xfs_extent_busy *busyp;
struct bio *bio = NULL;
struct blk_plug plug;
- int error = 0;
blk_start_plug(&plug);
list_for_each_entry(busyp, &extents->extent_list, list) {
@@ -126,18 +125,10 @@ xfs_discard_extents(
trace_xfs_discard_extent(xg, busyp->bno, busyp->length);
- error = __blkdev_issue_discard(btp->bt_bdev,
+ __blkdev_issue_discard(btp->bt_bdev,
xfs_gbno_to_daddr(xg, busyp->bno),
XFS_FSB_TO_BB(mp, busyp->length),
GFP_KERNEL, &bio);
- if (error && error != -EOPNOTSUPP) {
- xfs_info(mp,
- "discard failed for extent [0x%llx,%u], error %d",
- (unsigned long long)busyp->bno,
- busyp->length,
- error);
- break;
- }
}
if (bio) {
@@ -148,8 +139,6 @@ xfs_discard_extents(
xfs_discard_endio_work(&extents->endio_work);
}
blk_finish_plug(&plug);
-
- return error;
}
/*
@@ -385,9 +374,7 @@ xfs_trim_perag_extents(
* list after this function call, as it may have been freed by
* the time control returns to us.
*/
- error = xfs_discard_extents(pag_mount(pag), extents);
- if (error)
- break;
+ xfs_discard_extents(pag_mount(pag), extents);
if (xfs_trim_should_stop())
break;
@@ -496,12 +483,10 @@ xfs_discard_rtdev_extents(
trace_xfs_discard_rtextent(mp, busyp->bno, busyp->length);
- error = __blkdev_issue_discard(bdev,
+ __blkdev_issue_discard(bdev,
xfs_rtb_to_daddr(mp, busyp->bno),
XFS_FSB_TO_BB(mp, busyp->length),
GFP_NOFS, &bio);
- if (error)
- break;
}
xfs_discard_free_rtdev_extents(tr);
@@ -741,9 +726,7 @@ xfs_trim_rtgroup_extents(
* list after this function call, as it may have been freed by
* the time control returns to us.
*/
- error = xfs_discard_extents(rtg_mount(rtg), tr.extents);
- if (error)
- break;
+ xfs_discard_extents(rtg_mount(rtg), tr.extents);
low = tr.restart_rtx;
} while (!xfs_trim_should_stop() && low <= high);
diff --git a/fs/xfs/xfs_discard.h b/fs/xfs/xfs_discard.h
index 2b1a85223a56..8c5cc4af6a07 100644
--- a/fs/xfs/xfs_discard.h
+++ b/fs/xfs/xfs_discard.h
@@ -6,7 +6,7 @@ struct fstrim_range;
struct xfs_mount;
struct xfs_busy_extents;
-int xfs_discard_extents(struct xfs_mount *mp, struct xfs_busy_extents *busy);
+void xfs_discard_extents(struct xfs_mount *mp, struct xfs_busy_extents *busy);
int xfs_ioc_trim(struct xfs_mount *mp, struct fstrim_range __user *fstrim);
#endif /* XFS_DISCARD_H */
--
2.40.0
|
|
From: Christoph H. <hc...@ls...> - 2025-11-24 06:31:20
|
Looks good: Reviewed-by: Christoph Hellwig <hc...@ls...> |
|
From: Johannes T. <Joh...@wd...> - 2025-11-24 08:05:16
|
Looks good, Reviewed-by: Johannes Thumshirn <joh...@wd...> |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 23:48:32
|
Hi, __blkdev_issue_discard() only returns value 0, that makes post call error checking code dead. This patch series revmoes this dead code at all the call sites and adjust the callers. Please note that it doesn't change the return type of the function from int to void in this series, it will be done once this series gets merged smoothly. For f2fs and xfs I've ran following test which includes discard they produce same PASS and FAIL output with and without this patch series. for-next (lblk-fnext) discard-ret (lblk-discard) --------------------- -------------------------- FAIL f2fs/008 FAIL f2fs/008 FAIL f2fs/014 FAIL f2fs/014 FAIL f2fs/015 FAIL f2fs/015 PASS f2fs/017 PASS f2fs/017 PASS xfs/016 PASS xfs/016 PASS xfs/288 PASS xfs/288 PASS xfs/432 PASS xfs/432 PASS xfs/449 PASS xfs/449 PASS xfs/513 PASS xfs/513 PASS generic/033 PASS generic/033 PASS generic/038 PASS generic/038 PASS generic/098 PASS generic/098 PASS generic/224 PASS generic/224 PASS generic/251 PASS generic/251 PASS generic/260 PASS generic/260 PASS generic/288 PASS generic/288 PASS generic/351 PASS generic/351 PASS generic/455 PASS generic/455 PASS generic/457 PASS generic/457 PASS generic/470 PASS generic/470 PASS generic/482 PASS generic/482 PASS generic/500 PASS generic/500 PASS generic/537 PASS generic/537 PASS generic/608 PASS generic/608 PASS generic/619 PASS generic/619 PASS generic/746 PASS generic/746 PASS generic/757 PASS generic/757 For NVMeOF taret I've testing blktest with nvme_trtype=nvme_loop and all the testcases are passing. -ck Changes from V2:- 1. Add Reviewed-by: tags. 2. Split patch 2 into two separate patches dm and md. 3. Condense __blkdev_issue_discard() parameters for in nvmet patch. 4. Condense __blkdev_issue_discard() parameters for in f2fs patch. Chaitanya Kulkarni (6): block: ignore discard return value md: ignore discard return value dm: ignore discard return value nvmet: ignore discard return value f2fs: ignore discard return value xfs: ignore discard return value block/blk-lib.c | 6 +++--- drivers/md/dm-thin.c | 12 +++++------- drivers/md/md.c | 4 ++-- drivers/nvme/target/io-cmd-bdev.c | 28 +++++++--------------------- fs/f2fs/segment.c | 10 +++------- fs/xfs/xfs_discard.c | 27 +++++---------------------- fs/xfs/xfs_discard.h | 2 +- 7 files changed, 26 insertions(+), 63 deletions(-) -- 2.40.0 |
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 23:48:28
|
__blkdev_issue_discard() always returns 0, making the error check
in blkdev_issue_discard() dead code.
In function blkdev_issue_discard() initialize ret = 0, remove ret
assignment from __blkdev_issue_discard(), rely on bio == NULL check to
call submit_bio_wait(), preserve submit_bio_wait() error handling, and
preserve -EOPNOTSUPP to 0 mapping.
Reviewed-by: Christoph Hellwig <hc...@ls...>
Reviewed-by: Johannes Thumshirn <joh...@wd...>
Reviewed-by: Martin K. Petersen <mar...@or...>
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
block/blk-lib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 3030a772d3aa..19e0203cc18a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -87,11 +87,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
{
struct bio *bio = NULL;
struct blk_plug plug;
- int ret;
+ int ret = 0;
blk_start_plug(&plug);
- ret = __blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, &bio);
- if (!ret && bio) {
+ __blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, &bio);
+ if (bio) {
ret = submit_bio_wait(bio);
if (ret == -EOPNOTSUPP)
ret = 0;
--
2.40.0
|
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 23:48:33
|
__blkdev_issue_discard() always returns 0, making the error checking
in nvmet_bdev_discard_range() dead code.
Kill the function nvmet_bdev_discard_range() and call
__blkdev_issue_discard() directly from nvmet_bdev_execute_discard(),
since no error handling is needed anymore for __blkdev_issue_discard()
call.
Reviewed-by: Martin K. Petersen <mar...@or...>
Reviewed-by: Johannes Thumshirn <joh...@wd...>
Reviewed-by: Christoph Hellwig <hc...@ls...>
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
drivers/nvme/target/io-cmd-bdev.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 8d246b8ca604..ca7731048940 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -362,29 +362,14 @@ u16 nvmet_bdev_flush(struct nvmet_req *req)
return 0;
}
-static u16 nvmet_bdev_discard_range(struct nvmet_req *req,
- struct nvme_dsm_range *range, struct bio **bio)
-{
- struct nvmet_ns *ns = req->ns;
- int ret;
-
- ret = __blkdev_issue_discard(ns->bdev,
- nvmet_lba_to_sect(ns, range->slba),
- le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
- GFP_KERNEL, bio);
- if (ret && ret != -EOPNOTSUPP) {
- req->error_slba = le64_to_cpu(range->slba);
- return errno_to_nvme_status(req, ret);
- }
- return NVME_SC_SUCCESS;
-}
-
static void nvmet_bdev_execute_discard(struct nvmet_req *req)
{
+ struct nvmet_ns *ns = req->ns;
struct nvme_dsm_range range;
struct bio *bio = NULL;
+ sector_t nr_sects;
int i;
- u16 status;
+ u16 status = NVME_SC_SUCCESS;
for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
status = nvmet_copy_from_sgl(req, i * sizeof(range), &range,
@@ -392,9 +377,10 @@ static void nvmet_bdev_execute_discard(struct nvmet_req *req)
if (status)
break;
- status = nvmet_bdev_discard_range(req, &range, &bio);
- if (status)
- break;
+ nr_sects = le32_to_cpu(range.nlb) << (ns->blksize_shift - 9);
+ __blkdev_issue_discard(ns->bdev,
+ nvmet_lba_to_sect(ns, range.slba), nr_sects,
+ GFP_KERNEL, &bio);
}
if (bio) {
--
2.40.0
|
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 23:48:31
|
__blkdev_issue_discard() always returns 0, making all error checking
at call sites dead code.
For dm-thin change issue_discard() return type to void, in
passdown_double_checking_shared_status() remove the r assignment from
return value of the issue_discard(), for end_discard() hardcode value of
r to 0 that matches only value returned from __blkdev_issue_discard().
Reviewed-by: Martin K. Petersen <mar...@or...>
Reviewed-by: Johannes Thumshirn <joh...@wd...>
Reviewed-by: Christoph Hellwig <hc...@ls...>
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
drivers/md/dm-thin.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index c84149ba4e38..77c76f75c85f 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -395,13 +395,13 @@ static void begin_discard(struct discard_op *op, struct thin_c *tc, struct bio *
op->bio = NULL;
}
-static int issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
+static void issue_discard(struct discard_op *op, dm_block_t data_b, dm_block_t data_e)
{
struct thin_c *tc = op->tc;
sector_t s = block_to_sectors(tc->pool, data_b);
sector_t len = block_to_sectors(tc->pool, data_e - data_b);
- return __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
+ __blkdev_issue_discard(tc->pool_dev->bdev, s, len, GFP_NOIO, &op->bio);
}
static void end_discard(struct discard_op *op, int r)
@@ -1113,9 +1113,7 @@ static void passdown_double_checking_shared_status(struct dm_thin_new_mapping *m
break;
}
- r = issue_discard(&op, b, e);
- if (r)
- goto out;
+ issue_discard(&op, b, e);
b = e;
}
@@ -1188,8 +1186,8 @@ static void process_prepared_discard_passdown_pt1(struct dm_thin_new_mapping *m)
struct discard_op op;
begin_discard(&op, tc, discard_parent);
- r = issue_discard(&op, m->data_block, data_end);
- end_discard(&op, r);
+ issue_discard(&op, m->data_block, data_end);
+ end_discard(&op, 0);
}
}
--
2.40.0
|
|
From: Chaitanya K. <cku...@gm...> - 2025-11-24 23:48:34
|
__blkdev_issue_discard() always returns 0, making the error assignment
in __submit_discard_cmd() dead code.
Initialize err to 0 and remove the error assignment from the
__blkdev_issue_discard() call to err. Move fault injection code into
already present if branch where err is set to -EIO.
This preserves the fault injection behavior while removing dead error
handling.
Reviewed-by: Martin K. Petersen <mar...@or...>
Reviewed-by: Johannes Thumshirn <joh...@wd...>
Reviewed-by: Christoph Hellwig <hc...@ls...>
Signed-off-by: Chaitanya Kulkarni <cku...@gm...>
---
fs/f2fs/segment.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b45eace879d7..22b736ec9c51 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1343,15 +1343,9 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
dc->di.len += len;
+ err = 0;
if (time_to_inject(sbi, FAULT_DISCARD)) {
err = -EIO;
- } else {
- err = __blkdev_issue_discard(bdev,
- SECTOR_FROM_BLOCK(start),
- SECTOR_FROM_BLOCK(len),
- GFP_NOFS, &bio);
- }
- if (err) {
spin_lock_irqsave(&dc->lock, flags);
if (dc->state == D_PARTIAL)
dc->state = D_SUBMIT;
@@ -1360,6 +1354,8 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
break;
}
+ __blkdev_issue_discard(bdev, SECTOR_FROM_BLOCK(start),
+ SECTOR_FROM_BLOCK(len), GFP_NOFS, &bio);
f2fs_bug_on(sbi, !bio);
/*
--
2.40.0
|
Thanks for helping keep SourceForge clean.
X