Hi,
I would like to know whether the attached patch is something
acceptable or not.
In dm-raid1.c, 'clean_regions' in region hash is a list used
to link regions which are clean AND no longer interested (freeable).
So I think it's safe not to add newly-allocated RH_CLEAN entry
to the list because someone who allocates it has interest
on the region.
In the current code, rh_alloc()/rh_find() is called from:
- rh_inc(), which makes the state RH_DIRTY if it's RH_CLEAN
- __rh_recovery_prepare(), which makes the state RH_RECOVERING
- rh_delay(), which is called when the region is RH_RECOVERING
So none uses the newly allocated RH_CLEAN region as-is.
RH_DIRTY region is changed back to RH_CLEAN and then connected
to clean_regions when pending I/O completes.
RH_RECOVERING region is reclaimed when the recovery on the
region completes.
If this patch is acceptable, we can also merge RH_CLEAN and
RH_DIRTY into one state.
Comments are welcome.
Thanks,
Jun'ichi "Nick" Nomura
[RFC] dm-raid1: not to add newly-allocated RH_CLEAN region to clean_regions
The clean_regions list is used to maintain reclaimable hash entries.
If the region is known to be marked RH_DIRTY or RH_RECOVERING later,
it's not necessarily connected to clean_regions even if it's clean.
OTOH, region hash entries are allocated only when the region is going
to be written (RH_DIRTY) or recoverred (RH_RECOVERING).
So it's safe not to add RH_CLEAN region to clean_regions in __rh_alloc().
Not adding to clean_regions protects the region from potential race
condition that the hash entry is reclaimed by rh_update_states()
before use.
Signed-off-by: Jun'ichi Nomura <j-nomura@xxxxxxxxxxxxx>
diff -urp linux.orig/drivers/md/dm-raid1.c linux/drivers/md/dm-raid1.c
--- linux.orig/drivers/md/dm-raid1.c 2005-12-26 05:25:04.000000000 -0500
+++ linux/drivers/md/dm-raid1.c 2006-01-06 11:47:03.000000000 -0500
@@ -255,11 +254,6 @@ static struct region *__rh_alloc(struct
else {
__rh_insert(rh, nreg);
- if (nreg->state == RH_CLEAN) {
- spin_lock(&rh->region_lock);
- list_add(&nreg->list, &rh->clean_regions);
- spin_unlock(&rh->region_lock);
- }
reg = nreg;
}
write_unlock_irq(&rh->hash_lock);
--
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
