dmdedup / dmdedup3.19

Device-mapper Deduplication Target
20 stars 11 forks source link

Logical Block counter value is not incremented properly #19

Closed vinothkumarraja closed 7 years ago

vinothkumarraja commented 7 years ago

Hi, Recently we noticed that the logical block counter is not showing the correct values using "dmsetup status" command.

A sample execution output :

[root@dhcp156 scripts]# dmsetup status mydedup: 0 314572800 dedup 26214400 26214180 220 0 4096 8:32 8:48 648892 220 648672 0 648892 0

Here : physical block count is 220 but logical block count is 0

When looking into the code,, we noticed that kvs lookup for lbn->pbn mapping always succeeds with pbn value as 0. Hence as soon as we allocate a new block and increment both counters, we go ahead and decrement the LBN counter. Added couple of printk's to verify the same.

542629.349175] LBN number, PBN Old is: 19431685 , 0 [542629.349192] Allocating block ... 217 [542629.349340] Decrementing logical block counter .. 217 [542635.343086] LBN number, PBN Old is: 19431687 , 0 [542635.343124] Allocating block ... 218 [542635.343293] Decrementing logical block counter .. 218 [542635.356710] LBN number, PBN Old is: 19431688 , 0 [542635.356727] Allocating block ... 219 [542635.356886] Decrementing logical block counter .. 219

If I add a special condition to treat pbn value == 0 as No LBN - PBN mapping , the counter values are correctly updated.

FROM:

r = dc->kvs_lbn_pbn->kvs_lookup(dc->kvs_lbn_pbn, (void )&lbn, sizeof(lbn), (void )&lbnpbn_value, &vsize); if (r == 0) { / No LBN->PBN mapping entry /

TO:

r = dc->kvs_lbn_pbn->kvs_lookup(dc->kvs_lbn_pbn, (void )&lbn, sizeof(lbn), (void )&lbnpbn_value, &vsize); if (r == 0 || lbnpbn_value == 0 ) { / No LBN->PBN mapping entry /

But we couldn't figure out why key value store lookup returns true in the first place?

sectorsize512 commented 7 years ago

Hi, where is this line

mydedup: 0 314572800 dedup 26214400 26214180 220 0 4096 8:32 8:48 648892 220 648672 0 648892 0

printed in dmdedup's code? That's what I see in dm_dedup_status() function:

828 static void dm_dedup_status(struct dm_target *ti, status_type_t status_type,
829                             unsigned status_flags, char *result, unsigned maxlen)
830 {
831         struct dedup_config *dc = ti->private;
832         uint64_t data_total_block_count;
833         uint64_t data_used_block_count;
834         uint64_t data_free_block_count;
835         uint64_t data_actual_block_count;
836         int sz = 0;
837 
838         switch (status_type) {
839         case STATUSTYPE_INFO:
840                 data_used_block_count = dc->physical_block_counter;
841                 data_actual_block_count = dc->logical_block_counter;                              
842                 data_total_block_count = dc->pblocks;
843 
844                 data_free_block_count =
845                         data_total_block_count - data_used_block_count;
846 
847                 DMEMIT("%llu %llu %llu %llu ",
848                         data_total_block_count, data_free_block_count,
849                         data_used_block_count, data_actual_block_count);
850 
851                 DMEMIT("%llu %llu %llu %llu %llu %llu",
852                         dc->writes, dc->uniqwrites, dc->dupwrites,
853                         dc->reads_on_writes, dc->overwrites, dc->newwrites);
854                 break;
855         case STATUSTYPE_TABLE:
856                 DMEMIT("%s %s %u %s %s %u",
857                         dc->metadata_dev->name, dc->data_dev->name, dc->block_size,
858                         dc->crypto_alg, dc->backend_str, dc->flushrq);
859         }
860 }

And I cannot see how the code above generates the line in format you specified...

vinothkumarraja commented 7 years ago

Hi,

The format according to your code is :

mydedup: 0 314572800 dedup 26214400 26214180 220 0 648892 220 648672 0 648892 0

But this one misses the following three parameters wrt to the documentation:

block size , data_Device -minor:major , metadata_device - minor:major

So I added another DMEMIT to include these info in the status output. Only i have the code for that.

But the physical block counter and logical block counter remains the same (which is printed here)

                   data_used_block_count = dc->physical_block_counter;
                   data_actual_block_count = dc->logical_block_counter;

             DMEMIT("%llu %llu %llu %llu ",
                    data_total_block_count, data_free_block_count,
                    **data_used_block_count, data_actual_block_count**); 
vinothkumarraja commented 7 years ago

Hi, We missed zeroing the first block of the metadata device. Hence the wrong counter updates. It works fine when the the metadata device is initialized first. Hence closing the issue.

Thanks, Vinothkumar Raja