From 04ea1579bc7707366de1d642115ad3b65c6171e1 Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Wed, 5 Aug 2026 13:16:11 +0200 Subject: [PATCH] s390/dasd: Re-enable discard support for ESE volumes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-enable block-layer discard for ESE ECKD volumes, releasing thin space via release allocated space (RAS). This is based on commit 7e64db1597fe ("s390/dasd: Add discard support for ESE volumes") but adapted to the current code and fixed. REQ_OP_DISCARD is routed to a RAS release over the request's track range, and discard requests run on the base device only. Discard limits use extent granularity via the disc_limits discipline hook so the block layer only issues extent-aligned discards. Discard is gated on the DASD_FEATURE_DISCARD device feature rather than a per-discipline flag: the driver sets the feature when the volume is on ESE hardware (i.e. RAS is available), and the block-layer setup enables discard limits for a device that has it. Reviewed-by: Jan Höppner Signed-off-by: Stefan Haberland Link: https://patch.msgid.link/20260805111612.1285190-19-sth@linux.ibm.com Signed-off-by: Jens Axboe --- drivers/s390/block/dasd.c | 34 +++++-- drivers/s390/block/dasd_eckd.c | 173 ++++++++++++++++++++++++++------- drivers/s390/block/dasd_int.h | 2 + 3 files changed, 166 insertions(+), 43 deletions(-) diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index 0e01b498b0e8..da5e6813d391 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -353,17 +353,19 @@ static int dasd_state_basic_to_ready(struct dasd_device *device) */ lim.dma_alignment = lim.logical_block_size - 1; - if (device->discipline->has_discard) { + if (device->features & DASD_FEATURE_DISCARD) { unsigned int max_bytes; - lim.discard_granularity = block->bp_block; - - /* Calculate max_discard_sectors and make it PAGE aligned */ - max_bytes = USHRT_MAX * block->bp_block; - max_bytes = ALIGN_DOWN(max_bytes, PAGE_SIZE); - - lim.max_hw_discard_sectors = max_bytes / block->bp_block; - lim.max_write_zeroes_sectors = lim.max_hw_discard_sectors; + if (device->discipline->disc_limits) { + device->discipline->disc_limits(block, &lim); + } else { + lim.discard_granularity = block->bp_block; + /* Calculate max_discard_sectors and make it PAGE aligned */ + max_bytes = USHRT_MAX * block->bp_block; + max_bytes = ALIGN_DOWN(max_bytes, PAGE_SIZE); + lim.max_hw_discard_sectors = max_bytes / block->bp_block; + lim.max_write_zeroes_sectors = lim.max_hw_discard_sectors; + } } rc = queue_limits_commit_update(block->gdp->queue, &lim); if (rc) @@ -3124,6 +3126,7 @@ static blk_status_t do_dasd_request(struct blk_mq_hw_ctx *hctx, struct dasd_device *basedev; struct dasd_ccw_req *cqr; blk_status_t rc = BLK_STS_OK; + bool complete_noop = false; basedev = block->base; spin_lock_irq(&dq->lock); @@ -3172,6 +3175,17 @@ static blk_status_t do_dasd_request(struct blk_mq_hw_ctx *hctx, rc = BLK_STS_RESOURCE; } else if (PTR_ERR(cqr) == -EINVAL) { rc = BLK_STS_INVAL; + } else if (PTR_ERR(cqr) == -EOPNOTSUPP) { + /* + * A discard that covers no whole extent releases + * nothing. Discard is advisory, so complete it as a + * benign no-op: the device does support discard, this + * range just does not align to the large ESE extent + * granularity. + * Completed after the lock is dropped. + */ + rc = BLK_STS_OK; + complete_noop = true; } else { DBF_DEV_EVENT(DBF_ERR, basedev, "CCW creation failed (rc=%ld) on request %p", @@ -3205,6 +3219,8 @@ static blk_status_t do_dasd_request(struct blk_mq_hw_ctx *hctx, out: spin_unlock_irq(&dq->lock); + if (complete_noop) + blk_mq_end_request(req, BLK_STS_OK); return rc; } diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index c2c60530fe0d..6eb2879b479b 100644 --- a/drivers/s390/block/dasd_eckd.c +++ b/drivers/s390/block/dasd_eckd.c @@ -2326,6 +2326,18 @@ dasd_eckd_check_characteristics(struct dasd_device *device) /* Read Volume Information */ dasd_eckd_read_vol_info(device); + /* + * Advertise discard through the device feature so the block layer sets + * up discard limits. Discard releases allocated space, so require a thin + * (ESE) volume whose storage reports support for the space-release + * function. Raw-track access bypasses the normal block CCW path (discard + * would reach the raw builder, which has no record data), so exclude it. + */ + if (dasd_eckd_ese_capable(device) && + (private->features.feature[56] & 0x01) && + !(device->features & DASD_FEATURE_USERAW)) + device->features |= DASD_FEATURE_DISCARD; + /* Read the on-disk format label for ESE detection */ dasd_eckd_read_format_label(device); @@ -4133,37 +4145,13 @@ static int dasd_eckd_ras_sanity_checks(struct dasd_device *device, } /* - * Helper function to count the amount of involved extents within a given range - * with extent alignment in mind. + * Number of extents the track range [from, to] spans. Extent n covers tracks + * [n * trks_per_ext, (n + 1) * trks_per_ext - 1], so the range touches the + * extents from (from / trks_per_ext) to (to / trks_per_ext) inclusive. */ static int count_exts(unsigned int from, unsigned int to, int trks_per_ext) { - int cur_pos = 0; - int count = 0; - int tmp; - - if (from == to) - return 1; - - /* Count first partial extent */ - if (from % trks_per_ext != 0) { - tmp = from + trks_per_ext - (from % trks_per_ext) - 1; - if (tmp > to) - tmp = to; - cur_pos = tmp - from + 1; - count++; - } - /* Count full extents */ - if (to - (from + cur_pos) + 1 >= trks_per_ext) { - tmp = to - ((to - trks_per_ext + 1) % trks_per_ext); - count += (tmp - (from + cur_pos) + 1) / trks_per_ext; - cur_pos = tmp; - } - /* Count last partial extent */ - if (cur_pos < to) - count++; - - return count; + return to / trks_per_ext - from / trks_per_ext + 1; } static int dasd_in_copy_relation(struct dasd_device *device) @@ -4214,9 +4202,17 @@ dasd_eckd_dso_ras(struct dasd_device *device, struct dasd_block *block, if (dasd_eckd_ras_sanity_checks(device, first_trk, last_trk)) return ERR_PTR(-EINVAL); - copy_relation = dasd_in_copy_relation(device); - if (copy_relation < 0) - return ERR_PTR(copy_relation); + /* + * The block-layer discard path (req != NULL) runs in atomic context, so + * it must not issue the sleeping copy-relation (PPRC) query. It also + * leaves guarantee_init off - discard does not promise zeroing anyway. + */ + copy_relation = 0; + if (!req) { + copy_relation = dasd_in_copy_relation(device); + if (copy_relation < 0) + return ERR_PTR(copy_relation); + } rq = req ? blk_mq_rq_to_pdu(req) : NULL; @@ -4248,7 +4244,7 @@ dasd_eckd_dso_ras(struct dasd_device *device, struct dasd_block *block, * not fully specified, but is only supported with a certain feature * subset and for devices not in a copy relation. */ - if (features->feature[56] & 0x01 && !copy_relation) + if (!req && features->feature[56] & 0x01 && !copy_relation) ras_data->op_flags.guarantee_init = 1; ras_data->lss = private->conf.ned->ID; @@ -4344,6 +4340,9 @@ static int dasd_eckd_release_space_trks(struct dasd_device *device, INIT_LIST_HEAD(&ras_queue); + if (dasd_eckd_ext_size(device) == 0) + return -EINVAL; + device_exts = private->real_cyl / dasd_eckd_ext_size(device); trks_per_ext = dasd_eckd_ext_size(device) * private->rdc_data.trk_per_cyl; @@ -5481,6 +5480,58 @@ dasd_eckd_build_cp_tpm_writefulltrack(struct dasd_device *startdev, return ERR_PTR(ret); } +static struct dasd_ccw_req * +dasd_eckd_build_cp_discard(struct dasd_device *device, struct dasd_block *block, + struct request *req, sector_t first_trk, + sector_t last_trk, unsigned int first_offs, + unsigned int last_offs, unsigned int blk_per_trk) +{ + struct dasd_eckd_private *private = device->private; + sector_t first_ext_trk, last_ext_end, last_ext_trk; + unsigned int trks_per_ext; + + trks_per_ext = dasd_eckd_ext_size(device) * private->rdc_data.trk_per_cyl; + if (!trks_per_ext) + return ERR_PTR(-EOPNOTSUPP); + + /* + * A discard range is rarely track-aligned: fstrim is FS-block granular + * and discard_granularity is only a hint. If it starts or ends mid-track, + * that boundary track still holds live records outside the range, so drop + * it from the whole-track span first. Otherwise a partial boundary track + * that happens to sit on an extent boundary would be released together + * with its live records resulting in silent data loss + */ + if (first_offs) /* partial first track */ + first_trk++; + if (last_offs != blk_per_trk - 1) { /* partial last track */ + if (!last_trk) + return ERR_PTR(-EOPNOTSUPP); + last_trk--; + } + if (first_trk > last_trk) + return ERR_PTR(-EOPNOTSUPP); /* no whole track fully covered */ + + /* + * RAS releases whole extents. Only release extents that lie entirely + * within the (now whole-track) discard range by rounding inward to extent + * boundaries - an extent shared with a live allocation must never be + * released. If no whole extent is covered there is nothing to release + * safely (e.g. a sub-extent discard, unavoidable with large extents), so + * reject the request rather than release too much. + */ + first_ext_trk = roundup(first_trk, trks_per_ext); + /* one past the last whole extent inside the range (exclusive) */ + last_ext_end = rounddown(last_trk + 1, trks_per_ext); + if (first_ext_trk >= last_ext_end) + return ERR_PTR(-EOPNOTSUPP); + /* inclusive last track; the guard above keeps this from underflowing */ + last_ext_trk = last_ext_end - 1; + + return dasd_eckd_dso_ras(device, block, req, first_ext_trk, + last_ext_trk, 1); +} + static struct dasd_ccw_req *dasd_eckd_build_cp(struct dasd_device *startdev, struct dasd_block *block, struct request *req) @@ -5519,6 +5570,12 @@ static struct dasd_ccw_req *dasd_eckd_build_cp(struct dasd_device *startdev, last_offs = sector_div(last_trk, blk_per_trk); cdlspecial = (private->uses_cdl && first_rec < 2*blk_per_trk); + if (req_op(req) == REQ_OP_DISCARD) + return dasd_eckd_build_cp_discard(startdev, block, req, + first_trk, last_trk, + first_offs, last_offs, + blk_per_trk); + fcx_multitrack = private->features.feature[40] & 0x20; data_size = blk_rq_bytes(req); if (data_size % blksize || data_size == 0) @@ -5832,11 +5889,13 @@ static struct dasd_ccw_req *dasd_eckd_build_alias_cp(struct dasd_device *base, struct request *req) { struct dasd_eckd_private *private; - struct dasd_device *startdev; + struct dasd_device *startdev = NULL; unsigned long flags; struct dasd_ccw_req *cqr; - startdev = dasd_alias_get_start_dev(base); + /* Discard requests (space release) can only run on the base device. */ + if (req_op(req) != REQ_OP_DISCARD) + startdev = dasd_alias_get_start_dev(base); if (!startdev) startdev = base; private = startdev->private; @@ -7724,6 +7783,51 @@ static unsigned int dasd_eckd_max_sectors(struct dasd_block *block) return DASD_ECKD_MAX_BLOCKS << block->s2b_shift; } +/* + * Discard on ECKD releases space through RAS, which works on whole extents. + * Advertise extent granularity so the block layer only sends extent-aligned + * discards (avoiding partially specified extents), and only for volumes on ESE + * hardware. Non-ESE devices are left without discard limits. + */ +static void dasd_eckd_disc_limits(struct dasd_block *block, + struct queue_limits *lim) +{ + struct dasd_device *device = block->base; + struct dasd_eckd_private *private = device->private; + unsigned int logical_block_size = block->bp_block; + unsigned int max_discard_sectors, max_bytes, ext_bytes; + int recs_per_trk, trks_per_cyl, ext_limit, ext_size; + + if (!dasd_eckd_ese_capable(device) || dasd_eckd_ext_size(device) == 0) + return; + + trks_per_cyl = private->rdc_data.trk_per_cyl; + recs_per_trk = recs_per_track(&private->rdc_data, 0, logical_block_size); + + ext_size = dasd_eckd_ext_size(device); + ext_limit = min(private->real_cyl / ext_size, DASD_ECKD_RAS_EXTS_MAX); + ext_bytes = ext_size * trks_per_cyl * recs_per_trk * logical_block_size; + if (!ext_bytes) /* malformed RDC data - leave discard unset */ + return; + max_bytes = UINT_MAX - (UINT_MAX % ext_bytes); + if (max_bytes / ext_bytes > ext_limit) + max_bytes = ext_bytes * ext_limit; + + max_discard_sectors = max_bytes / 512; + + lim->max_hw_discard_sectors = max_discard_sectors; + /* + * ext_bytes is the hardware extent size and is not a power of two, so + * the block layer's power-of-two round_up()/round_down() alignment + * helpers compute it only approximately. That is a hint, not a + * correctness requirement: RAS safety is enforced in the CCW builder, + * which rounds the range inward to whole extents and rejects a request + * that covers no whole extent, so a misaligned range is never + * over-released. At worst a few sub-extent discards are declined. + */ + lim->discard_granularity = ext_bytes; +} + static struct ccw_driver dasd_eckd_driver = { .driver = { .name = "dasd-eckd", @@ -7746,6 +7850,7 @@ static struct dasd_discipline dasd_eckd_discipline = { .owner = THIS_MODULE, .name = "ECKD", .ebcname = "ECKD", + .disc_limits = dasd_eckd_disc_limits, .check_device = dasd_eckd_check_characteristics, .uncheck_device = dasd_eckd_uncheck_device, .do_analysis = dasd_eckd_do_analysis, diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h index 8c73850f7947..ef4930432c09 100644 --- a/drivers/s390/block/dasd_int.h +++ b/drivers/s390/block/dasd_int.h @@ -404,6 +404,8 @@ struct dasd_discipline { int (*ese_capable)(struct dasd_device *); /* Whether the volume is formatted on demand (thin), from the label */ int (*on_demand_format)(struct dasd_device *); + /* Fill discard queue limits */ + void (*disc_limits)(struct dasd_block *, struct queue_limits *); /* Capacity */ int (*space_allocated)(struct dasd_device *); int (*space_configured)(struct dasd_device *);