Re: new scrub code vs zoned file systems

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 31.05.23 15:20, Christoph Hellwig wrote:
> On Wed, May 31, 2023 at 01:10:55PM +0000, Johannes Thumshirn wrote:
>> So it looks like we're calling btrfs_lookup_ordered_extent() with a NULL
>> inode.
>>
>> This actually makes sense as the current scrub code does not have an inode
>> in the bbio so:
>>
>> btrfs_simple_end_io(bio)
>> `-> btrfs_record_physical_zoned(btrfs_bio(bio))
>>     `-> btrfs_lookup_ordered_extent(bbio->inode, ...)
>>         `-> tree = &inode->ordered_tree;
>>             spin_lock_irqsave(&tree->lock, flags); <--- BOOM
>>
>> We don't really need the inode in the zoned code, but the ordered_extent.
>>
>> I've just quickly skimmed over "add an ordered_extent pointer to struct 
>> btrfs_bio v2" but didn't find anything that adds it for scrub writes as well.
> 
> That is correct, but as far as I can tell it is just the symptom.
> 
> The underlying issue is that the scrub code has no zone awareness at
> all, and just tries to rewrite sectors in place.  The old code OTOH
> tried to always migrate the entire BG (aka zone).
> 

Hmm at least flush_scrub_stripes() should not go into the simple write 
path at all:

	[...]
        /*                                                                      
         * Submit the repaired sectors.  For zoned case, we cannot do repair    
         * in-place, but queue the bg to be relocated.                          
         */                                                                     
        if (btrfs_is_zoned(fs_info)) {                                          
                for (int i = 0; i < nr_stripes; i++) {                          
                        stripe = &sctx->stripes[i];                             
                                                                                
                        if (!bitmap_empty(&stripe->error_bitmap, stripe->nr_sectors)) {
                                btrfs_repair_one_zone(fs_info,                  
                                                      sctx->stripes[0].bg->start);
                                break;                                          
                        }
                }
	} else {
                for (int i = 0; i < nr_stripes; i++) {
                        unsigned long repaired;                                 
                                                                                
                        stripe = &sctx->stripes[i];                             
                                                                                
                        bitmap_andnot(&repaired, &stripe->init_error_bitmap,    
                                      &stripe->error_bitmap, stripe->nr_sectors);
                        scrub_write_sectors(sctx, stripe, repaired, false);     
                }                                                               
        }