| Message ID | 20220207141348.4235-9-nj.shetty@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,01/10] block: make bio_map_kern() non static | expand |
On Mon, 7 Feb 2022, Nitesh Shetty wrote: > Before enabling copy for dm target, check if underlaying devices and > dm target support copy. Avoid split happening inside dm target. > Fail early if the request needs split, currently spliting copy > request is not supported > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> If a dm device is reconfigured, you must invalidate all the copy tokens that are in flight, otherwise they would copy stale data. I suggest that you create a global variable "atomic64_t dm_changed". In nvme_setup_copy_read you copy this variable to the token. In nvme_setup_copy_write you compare the variable with the value in the token and fail if there is mismatch. In dm.c:__bind you increase the variable, so that all the tokens will be invalidated if a dm table is changed. Mikulas
On Wed, Feb 16, 2022 at 08:51:08AM -0500, Mikulas Patocka wrote: > > > On Mon, 7 Feb 2022, Nitesh Shetty wrote: > > > Before enabling copy for dm target, check if underlaying devices and > > dm target support copy. Avoid split happening inside dm target. > > Fail early if the request needs split, currently spliting copy > > request is not supported > > > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > > If a dm device is reconfigured, you must invalidate all the copy tokens > that are in flight, otherwise they would copy stale data. > > I suggest that you create a global variable "atomic64_t dm_changed". > In nvme_setup_copy_read you copy this variable to the token. > In nvme_setup_copy_write you compare the variable with the value in the > token and fail if there is mismatch. > In dm.c:__bind you increase the variable, so that all the tokens will be > invalidated if a dm table is changed. > > Mikulas > > Yes, you are right about the reconfiguration of dm device. But wouldn't having a single global counter(dm_changed), will invalidate for all in-flight copy IO's across all dm devices. Is my understanding correct? -- Nitesh Shetty
On Thu, 24 Feb 2022, Nitesh Shetty wrote: > On Wed, Feb 16, 2022 at 08:51:08AM -0500, Mikulas Patocka wrote: > > > > > > On Mon, 7 Feb 2022, Nitesh Shetty wrote: > > > > > Before enabling copy for dm target, check if underlaying devices and > > > dm target support copy. Avoid split happening inside dm target. > > > Fail early if the request needs split, currently spliting copy > > > request is not supported > > > > > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > > > > If a dm device is reconfigured, you must invalidate all the copy tokens > > that are in flight, otherwise they would copy stale data. > > > > I suggest that you create a global variable "atomic64_t dm_changed". > > In nvme_setup_copy_read you copy this variable to the token. > > In nvme_setup_copy_write you compare the variable with the value in the > > token and fail if there is mismatch. > > In dm.c:__bind you increase the variable, so that all the tokens will be > > invalidated if a dm table is changed. > > > > Mikulas > > > > > Yes, you are right about the reconfiguration of dm device. But wouldn't having a > single global counter(dm_changed), will invalidate for all in-flight copy IO's > across all dm devices. Is my understanding correct? > > -- > Nitesh Shetty Yes, changing it will invalidate all the copy IO's. But invalidating only IO's affected by the table reload would be hard to achieve. Mikulas
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index e43096cfe9e2..cb5cdaf1d8b9 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1903,6 +1903,39 @@ static bool dm_table_supports_nowait(struct dm_table *t) return true; } +static int device_not_copy_capable(struct dm_target *ti, struct dm_dev *dev, + sector_t start, sector_t len, void *data) +{ + struct request_queue *q = bdev_get_queue(dev->bdev); + + return !blk_queue_copy(q); +} + +static bool dm_table_supports_copy(struct dm_table *t) +{ + struct dm_target *ti; + unsigned int i; + + for (i = 0; i < dm_table_get_num_targets(t); i++) { + ti = dm_table_get_target(t, i); + + if (!ti->copy_supported) + return false; + + /* + * Either the target provides copy support (as implied by setting + * 'copy_supported') or it relies on _all_ data devices having + * discard support. + */ + if (!ti->copy_supported && + (!ti->type->iterate_devices || + ti->type->iterate_devices(ti, device_not_copy_capable, NULL))) + return false; + } + + return true; +} + static int device_not_discard_capable(struct dm_target *ti, struct dm_dev *dev, sector_t start, sector_t len, void *data) { @@ -2000,6 +2033,16 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, } else blk_queue_flag_set(QUEUE_FLAG_DISCARD, q); + if (!dm_table_supports_copy(t)) { + blk_queue_flag_clear(QUEUE_FLAG_COPY, q); + /* Must also clear discard limits... */ + q->limits.max_copy_sectors = 0; + q->limits.max_copy_range_sectors = 0; + q->limits.max_copy_nr_ranges = 0; + } else { + blk_queue_flag_set(QUEUE_FLAG_COPY, q); + } + if (dm_table_supports_secure_erase(t)) blk_queue_flag_set(QUEUE_FLAG_SECERASE, q); diff --git a/drivers/md/dm.c b/drivers/md/dm.c index fa596b654c99..2a6d55722139 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1420,6 +1420,12 @@ static int __split_and_process_non_flush(struct clone_info *ci) if (__process_abnormal_io(ci, ti, &r)) return r; + if ((unlikely(op_is_copy(ci->bio->bi_opf)) && + max_io_len(ti, ci->sector) < ci->sector_count)) { + DMERR("%s: Error IO size(%u) is greater than maximum target size(%llu)\n", + __func__, ci->sector_count, max_io_len(ti, ci->sector)); + return -EIO; + } len = min_t(sector_t, max_io_len(ti, ci->sector), ci->sector_count); r = __clone_and_map_data_bio(ci, ti, ci->sector, &len); diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index b26fecf6c8e8..acfd4018125a 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -362,6 +362,11 @@ struct dm_target { * zone append operations using regular writes. */ bool emulate_zone_append:1; + + /* + * copy offload is supported + */ + bool copy_supported:1; }; void *dm_per_bio_data(struct bio *bio, size_t data_size);
Before enabling copy for dm target, check if underlaying devices and dm target support copy. Avoid split happening inside dm target. Fail early if the request needs split, currently spliting copy request is not supported Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> --- drivers/md/dm-table.c | 43 +++++++++++++++++++++++++++++++++++ drivers/md/dm.c | 6 +++++ include/linux/device-mapper.h | 5 ++++ 3 files changed, 54 insertions(+)