[PATCH 4/9] btrfs: Avoid a rescan for a device which was already not found.

[Goffredo Baroncelli]

On 25/09/2018 19.29, Daniel Kiper wrote:
> On Wed, Sep 19, 2018 at 08:40:35PM +0200, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli <address@hidden>
>>
>> If a device is not found, do not return immediately but
>> record this failure by storing NULL in data->devices_attached[].
> 
> Still the same question: Where the store happens in the code?
> I cannot find it in the patch below. This have to be clarified.
> 
> Daniel


What about the following commit description
---------------------------------------------
Change the behavior of find_device(): before the patch, a read of a missed 
device might trigger a rescan. However, it is never recorded that a device is 
missed, so each single read of a missed device might triggers a rescan.
It is the caller who decides if a rescan is performed in case of a missed 
device. And it does quite often, without considering if in the past a devices 
was already found as "missed"
This behavior causes a lot of unneeded rescan, causing a huge slowdown in case 
of a missed device.

After the patch, the "missed device" information is stored in the cache (as a 
NULL value). A rescan is triggered only if no information at all is found in 
the cache. This means that only the first time a read of a missed device 
triggers a rescan.

The change in the code is done removing "return NULL" when the disk is not 
found. So it is always executed the code which stores in the cache the value 
returned by grub_device_iterate(): NULL if the device is missed, or a valid 
data otherwise.
---------------------------------------------
> 
>> This way we avoid unnecessary devices rescan, and speedup the
>> reads in case of a degraded array.
>>
>> Signed-off-by: Goffredo Baroncelli <address@hidden>
>> ---
>>  grub-core/fs/btrfs.c | 19 +++++++++----------
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
>> index 0cdfaf7c0..6e42c33f6 100644
>> --- a/grub-core/fs/btrfs.c
>> +++ b/grub-core/fs/btrfs.c
>> @@ -588,7 +588,7 @@ find_device_iter (const char *name, void *data)
>>  }
>>
>>  static grub_device_t
>> -find_device (struct grub_btrfs_data *data, grub_uint64_t id, int do_rescan)
>> +find_device (struct grub_btrfs_data *data, grub_uint64_t id)
>>  {
>>    struct find_device_ctx ctx = {
>>      .data = data,
>> @@ -600,12 +600,9 @@ find_device (struct grub_btrfs_data *data, 
>> grub_uint64_t id, int do_rescan)
>>    for (i = 0; i < data->n_devices_attached; i++)
>>      if (id == data->devices_attached[i].id)
>>        return data->devices_attached[i].dev;
>> -  if (do_rescan)
>> -    grub_device_iterate (find_device_iter, &ctx);
>> -  if (!ctx.dev_found)
>> -    {
>> -      return NULL;
>> -    }
>> +
>> +  grub_device_iterate (find_device_iter, &ctx);
>> +
>>    data->n_devices_attached++;
>>    if (data->n_devices_attached > data->n_devices_allocated)
>>      {
>> @@ -617,7 +614,8 @@ find_device (struct grub_btrfs_data *data, grub_uint64_t 
>> id, int do_rescan)
>>                      * sizeof (data->devices_attached[0]));
>>        if (!data->devices_attached)
>>      {
>> -      grub_device_close (ctx.dev_found);
>> +      if (ctx.dev_found)
>> +        grub_device_close (ctx.dev_found);
>>        data->devices_attached = tmp;
>>        return NULL;
>>      }
>> @@ -892,7 +890,7 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
>> grub_disk_addr_t addr,
>>                            " for laddr 0x%" PRIxGRUB_UINT64_T "\n", paddr,
>>                            addr);
>>
>> -            dev = find_device (data, stripe->device_id, j);
>> +            dev = find_device (data, stripe->device_id);
>>              if (!dev)
>>                {
>>                  grub_dprintf ("btrfs",
>> @@ -969,7 +967,8 @@ grub_btrfs_unmount (struct grub_btrfs_data *data)
>>    unsigned i;
>>    /* The device 0 is closed one layer upper.  */
>>    for (i = 1; i < data->n_devices_attached; i++)
>> -    grub_device_close (data->devices_attached[i].dev);
>> +    if (data->devices_attached[i].dev)
>> +        grub_device_close (data->devices_attached[i].dev);
>>    grub_free (data->devices_attached);
>>    grub_free (data->extent);
>>    grub_free (data);
>> --
>> 2.19.0
>>
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5