On 11/25/25 07:48, Chaitanya Kulkarni wrote:
__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.

Reviewed-by: Johannes Thumshirn <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
  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 blk_alloc_discard_bio() fails to allocate a bio inside
__blkdev_issue_discard(), this may lead to an invalid loop in
list_for_each_entry{}. Instead of using __blkdev_issue_discard(), how
about allocate and submit the discard bios explicitly in
list_for_each_entry{}?

Yongpeng,

-               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 */



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to