mbox series

[v3,0/2] blk-mq: Avoid memory reclaim when allocating

Message ID 20190917120910.24842-1-xiubli@redhat.com (mailing list archive)
Headers show
Series blk-mq: Avoid memory reclaim when allocating | expand

Message

Xiubo Li Sept. 17, 2019, 12:09 p.m. UTC
From: Xiubo Li <xiubli@redhat.com>

Changed in V2:
- Addressed the comment from Ming Lei, thanks.

Changed in V3:
- Switch to memalloc_noio_save/restore from Christoph's comment, thanks.

Xiubo Li (2):
  blk-mq: Avoid memory reclaim when allocating request map
  blk-mq: use BLK_MQ_GFP_FLAGS and memalloc_noio_save/restore instead

 block/blk-mq-tag.c |  5 +++--
 block/blk-mq-tag.h |  5 ++++-
 block/blk-mq.c     | 45 +++++++++++++++++++++++++++++++++------------
 3 files changed, 40 insertions(+), 15 deletions(-)

Comments

Jens Axboe Sept. 17, 2019, 2:13 p.m. UTC | #1
On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> Changed in V2:
> - Addressed the comment from Ming Lei, thanks.
> 
> Changed in V3:
> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.

This now seems to be a mix of both approaches, which I don't think makes
sense at all. I think we should just stick to the gfp_t being passed in,
and defining the standard mask for init time blk-mq memory allocations.
Xiubo Li Sept. 17, 2019, 10:54 p.m. UTC | #2
On 2019/9/17 22:13, Jens Axboe wrote:
> On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Changed in V2:
>> - Addressed the comment from Ming Lei, thanks.
>>
>> Changed in V3:
>> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.
> This now seems to be a mix of both approaches, which I don't think makes
> sense at all. I think we should just stick to the gfp_t being passed in,
> and defining the standard mask for init time blk-mq memory allocations.
>
Hmm, I might missed or misunderstand from the last thread. In this 
thread with the save/store, the GFP_KERNEL is using instead. Maybe 
save/store pair is not a exactly correct place or occasion to use here 
as @Bart mentioned.

Thanks.

BRs
Jens Axboe Sept. 17, 2019, 11:11 p.m. UTC | #3
On 9/17/19 4:54 PM, Xiubo Li wrote:
> On 2019/9/17 22:13, Jens Axboe wrote:
>> On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
>>> From: Xiubo Li <xiubli@redhat.com>
>>>
>>> Changed in V2:
>>> - Addressed the comment from Ming Lei, thanks.
>>>
>>> Changed in V3:
>>> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.
>> This now seems to be a mix of both approaches, which I don't think makes
>> sense at all. I think we should just stick to the gfp_t being passed in,
>> and defining the standard mask for init time blk-mq memory allocations.
>>
> Hmm, I might missed or misunderstand from the last thread. In this
> thread with the save/store, the GFP_KERNEL is using instead. Maybe
> save/store pair is not a exactly correct place or occasion to use here
> as @Bart mentioned.

Just make them all gfp based please, and skip the memalloc() stuff.
Xiubo Li Sept. 17, 2019, 11:19 p.m. UTC | #4
On 2019/9/18 7:11, Jens Axboe wrote:
> On 9/17/19 4:54 PM, Xiubo Li wrote:
>> On 2019/9/17 22:13, Jens Axboe wrote:
>>> On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>
>>>> Changed in V2:
>>>> - Addressed the comment from Ming Lei, thanks.
>>>>
>>>> Changed in V3:
>>>> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.
>>> This now seems to be a mix of both approaches, which I don't think makes
>>> sense at all. I think we should just stick to the gfp_t being passed in,
>>> and defining the standard mask for init time blk-mq memory allocations.
>>>
>> Hmm, I might missed or misunderstand from the last thread. In this
>> thread with the save/store, the GFP_KERNEL is using instead. Maybe
>> save/store pair is not a exactly correct place or occasion to use here
>> as @Bart mentioned.
> Just make them all gfp based please, and skip the memalloc() stuff.

Yeah, isn't the v2 thread needed here ?

Thanks

BRs
Jens Axboe Sept. 17, 2019, 11:25 p.m. UTC | #5
On 9/17/19 5:19 PM, Xiubo Li wrote:
> On 2019/9/18 7:11, Jens Axboe wrote:
>> On 9/17/19 4:54 PM, Xiubo Li wrote:
>>> On 2019/9/17 22:13, Jens Axboe wrote:
>>>> On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>
>>>>> Changed in V2:
>>>>> - Addressed the comment from Ming Lei, thanks.
>>>>>
>>>>> Changed in V3:
>>>>> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.
>>>> This now seems to be a mix of both approaches, which I don't think makes
>>>> sense at all. I think we should just stick to the gfp_t being passed in,
>>>> and defining the standard mask for init time blk-mq memory allocations.
>>>>
>>> Hmm, I might missed or misunderstand from the last thread. In this
>>> thread with the save/store, the GFP_KERNEL is using instead. Maybe
>>> save/store pair is not a exactly correct place or occasion to use here
>>> as @Bart mentioned.
>> Just make them all gfp based please, and skip the memalloc() stuff.
> 
> Yeah, isn't the v2 thread needed here ?

It might be, I didn't look super closely at v2. I'll take a look.
Xiubo Li Sept. 17, 2019, 11:30 p.m. UTC | #6
On 2019/9/18 7:25, Jens Axboe wrote:
> On 9/17/19 5:19 PM, Xiubo Li wrote:
>> On 2019/9/18 7:11, Jens Axboe wrote:
>>> On 9/17/19 4:54 PM, Xiubo Li wrote:
>>>> On 2019/9/17 22:13, Jens Axboe wrote:
>>>>> On 9/17/19 6:09 AM, xiubli@redhat.com wrote:
>>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>>
>>>>>> Changed in V2:
>>>>>> - Addressed the comment from Ming Lei, thanks.
>>>>>>
>>>>>> Changed in V3:
>>>>>> - Switch to memalloc_noio_save/restore from Christoph's comment, thanks.
>>>>> This now seems to be a mix of both approaches, which I don't think makes
>>>>> sense at all. I think we should just stick to the gfp_t being passed in,
>>>>> and defining the standard mask for init time blk-mq memory allocations.
>>>>>
>>>> Hmm, I might missed or misunderstand from the last thread. In this
>>>> thread with the save/store, the GFP_KERNEL is using instead. Maybe
>>>> save/store pair is not a exactly correct place or occasion to use here
>>>> as @Bart mentioned.
>>> Just make them all gfp based please, and skip the memalloc() stuff.
>> Yeah, isn't the v2 thread needed here ?
> It might be, I didn't look super closely at v2. I'll take a look.

Sure :-)

BRs