On 11/18/25 00:04, Christoph Hellwig wrote:
> On Mon, Nov 17, 2025 at 11:42:43PM -0800, Chaitanya Kulkarni wrote:
>> Due to involvement of all the subsystem making it as an RFC, ideally
>> it shuoldn't be an RFC.
> I think best would be a series that drops error checking first,
> and then changes the return type. That way we can maybe get all
> the callers fixed up in this merge window and then drop the return
> value after -rc1.
thanks for the suggestion
>> gfp_mask)))
>> *biop = bio_chain_and_submit(*biop, bio);
>> - return 0;
>> }
>> EXPORT_SYMBOL(__blkdev_issue_discard);
>>
>> @@ -90,8 +89,8 @@ int blkdev_issue_discard(struct block_device *bdev,
>> sector_t sector,
>> int ret;
>>
>> 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);
> ret now needs to be initialized to 0 above.
done.
>
>> index 8d246b8ca604..f26010c46c33 100644
>> --- a/drivers/nvme/target/io-cmd-bdev.c
>> +++ b/drivers/nvme/target/io-cmd-bdev.c
>> @@ -366,16 +366,11 @@ 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,
>> + __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;
> nvmet_bdev_discard_range can return void now.
done.
>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index b45eace879d7..e6078176f733 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1346,7 +1346,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info
>> *sbi,
>> if (time_to_inject(sbi, FAULT_DISCARD)) {
>> err = -EIO;
>> } else {
>> - err = __blkdev_issue_discard(bdev,
>> + __blkdev_issue_discard(bdev,
>> SECTOR_FROM_BLOCK(start),
>> SECTOR_FROM_BLOCK(len),
>> GFP_NOFS, &bio);
> Please fold the following 'if (err)' block directly into the injection
> one, and either initialize err to 0, or use a direct return from that
> block to skip the last branch in the function checking err.
done :-
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
>
>> blk_finish_plug(&plug);
>>
>> - return error;
>> + return 0;
> Please drop the error return for xfs_discard_extents entirely.
>
done :-
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index ee49f20875af..1f35c1d80cea 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);
@@ -739,9 +724,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
will run the basic xfstest and send out a series to just remove the dead-code.
Thanks for the review comments.
-ck
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
