[dm-devel] [PATCH -next] dm snapshot: record cause of invalidating snapshot

Li Lingfeng lilingfeng3 at huawei.com
Wed Oct 11 02:36:38 UTC 2023


Ping again.

Thanks

在 2023/9/9 19:09, Li Lingfeng 写道:
> Friendly ping ...
>
> Thanks
>
> 在 2023/8/10 14:44, Li Lingfeng 写道:
>> The cause of invalidating snapshot will be print in log. However, if we
>> lost the log, we will never get the cause. What we can get by "dmsetup
>> status" is the state of "Invalid" without the cause.
>>
>> Record cause of invalidating snapshot in snapshot->valid, so that we can
>> query the cause by "dmsetup status".
>>
>> Signed-off-by: Li Lingfeng <lilingfeng3 at huawei.com>
>> ---
>>   drivers/md/dm-snap.c | 60 +++++++++++++++++++++++++++++++++-----------
>>   1 file changed, 45 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
>> index 41735a25d50a..48788431e7bf 100644
>> --- a/drivers/md/dm-snap.c
>> +++ b/drivers/md/dm-snap.c
>> @@ -39,6 +39,28 @@ static const char dm_snapshot_merge_target_name[] 
>> = "snapshot-merge";
>>   #define DM_TRACKED_CHUNK_HASH(x)    ((unsigned long)(x) & \
>>                        (DM_TRACKED_CHUNK_HASH_SIZE - 1))
>>   +#define INVALID_CAUSE_BIT        16
>> +#define DM_SNAPSHOT_VALID(s)        (s->valid & \
>> +                    ((1 << INVALID_CAUSE_BIT) - 1))
>> +#define DM_SNAPSHOT_INVALID_CAUSE(s)    (s->valid >> INVALID_CAUSE_BIT)
>> +#define SET_INVALID_CAUSE(s, cause)    (s->valid = \
>> +                    (DM_SNAPSHOT_VALID(s) | \
>> +                    (cause << INVALID_CAUSE_BIT)))
>> +
>> +enum invalid_cause {
>> +    INVALID_BY_METADATA = 1,
>> +    INVALID_BY_HANDOVER = 2,
>> +    INVALID_BY_CACELLING_HANDOVER = 3,
>> +    INVALID_BY_EIO = 4,
>> +    INVALID_BY_ENOMEM = 5
>> +};
>> +
>> +char *display_invalid_cause[] = {"by METADATA",
>> +                 "by HANDOVER",
>> +                 "by CANCELLING HANDOVER",
>> +                 "by EIO",
>> +                 "by ENOMEM"};
>> +
>>   struct dm_exception_table {
>>       uint32_t hash_mask;
>>       unsigned hash_shift;
>> @@ -1054,7 +1076,7 @@ static void snapshot_merge_next_chunks(struct 
>> dm_snapshot *s)
>>       /*
>>        * valid flag never changes during merge, so no lock required.
>>        */
>> -    if (!s->valid) {
>> +    if (!DM_SNAPSHOT_VALID(s)) {
>>           DMERR("Snapshot is invalid: can't merge");
>>           goto shut;
>>       }
>> @@ -1403,6 +1425,7 @@ static int snapshot_ctr(struct dm_target *ti, 
>> unsigned int argc, char **argv)
>>           goto bad_read_metadata;
>>       } else if (r > 0) {
>>           s->valid = 0;
>> +        SET_INVALID_CAUSE(s, INVALID_BY_METADATA);
>>           DMWARN("Snapshot is marked invalid.");
>>       }
>>   @@ -1480,6 +1503,7 @@ static void __handover_exceptions(struct 
>> dm_snapshot *snap_src,
>>        * Set source invalid to ensure it receives no further I/O.
>>        */
>>       snap_src->valid = 0;
>> +    SET_INVALID_CAUSE(snap_src, INVALID_BY_HANDOVER);
>>   }
>>     static void snapshot_dtr(struct dm_target *ti)
>> @@ -1496,6 +1520,7 @@ static void snapshot_dtr(struct dm_target *ti)
>>       if (snap_src && snap_dest && (s == snap_src)) {
>>           down_write(&snap_dest->lock);
>>           snap_dest->valid = 0;
>> +        SET_INVALID_CAUSE(snap_dest, INVALID_BY_HANDOVER);
>>           up_write(&snap_dest->lock);
>>           DMERR("Cancelling snapshot handover.");
>>       }
>> @@ -1635,18 +1660,21 @@ static void error_bios(struct bio *bio)
>>     static void __invalidate_snapshot(struct dm_snapshot *s, int err)
>>   {
>> -    if (!s->valid)
>> +    if (!DM_SNAPSHOT_VALID(s))
>>           return;
>>   -    if (err == -EIO)
>> -        DMERR("Invalidating snapshot: Error reading/writing.");
>> -    else if (err == -ENOMEM)
>> -        DMERR("Invalidating snapshot: Unable to allocate exception.");
>> -
>>       if (s->store->type->drop_snapshot)
>>           s->store->type->drop_snapshot(s->store);
>>         s->valid = 0;
>> +    if (err == -EIO) {
>> +        SET_INVALID_CAUSE(s, INVALID_BY_EIO);
>> +        DMERR("Invalidating snapshot: Error reading/writing.");
>> +    } else if (err == -ENOMEM) {
>> +        SET_INVALID_CAUSE(s, INVALID_BY_ENOMEM);
>> +        DMERR("Invalidating snapshot: Unable to allocate exception.");
>> +    }
>> +
>>         dm_table_event(s->ti->table);
>>   }
>> @@ -1692,7 +1720,7 @@ static void pending_complete(void *context, int 
>> success)
>>         down_read(&s->lock);
>>       dm_exception_table_lock(&lock);
>> -    if (!s->valid) {
>> +    if (!DM_SNAPSHOT_VALID(s)) {
>>           up_read(&s->lock);
>>           free_completed_exception(e);
>>           error = 1;
>> @@ -1984,7 +2012,7 @@ static int snapshot_map(struct dm_target *ti, 
>> struct bio *bio)
>>         /* Full snapshots are not usable */
>>       /* To get here the table must be live so s->active is always 
>> set. */
>> -    if (!s->valid)
>> +    if (!DM_SNAPSHOT_VALID(s))
>>           return DM_MAPIO_KILL;
>>         if (bio_data_dir(bio) == WRITE) {
>> @@ -1995,7 +2023,7 @@ static int snapshot_map(struct dm_target *ti, 
>> struct bio *bio)
>>       down_read(&s->lock);
>>       dm_exception_table_lock(&lock);
>>   -    if (!s->valid || (unlikely(s->snapshot_overflowed) &&
>> +    if (!DM_SNAPSHOT_VALID(s) || (unlikely(s->snapshot_overflowed) &&
>>           bio_data_dir(bio) == WRITE)) {
>>           r = DM_MAPIO_KILL;
>>           goto out_unlock;
>> @@ -2068,7 +2096,7 @@ static int snapshot_map(struct dm_target *ti, 
>> struct bio *bio)
>>                   down_write(&s->lock);
>>                     if (s->store->userspace_supports_overflow) {
>> -                    if (s->valid && !s->snapshot_overflowed) {
>> +                    if (DM_SNAPSHOT_VALID(s) && 
>> !s->snapshot_overflowed) {
>>                           s->snapshot_overflowed = 1;
>>                           DMERR("Snapshot overflowed: Unable to 
>> allocate exception.");
>>                       }
>> @@ -2159,7 +2187,7 @@ static int snapshot_merge_map(struct dm_target 
>> *ti, struct bio *bio)
>>       down_write(&s->lock);
>>         /* Full merging snapshots are redirected to the origin */
>> -    if (!s->valid)
>> +    if (!DM_SNAPSHOT_VALID(s))
>>           goto redirect_to_origin;
>>         /* If the block is already remapped - use that */
>> @@ -2344,8 +2372,10 @@ static void snapshot_status(struct dm_target 
>> *ti, status_type_t type,
>>             down_write(&snap->lock);
>>   -        if (!snap->valid)
>> -            DMEMIT("Invalid");
>> +        if (!DM_SNAPSHOT_VALID(snap))
>> +            DMEMIT("Invalid %s", DM_SNAPSHOT_INVALID_CAUSE(snap) ?
>> + display_invalid_cause[DM_SNAPSHOT_INVALID_CAUSE(snap) - 1] :
>> +                    "");
>>           else if (snap->merge_failed)
>>               DMEMIT("Merge failed");
>>           else if (snap->snapshot_overflowed)
>> @@ -2477,7 +2507,7 @@ static int __origin_write(struct list_head 
>> *snapshots, sector_t sector,
>>           dm_exception_table_lock(&lock);
>>             /* Only deal with valid and active snapshots */
>> -        if (!snap->valid || !snap->active)
>> +        if (!DM_SNAPSHOT_VALID(snap) || !snap->active)
>>               goto next_snapshot;
>>             pe = __lookup_pending_exception(snap, chunk);



More information about the dm-devel mailing list