Fixed the number of chunks issue

[hkadayam]

At present the number of chunk info slots pre created is incorrectly did not convert number of bytes to bits. Fixed that issue.

[xiaoxichen]

LGTM through I dont really understand where the max_possible_bits comes from ?

[xiaoxichen]

Is that better to hornor the requested num_bits instead of honor the serialize size?

we are now:

1. round num_chunks to 4K boundary and convert to byte, to get bit_map_size
2. bitmap_size - serialization_tax then convert to bit, to the Bitset size.

which is a bit disconnect between the original requests (num_chunks) and final bitmap available bits.

Considering doing

m_chunk_info_slots = std::make_unique< sisl::Bitset >(hs_super_blk::max_chunks_in_pdev(m_dev_info));

the bitmap_mem is align to align_size (512) not 4K but it should be fine right

[hkadayam]

Is that better to hornor the requested num_bits instead of honor the serialize size?

we are now:

1. round num_chunks to 4K boundary and convert to byte, to get bit_map_size
2. bitmap_size - serialization_tax then convert to bit, to the Bitset size.

which is a bit disconnect between the original requests (num_chunks) and final bitmap available bits.

Considering doing

m_chunk_info_slots = std::make_unique< sisl::Bitset >(hs_super_blk::max_chunks_in_pdev(m_dev_info));

the bitmap_mem is align to align_size (512) not 4K but it should be fine right

Yes I adjusted the code accordingly @xiaoxichen please take a look.

[xiaoxichen]

I see chunk_info_bitmap_size round up to 4K, and didnt properly handle the boundary when max_chunks_in_pdev % 8 != 0

however the bitset->serilize(align_size == 512) added up sizeof(bitset_serialized) and align to 512.

    static uint64_t chunk_info_bitmap_size(const dev_info& dinfo) {
        // Chunk bitmap area has bitmap of max_chunks rounded off to 4k page
        return sisl::round_up(std::max(1u, hs_super_blk::max_chunks_in_pdev(dinfo) / 8), 4096);
    }

           const uint64_t total_words{bitset_serialized::total_words(num_bits)};
           const uint64_t total_bytes{sizeof(bitset_serialized) + sizeof(word_t) * total_words};
           const uint64_t size{(alignment_size > 0) ? round_up(total_bytes, alignment_size) : total_bytes};

[xiaoxichen]

so if hs_super_blk::max_chunks_in_pdev(m_dev_info) == 4096 * 8, we get
hs_super_blk::chunk_info_bitmap_size ==4096
and
m_chunk_info_slots->serialize = round_up (4096 + sizeof(bitset_serialized), 512) = 4608

[xiaoxichen]

maybe change chunk_info_bitmap_size to

static uint64_t chunk_info_bitmap_size(const dev_info& dinfo) {
        // Chunk bitmap area has bitmap of max_chunks rounded off to 4k page
        return sisl::round_up(bitset_serialized::nbytes(hs_super_blk::max_chunks_in_pdev(dinfo)), 4096));
}

[xiaoxichen]

add alignment_size = 4096

[hkadayam]

I didn't understand the comment, why would I need to alignment_size here

[xiaoxichen]

if you specified alignment when initializing the bitset , and use same alignment configuration for serialize(), the serialize() is free, otherwise it is a copy.
c.f https://github.com/eBay/sisl/blob/03282536e309252d85319bebf4b227be0be6f9fe/include/sisl/fds/bitset.hpp#L647

[JacksonYao287]

now , when we call max_chunks_in_pdev, we calculate the max_chunks_num using the following code.

(dinfo.dev_size - 1) / min_chunk_size + 1;

however , we have superblk and vdevinfo in the starting part of a pdev, which will also take some space of a pdev. so should we take into account this when calculating?

[xiaoxichen]

m_chunk_info_slots->serialized_size(4096);

[hkadayam]

We are asking the bitset library to serialize based on pdev align size. So I don't see anything wrong here.

[xiaoxichen]

in chunk_info_bitmap_size we round up to 4K instead of pdev align size. see the example here #415 (comment) we might hit the assert.

[xiaoxichen]

maybe change chunk_info_bitmap_size to

static uint64_t chunk_info_bitmap_size(const dev_info& dinfo) {
        // Chunk bitmap area has bitmap of max_chunks rounded off to 4k page
        return sisl::round_up(bitset_serialized::nbytes(hs_super_blk::max_chunks_in_pdev(dinfo)), 4096));
}