On Thu, Jan 11 2024 at 11:34P -0500,
Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> On Thu, Jan 11 2024 at 11:24P -0500,
> Martin K. Petersen <martin.petersen@xxxxxxxxxx> wrote:
>
> >
> > > Bw, can someone help with what dm_integrity_profile is for?
> > > It is basically identical to the no-op one, just with a different
> > > name. With the no-op removal it is the only one outside of the pi
> > > once, and killing it would really help with some de-virtualization
> > > I've looked at a while ago.
> >
> > No particular objections from me wrt. using a flag.
> >
> > However, I believe the no-op profile and associated plumbing was a
> > requirement for DM. I forget the details. Mike?
>
> I'll have to take a closer look.. stacking device always complicates
> things.
The bulk of the following drivers/md/dm-table.c code snippets are from
these 2 commits (first from me, 2nd from Martin):
a63a5cf84dac dm: improve block integrity support
25520d55cdb6 block: Inline blk_integrity in struct gendisk
static bool integrity_profile_exists(struct gendisk *disk)
{
return !!blk_get_integrity(disk);
}
/*
* Get a disk whose integrity profile reflects the table's profile.
* Returns NULL if integrity support was inconsistent or unavailable.
*/
static struct gendisk *dm_table_get_integrity_disk(struct dm_table *t)
{
struct list_head *devices = dm_table_get_devices(t);
struct dm_dev_internal *dd = NULL;
struct gendisk *prev_disk = NULL, *template_disk = NULL;
for (unsigned int i = 0; i < t->num_targets; i++) {
struct dm_target *ti = dm_table_get_target(t, i);
if (!dm_target_passes_integrity(ti->type))
goto no_integrity;
}
list_for_each_entry(dd, devices, list) {
template_disk = dd->dm_dev->bdev->bd_disk;
if (!integrity_profile_exists(template_disk))
goto no_integrity;
else if (prev_disk &&
blk_integrity_compare(prev_disk, template_disk) < 0)
goto no_integrity;
prev_disk = template_disk;
}
return template_disk;
no_integrity:
if (prev_disk)
DMWARN("%s: integrity not set: %s and %s profile mismatch",
dm_device_name(t->md),
prev_disk->disk_name,
template_disk->disk_name);
return NULL;
}
/*
* Register the mapped device for blk_integrity support if the
* underlying devices have an integrity profile. But all devices may
* not have matching profiles (checking all devices isn't reliable
* during table load because this table may use other DM device(s) which
* must be resumed before they will have an initialized integity
* profile). Consequently, stacked DM devices force a 2 stage integrity
* profile validation: First pass during table load, final pass during
* resume.
*/
static int dm_table_register_integrity(struct dm_table *t)
{
struct mapped_device *md = t->md;
struct gendisk *template_disk = NULL;
/* If target handles integrity itself do not register it here. */
if (t->integrity_added)
return 0;
template_disk = dm_table_get_integrity_disk(t);
if (!template_disk)
return 0;
if (!integrity_profile_exists(dm_disk(md))) {
t->integrity_supported = true;
/*
* Register integrity profile during table load; we can do
* this because the final profile must match during resume.
*/
blk_integrity_register(dm_disk(md),
blk_get_integrity(template_disk));
return 0;
}
/*
* If DM device already has an initialized integrity
* profile the new profile should not conflict.
*/
if (blk_integrity_compare(dm_disk(md), template_disk) < 0) {
DMERR("%s: conflict with existing integrity profile: %s profile mismatch",
dm_device_name(t->md),
template_disk->disk_name);
return 1;
}
/* Preserve existing integrity profile */
t->integrity_supported = true;
return 0;
}
/*
* Verify that all devices have an integrity profile that matches the
* DM device's registered integrity profile. If the profiles don't
* match then unregister the DM device's integrity profile.
*/
static void dm_table_verify_integrity(struct dm_table *t)
{
struct gendisk *template_disk = NULL;
if (t->integrity_added)
return;
if (t->integrity_supported) {
/*
* Verify that the original integrity profile
* matches all the devices in this table.
*/
template_disk = dm_table_get_integrity_disk(t);
if (template_disk &&
blk_integrity_compare(dm_disk(t->md), template_disk) >= 0)
return;
}
if (integrity_profile_exists(dm_disk(t->md))) {
DMWARN("%s: unable to establish an integrity profile",
dm_device_name(t->md));
blk_integrity_unregister(dm_disk(t->md));
}
}
> But the dummy functions that got wired up with this commit are suspect:
> 54d4e6ab91eb block: centralize PI remapping logic to the block layer
>
> Effectively the entirety of the dm_integrity_profile is "we don't do
> anything special".. so yes it would be nice to not require indirect
> calls to accomplish what dm-integrity needs from block core.
All said, if blk_get_integrity() can be made to handle the removal of
the nop profile then all should "just work" right? Here is how it has
been:
static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
struct blk_integrity *bi = &disk->queue->integrity;
if (!bi->profile)
return NULL;
return bi;
}
Not looked closely at Jens' patchset (but will do), however it seems
Jens missed updating blk_get_integrity() to deal with his new
QUEUE_FLAG_INTG_PROFILE flag?
As for complete removal of integrity profiles entirely, DM needs a way
to analyze integrity configurations to try to stack up a sane end
result.
Mike
