Messages in this thread Patch in this message | | | Date | Thu, 16 May 2024 09:55:53 +0800 | Subject | Re: [PATCH 3/5] dm: support retrieving struct dm_target from struct dm_dev | From | YangYang <> |
| |
On 2024/5/15 23:42, Benjamin Marzinski wrote: > On Tue, May 14, 2024 at 05:04:42PM +0800, Yang Yang wrote: >> Add a list to the struct dm_dev structure to store the associated >> targets, while also allowing differentiation between different target >> types. > > I still think this is more complex than it needs to be. If devices that > support flush_pass_around can guarantee that: > > 1. They will send a flush bio to all of their table devices > 2. They are fine with another target sending the flush bio to their > table devices > > Then I don't see why we need the table devices to keep track of all the > different target types that are using them. Am I missing something here?
I attempted to enhance this solution to support additional target types, such as those with num_flush_bios greater than 1.
> If we don't need to worry about sending a flush bio to a target of each > type that is using a table device, then all we need to do is call > __send_empty_flush_bios() for enough targets to cover all the table > devices. This seems a lot easier to track. We just need another flag in > dm_target, something like sends_pass_around_flush. > > When a target calls dm_get_device(), if it adds a new table device to > t->devices, then it's the first target in this table to use that device. > If flush_pass_around is set for this target, then it also sets > sends_pass_around_flush. In __send_empty_flush() if the table has > flush_pass_around set, when you iterate through the devices, you only > call __send_empty_flush_bios() for the ones with sends_pass_around_flush > set. > > Or am I overlooking something?
If I understand correctly, you are suggesting to iterate through all the targets, handling those with sends_pass_around_flush set, and skipping those where sends_pass_around_flush is not set. I believe this approach may result in some CPU wastage.
for i in {0..1023}; do echo $((8000*$i)) 8000 linear /dev/sda2 $((16384*$i)) done | sudo dmsetup create example
In this specific scenario, a single iteration of the loop is all that is needed.
> > -Ben > >> >> Signed-off-by: Yang Yang <yang.yang@vivo.com> >> --- >> drivers/md/dm-table.c | 36 +++++++++++++++++++++++++++++++++++ >> include/linux/device-mapper.h | 3 +++ >> 2 files changed, 39 insertions(+) >> >> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c >> index bd68af10afed..f6554590b7af 100644 >> --- a/drivers/md/dm-table.c >> +++ b/drivers/md/dm-table.c >> @@ -741,6 +741,8 @@ int dm_table_add_target(struct dm_table *t, const char *type, >> if (ti->flush_pass_around == 0) >> t->flush_pass_around = 0; >> >> + INIT_LIST_HEAD(&ti->list); >> + >> return 0; >> >> bad: >> @@ -2134,6 +2136,25 @@ void dm_table_postsuspend_targets(struct dm_table *t) >> suspend_targets(t, POSTSUSPEND); >> } >> >> +static int dm_link_dev_to_target(struct dm_target *ti, struct dm_dev *dev, >> + sector_t start, sector_t len, void *data) >> +{ >> + struct list_head *targets = &dev->targets; >> + struct dm_target *pti; >> + >> + if (!list_empty(targets)) { >> + list_for_each_entry(pti, targets, list) { >> + if (pti->type == ti->type) >> + return 0; >> + } >> + } >> + >> + if (list_empty(&ti->list)) >> + list_add_tail(&ti->list, targets); >> + >> + return 0; >> +} >> + >> int dm_table_resume_targets(struct dm_table *t) >> { >> unsigned int i; >> @@ -2162,6 +2183,21 @@ int dm_table_resume_targets(struct dm_table *t) >> ti->type->resume(ti); >> } >> >> + if (t->flush_pass_around) { >> + struct list_head *devices = &t->devices; >> + struct dm_dev_internal *dd; >> + >> + list_for_each_entry(dd, devices, list) >> + INIT_LIST_HEAD(&dd->dm_dev->targets); >> + >> + for (i = 0; i < t->num_targets; i++) { >> + struct dm_target *ti = dm_table_get_target(t, i); >> + >> + if (ti->type->iterate_devices) >> + ti->type->iterate_devices(ti, dm_link_dev_to_target, NULL); >> + } >> + } >> + >> return 0; >> } >> >> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h >> index 0893ff8c01b6..19e03f9b2589 100644 >> --- a/include/linux/device-mapper.h >> +++ b/include/linux/device-mapper.h >> @@ -169,6 +169,7 @@ struct dm_dev { >> struct dax_device *dax_dev; >> blk_mode_t mode; >> char name[16]; >> + struct list_head targets; >> }; >> >> /* >> @@ -298,6 +299,8 @@ struct dm_target { >> struct dm_table *table; >> struct target_type *type; >> >> + struct list_head list; >> + >> /* target limits */ >> sector_t begin; >> sector_t len; >> -- >> 2.34.1 >> >
| |