| Message ID | 20190916021631.4327-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | blk-mq: Avoid memory reclaim when allocating request map | expand |
On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > To make the patch more readable and cleaner I just split them into 2 > small ones to address the issue from @Ming Lei, thanks very much. I'd be much happier to just see memalloc_noio_save + memalloc_noio_restore calls in the right places over sprinkling even more magic GFP_NOIO arguments.
On 2019/9/16 17:06, Christoph Hellwig wrote: > On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> To make the patch more readable and cleaner I just split them into 2 >> small ones to address the issue from @Ming Lei, thanks very much. > I'd be much happier to just see memalloc_noio_save + > memalloc_noio_restore calls in the right places over sprinkling even > more magic GFP_NOIO arguments. Hi Christoph, BTW, then to make the code to be more readable, should I just keep the BLK_MQ_GFP_GLAGS as: #define BLK_MQ_GFP_FLAGS (GFP_NOIO | __GFP_NOWARN | __GFP_NORETRY) Or just switch to: #define BLK_MQ_GFP_FLAGS (GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY) ? Any advice about this ? Thanks very much, BRs Xiubo
On 9/16/19 3:06 AM, Christoph Hellwig wrote: > On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> To make the patch more readable and cleaner I just split them into 2 >> small ones to address the issue from @Ming Lei, thanks very much. > > I'd be much happier to just see memalloc_noio_save + > memalloc_noio_restore calls in the right places over sprinkling even > more magic GFP_NOIO arguments. Ugh, I always thought those were kind of lame and band aiding around places where people are too lazy to fix the path to the gfp args. Or maybe areas where it's just feasible. This is not one of those.
On 9/16/19 2:06 AM, Christoph Hellwig wrote: > On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> To make the patch more readable and cleaner I just split them into 2 >> small ones to address the issue from @Ming Lei, thanks very much. > > I'd be much happier to just see memalloc_noio_save + > memalloc_noio_restore calls in the right places over sprinkling even > more magic GFP_NOIO arguments. Hi Christoph, My understanding is that memalloc_noio_save() and memalloc_noio_restore() have been introduced to avoid having to pass flags that specify the noio context along deep call chains. Are you sure it helps to use these functions if no other function than kmalloc() is called between the memalloc_noio_save() and memalloc_noio_restore() calls? Thanks, Bart.
On Mon, Sep 16, 2019 at 10:52:33AM -0600, Jens Axboe wrote: > On 9/16/19 3:06 AM, Christoph Hellwig wrote: > > On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: > >> From: Xiubo Li <xiubli@redhat.com> > >> > >> To make the patch more readable and cleaner I just split them into 2 > >> small ones to address the issue from @Ming Lei, thanks very much. > > > > I'd be much happier to just see memalloc_noio_save + > > memalloc_noio_restore calls in the right places over sprinkling even > > more magic GFP_NOIO arguments. > > Ugh, I always thought those were kind of lame and band aiding around > places where people are too lazy to fix the path to the gfp args. > Or maybe areas where it's just feasible. The way I understood the discussion around the introduction of these flags is that we want to phase out GFP_NOIO and GFP_NOFS in the long run, given that ammending all calls is basically impossibly, while marking contexts is pretty easy.
On 2019/9/19 0:38, Christoph Hellwig wrote: > On Mon, Sep 16, 2019 at 10:52:33AM -0600, Jens Axboe wrote: >> On 9/16/19 3:06 AM, Christoph Hellwig wrote: >>> On Mon, Sep 16, 2019 at 07:46:29AM +0530, xiubli@redhat.com wrote: >>>> From: Xiubo Li <xiubli@redhat.com> >>>> >>>> To make the patch more readable and cleaner I just split them into 2 >>>> small ones to address the issue from @Ming Lei, thanks very much. >>> I'd be much happier to just see memalloc_noio_save + >>> memalloc_noio_restore calls in the right places over sprinkling even >>> more magic GFP_NOIO arguments. >> Ugh, I always thought those were kind of lame and band aiding around >> places where people are too lazy to fix the path to the gfp args. >> Or maybe areas where it's just feasible. > The way I understood the discussion around the introduction of these > flags is that we want to phase out GFP_NOIO and GFP_NOFS in the long > run, given that ammending all calls is basically impossibly, while > marking contexts is pretty easy. This was also my understanding about the memalloc_xx stuff. By going through the code and the usage history of it, the best practice of using it seems as Bart mentioned for the possibly deep call chains. Thanks
From: Xiubo Li <xiubli@redhat.com> To make the patch more readable and cleaner I just split them into 2 small ones to address the issue from @Ming Lei, thanks very much. Changed in V2: - Addressed the comment from Ming Lei, thanks. Xiubo Li (2): blk-mq: Avoid memory reclaim when allocating request map blk-mq: use BLK_MQ_GFP_FLAGS macro instead block/blk-mq-tag.c | 5 +++-- block/blk-mq-tag.h | 5 ++++- block/blk-mq.c | 17 +++++++++-------- 3 files changed, 16 insertions(+), 11 deletions(-)