linux-kernel - Re: [PATCH -next v3 4/7] md/raid1-10: submit write io directly if bitmap is not enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0f01b4dc-8eea-f3e9-43a4-c33084fa72a1@huaweicloud.com>
Date:   Wed, 31 May 2023 16:25:19 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     Xiao Ni <xni@...hat.com>, Yu Kuai <yukuai1@...weicloud.com>
Cc:     song@...nel.org, neilb@...e.de, akpm@...l.org,
        linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
        yi.zhang@...wei.com, yangerkun@...wei.com,
        "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH -next v3 4/7] md/raid1-10: submit write io directly if
 bitmap is not enabled

Hi,

在 2023/05/31 15:26, Xiao Ni 写道:
> On Mon, May 29, 2023 at 9:14 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@...wei.com>
>>
>> Commit 6cce3b23f6f8 ("[PATCH] md: write intent bitmap support for raid10")
>> add bitmap support, and it changed that write io is submitted through
>> daemon thread because bitmap need to be updated before write io. And
>> later, plug is used to fix performance regression because all the write io
>> will go to demon thread, which means io can't be issued concurrently.
>>
>> However, if bitmap is not enabled, the write io should not go to daemon
>> thread in the first place, and plug is not needed as well.
>>
>> Fixes: 6cce3b23f6f8 ("[PATCH] md: write intent bitmap support for raid10")
>> Signed-off-by: Yu Kuai <yukuai3@...wei.com>
>> ---
>>   drivers/md/md-bitmap.c |  4 +---
>>   drivers/md/md-bitmap.h |  7 +++++++
>>   drivers/md/raid1-10.c  | 13 +++++++++++--
>>   3 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index ad5a3456cd8a..3ee590cf12a7 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -1016,7 +1016,6 @@ static int md_bitmap_file_test_bit(struct bitmap *bitmap, sector_t block)
>>          return set;
>>   }
>>
>> -
>>   /* this gets called when the md device is ready to unplug its underlying
>>    * (slave) device queues -- before we let any writes go down, we need to
>>    * sync the dirty pages of the bitmap file to disk */
>> @@ -1026,8 +1025,7 @@ void md_bitmap_unplug(struct bitmap *bitmap)
>>          int dirty, need_write;
>>          int writing = 0;
>>
>> -       if (!bitmap || !bitmap->storage.filemap ||
>> -           test_bit(BITMAP_STALE, &bitmap->flags))
>> +       if (!md_bitmap_enabled(bitmap))
>>                  return;
>>
>>          /* look at each page to see if there are any set bits that need to be
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index cfd7395de8fd..3a4750952b3a 100644
>> --- a/drivers/md/md-bitmap.h
>> +++ b/drivers/md/md-bitmap.h
>> @@ -273,6 +273,13 @@ int md_bitmap_copy_from_slot(struct mddev *mddev, int slot,
>>                               sector_t *lo, sector_t *hi, bool clear_bits);
>>   void md_bitmap_free(struct bitmap *bitmap);
>>   void md_bitmap_wait_behind_writes(struct mddev *mddev);
>> +
>> +static inline bool md_bitmap_enabled(struct bitmap *bitmap)
>> +{
>> +       return bitmap && bitmap->storage.filemap &&
>> +              !test_bit(BITMAP_STALE, &bitmap->flags);
>> +}
>> +
>>   #endif
>>
>>   #endif
>> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
>> index 506299bd55cb..73cc3cb9154d 100644
>> --- a/drivers/md/raid1-10.c
>> +++ b/drivers/md/raid1-10.c
>> @@ -131,9 +131,18 @@ static inline bool raid1_add_bio_to_plug(struct mddev *mddev, struct bio *bio,
>>                                        blk_plug_cb_fn unplug)
>>   {
>>          struct raid1_plug_cb *plug = NULL;
>> -       struct blk_plug_cb *cb = blk_check_plugged(unplug, mddev,
>> -                                                  sizeof(*plug));
>> +       struct blk_plug_cb *cb;
>> +
>> +       /*
>> +        * If bitmap is not enabled, it's safe to submit the io directly, and
>> +        * this can get optimal performance.
>> +        */
>> +       if (!md_bitmap_enabled(mddev->bitmap)) {
>> +               raid1_submit_write(bio);
>> +               return true;
>> +       }
> 
> Can we check this out of raid1_add_bio_to_plug and call
> raid1_submit_write directly in make_request function?

Of course we can, I'm trying to avoid redundant code here...

Thanks,
Kuai
> 
> Regards
> Xiao
>>
>> +       cb = blk_check_plugged(unplug, mddev, sizeof(*plug));
>>          if (!cb)
>>                  return false;
>>
>> --
>> 2.39.2
>>
> 
> .
>