| Message ID | 20221216042353.3132139-1-houtao@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dm: remove unnecessary check when using dm_get_mdptr() | expand |
ping ? On 12/16/2022 12:23 PM, Hou Tao wrote: > From: Hou Tao <houtao1@huawei.com> > > __hash_remove() removes hash_cell with _hash_lock locked, so acquiring > _hash_lock can guarantee no-NULL hc returned from dm_get_mdptr() must > have not been removed and hc->md must still be md. > > __hash_remove() also acquires dm_hash_cells_mutex before setting mdptr > as NULL, so in dm_copy_name_and_uuid() after acquiring > dm_hash_cells_mutex and ensuring returned hc is not NULL, the returned > hc must still be alive and hc->md must still be md. > > So removing these unnecessary hc->md != md checks when using > dm_get_mdptr() with _hash_lock or dm_hash_cells_mutex acquired. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > drivers/md/dm-ioctl.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c > index 3bfc1583c20a..2a86524661d1 100644 > --- a/drivers/md/dm-ioctl.c > +++ b/drivers/md/dm-ioctl.c > @@ -772,7 +772,7 @@ static struct dm_table *dm_get_inactive_table(struct mapped_device *md, int *src > > down_read(&_hash_lock); > hc = dm_get_mdptr(md); > - if (!hc || hc->md != md) { > + if (!hc) { > DMERR("device has been removed from the dev hash table."); > goto out; > } > @@ -1476,7 +1476,7 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si > /* stage inactive table */ > down_write(&_hash_lock); > hc = dm_get_mdptr(md); > - if (!hc || hc->md != md) { > + if (!hc) { > DMERR("device has been removed from the dev hash table."); > up_write(&_hash_lock); > r = -ENXIO; > @@ -2128,7 +2128,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) > > mutex_lock(&dm_hash_cells_mutex); > hc = dm_get_mdptr(md); > - if (!hc || hc->md != md) { > + if (!hc) { > r = -ENXIO; > goto out; > } -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
ping ? Any comments on this clean up patch ? On 1/18/2023 9:16 PM, Hou Tao wrote: > ping ? > > On 12/16/2022 12:23 PM, Hou Tao wrote: >> From: Hou Tao <houtao1@huawei.com> >> >> __hash_remove() removes hash_cell with _hash_lock locked, so acquiring >> _hash_lock can guarantee no-NULL hc returned from dm_get_mdptr() must >> have not been removed and hc->md must still be md. >> >> __hash_remove() also acquires dm_hash_cells_mutex before setting mdptr >> as NULL, so in dm_copy_name_and_uuid() after acquiring >> dm_hash_cells_mutex and ensuring returned hc is not NULL, the returned >> hc must still be alive and hc->md must still be md. >> >> So removing these unnecessary hc->md != md checks when using >> dm_get_mdptr() with _hash_lock or dm_hash_cells_mutex acquired. >> >> Signed-off-by: Hou Tao <houtao1@huawei.com> >> --- >> drivers/md/dm-ioctl.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c >> index 3bfc1583c20a..2a86524661d1 100644 >> --- a/drivers/md/dm-ioctl.c >> +++ b/drivers/md/dm-ioctl.c >> @@ -772,7 +772,7 @@ static struct dm_table *dm_get_inactive_table(struct mapped_device *md, int *src >> >> down_read(&_hash_lock); >> hc = dm_get_mdptr(md); >> - if (!hc || hc->md != md) { >> + if (!hc) { >> DMERR("device has been removed from the dev hash table."); >> goto out; >> } >> @@ -1476,7 +1476,7 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si >> /* stage inactive table */ >> down_write(&_hash_lock); >> hc = dm_get_mdptr(md); >> - if (!hc || hc->md != md) { >> + if (!hc) { >> DMERR("device has been removed from the dev hash table."); >> up_write(&_hash_lock); >> r = -ENXIO; >> @@ -2128,7 +2128,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) >> >> mutex_lock(&dm_hash_cells_mutex); >> hc = dm_get_mdptr(md); >> - if (!hc || hc->md != md) { >> + if (!hc) { >> r = -ENXIO; >> goto out; >> } -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
ping ? Any comments on this clean up patch ? On 1/31/2023 9:44 AM, Hou Tao wrote: > ping ? Any comments on this clean up patch ? > > On 1/18/2023 9:16 PM, Hou Tao wrote: >> ping ? >> >> On 12/16/2022 12:23 PM, Hou Tao wrote: >>> From: Hou Tao <houtao1@huawei.com> >>> >>> __hash_remove() removes hash_cell with _hash_lock locked, so acquiring >>> _hash_lock can guarantee no-NULL hc returned from dm_get_mdptr() must >>> have not been removed and hc->md must still be md. >>> >>> __hash_remove() also acquires dm_hash_cells_mutex before setting mdptr >>> as NULL, so in dm_copy_name_and_uuid() after acquiring >>> dm_hash_cells_mutex and ensuring returned hc is not NULL, the returned >>> hc must still be alive and hc->md must still be md. >>> >>> So removing these unnecessary hc->md != md checks when using >>> dm_get_mdptr() with _hash_lock or dm_hash_cells_mutex acquired. >>> >>> Signed-off-by: Hou Tao <houtao1@huawei.com> >>> --- >>> drivers/md/dm-ioctl.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c >>> index 3bfc1583c20a..2a86524661d1 100644 >>> --- a/drivers/md/dm-ioctl.c >>> +++ b/drivers/md/dm-ioctl.c >>> @@ -772,7 +772,7 @@ static struct dm_table *dm_get_inactive_table(struct mapped_device *md, int *src >>> >>> down_read(&_hash_lock); >>> hc = dm_get_mdptr(md); >>> - if (!hc || hc->md != md) { >>> + if (!hc) { >>> DMERR("device has been removed from the dev hash table."); >>> goto out; >>> } >>> @@ -1476,7 +1476,7 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si >>> /* stage inactive table */ >>> down_write(&_hash_lock); >>> hc = dm_get_mdptr(md); >>> - if (!hc || hc->md != md) { >>> + if (!hc) { >>> DMERR("device has been removed from the dev hash table."); >>> up_write(&_hash_lock); >>> r = -ENXIO; >>> @@ -2128,7 +2128,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) >>> >>> mutex_lock(&dm_hash_cells_mutex); >>> hc = dm_get_mdptr(md); >>> - if (!hc || hc->md != md) { >>> + if (!hc) { >>> r = -ENXIO; >>> goto out; >>> } > . -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c index 3bfc1583c20a..2a86524661d1 100644 --- a/drivers/md/dm-ioctl.c +++ b/drivers/md/dm-ioctl.c @@ -772,7 +772,7 @@ static struct dm_table *dm_get_inactive_table(struct mapped_device *md, int *src down_read(&_hash_lock); hc = dm_get_mdptr(md); - if (!hc || hc->md != md) { + if (!hc) { DMERR("device has been removed from the dev hash table."); goto out; } @@ -1476,7 +1476,7 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si /* stage inactive table */ down_write(&_hash_lock); hc = dm_get_mdptr(md); - if (!hc || hc->md != md) { + if (!hc) { DMERR("device has been removed from the dev hash table."); up_write(&_hash_lock); r = -ENXIO; @@ -2128,7 +2128,7 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid) mutex_lock(&dm_hash_cells_mutex); hc = dm_get_mdptr(md); - if (!hc || hc->md != md) { + if (!hc) { r = -ENXIO; goto out; }