Re: [PATCH v5 04/16] md/raid5: add a member of r5pages for struct stripe_head

[Jason Yan <yanaijie@huawei.com>]

在 2020/7/3 6:56, Song Liu 写道:
> On Thu, Jul 2, 2020 at 5:05 AM Yufen Yu <yuyufen@huawei.com> wrote:
>>
>> Since grow_buffers() uses alloc_page() to allocate the buffers for
>> each stripe_head(), means, it will allocate 64K buffers and just use
>> 4K of them, after setting stripe_size as 4096.
>>
>> To avoid wasting memory, we try to contain multiple 'page' of sh->dev
>> into one real page. That means, multiple sh->dev[i].page will point to
>> the only page with different offset. Example of 64K PAGE_SIZE and
>> 4K stripe_size as following:
>>
>>                      64K PAGE_SIZE
>>            +---+---+---+---+------------------------------+
>>            |   |   |   |   |
>>            |   |   |   |   |
>>            +-+-+-+-+-+-+-+-+------------------------------+
>>              ^   ^   ^   ^
>>              |   |   |   +----------------------------+
>>              |   |   |                                |
>>              |   |   +-------------------+            |
>>              |   |                       |            |
>>              |   +----------+            |            |
>>              |              |            |            |
>>              +-+            |            |            |
>>                |            |            |            |
>>          +-----+-----+------+-----+------+-----+------+------+
>> sh      | offset(0) | offset(4K) | offset(8K) | offset(12K) |
>>   +      +-----------+------------+------------+-------------+
>>   +----> dev[0].page  dev[1].page  dev[2].page  dev[3].page
>>
>> After trying to share one page, the users of sh->dev[i].page need to
>> take care:
>>
>>    1) When issue bio into stripe_head, bi_io_vec.bv_page will point to
>>       the page directly. So, we should make sure bv_offset to been set
>>       with correct offset.
>>
>>    2) When compute xor, the page will be passed to computer function.
>>       So, we also need to pass offset of that page to computer. Let it
>>       compute correct location of each sh->dev[i].page.
>>
>> This patch will add a new member of r5pages into stripe_head to manage
>> all pages needed by each sh->dev[i]. We also add 'offset' for each r5dev
>> so that users can get related page offset easily. And add helper function
>> to get page and it's index in r5pages array by disk index.
>>
>> Signed-off-by: Yufen Yu <yuyufen@huawei.com>
>> ---
>>   drivers/md/raid5.h | 61 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 61 insertions(+)
>>

[...]

>>
>> +/*
>> + * Return corresponding page index of r5pages array.
>> + */
>> +static inline int raid5_get_page_index(struct stripe_head *sh, int disk_idx)
>> +{
>> +       struct r5conf *conf = sh->raid_conf;
>> +       int cnt;
>> +
>> +       WARN_ON(!sh->pages.page);
>> +       BUG_ON(conf->stripe_size > PAGE_SIZE);
> 
> We have too many of these WARN_ON() and BUG_ON().
> 

Yes. Yufen, please avoid using BUG_ON() becuase we are reducing the 
usage of it and Linus hate it:

https://lkml.org/lkml/2013/5/17/254
https://linuxinsider.com/story/torvalds-blows-stack-over-buggy-new-kernel-83975.html
https://lore.kernel.org/patchwork/patch/568291/


Thanks,
Jason