Messages in this thread | | | From | Johannes Thumshirn <> | Subject | Re: [PATCH v9 01/11] btrfs: add raid stripe tree definitions | Date | Mon, 2 Oct 2023 09:32:47 +0000 |
| |
On 15.09.23 12:34, Qu Wenruo wrote: > > > > On 2023/9/15 19:25, Johannes Thumshirn wrote: >> On 15.09.23 02:27, Qu Wenruo wrote: >>>>> /* >>>>> * Records the overall state of the qgroups. >>>>> * There's only one instance of this key present, >>>>> @@ -719,6 +724,32 @@ struct btrfs_free_space_header { >>>>> __le64 num_bitmaps; >>>>> } __attribute__ ((__packed__)); >>>>> +struct btrfs_raid_stride { >>>>> + /* The btrfs device-id this raid extent lives on */ >>>>> + __le64 devid; >>>>> + /* The physical location on disk */ >>>>> + __le64 physical; >>>>> + /* The length of stride on this disk */ >>>>> + __le64 length; >>> >>> Forgot to mention, for btrfs_stripe_extent structure, its key is >>> (PHYSICAL, RAID_STRIPE_KEY, LENGTH) right? >>> >>> So is the length in the btrfs_raid_stride duplicated and we can save 8 >>> bytes? >> >> Nope. The length in the key is the stripe length. The length in the >> stride is the stride length. >> >> Here's an example for why this is needed: >> >> wrote 32768/32768 bytes at offset 0 >> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> wrote 131072/131072 bytes at offset 0 >> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> wrote 8192/8192 bytes at offset 65536 >> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> >> [snip] >> >> item 0 key (XXXXXX RAID_STRIPE_KEY 32768) itemoff XXXXX itemsize 32 >> encoding: RAID0 >> stripe 0 devid 1 physical XXXXXXXXX length 32768 >> item 1 key (XXXXXX RAID_STRIPE_KEY 131072) itemoff XXXXX >> itemsize 80 > > Maybe you want to put the whole RAID_STRIPE_KEY definition into the headers. > > In fact my initial assumption of such case would be something like this: > > item 0 key (X+0 RAID_STRIPE 32K) > stripe 0 devid 1 physical XXXXX len 32K > item 1 key (X+32K RAID_STRIPE 32K) > stripe 0 devid 1 physical XXXXX + 32K len 32K > item 2 key (X+64K RAID_STRIPE 64K) > stripe 0 devid 2 physical YYYYY len 64K > item 3 key (X+128K RAID_STRIPE 32K) > stripe 0 devid 1 physical XXXXX + 64K len 32K > ... > > AKA, each RAID_STRIPE_KEY would only contain a continous physical stripe. > And in above case, item 0 and item 1 can be easily merged, also length > can be removed. > > And this explains why the lookup code is more complex than I initially > thought. > > BTW, would the above layout make the code a little easier? > Or is there any special reason for the existing one layout? > > Thank, > Qu > > >> encoding: RAID0 >> stripe 0 devid 1 physical XXXXXXXXX length 32768 >> stripe 1 devid 2 physical XXXXXXXXX length 65536 >> stripe 2 devid 1 physical XXXXXXXXX length 32768 > > This current layout has another problem. > For RAID10 the interpretation of the RAID_STRIPE item can be very complex. > While > >> item 2 key (XXXXXX RAID_STRIPE_KEY 8192) itemoff XXXXX itemsize 32 >> encoding: RAID0 >> stripe 0 devid 1 physical XXXXXXXXX length 8192 >> >> Without the length in the stride, we don't know when to select the next >> stride in item 1 above. >
JFYI preliminary tests for your suggestion look reasonably good. I'll give it some more testing and code cleanup but it actually seems sensible to do.
| |