| Message ID | 20181229013147.211079-1-shakeelb@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | percpu: plumb gfp flag to pcpu_get_pages | expand |
On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt <shakeelb@google.com> wrote: > __alloc_percpu_gfp() can be called from atomic context, so, make > pcpu_get_pages use the gfp provided to the higher layer. Does this fix any user-visible issues?
Hi Shakeel, On Fri, Dec 28, 2018 at 05:31:47PM -0800, Shakeel Butt wrote: > __alloc_percpu_gfp() can be called from atomic context, so, make > pcpu_get_pages use the gfp provided to the higher layer. > > Signed-off-by: Shakeel Butt <shakeelb@google.com> > --- > mm/percpu-vm.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c > index d8078de912de..4f42c4c5c902 100644 > --- a/mm/percpu-vm.c > +++ b/mm/percpu-vm.c > @@ -21,6 +21,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk *chunk, > > /** > * pcpu_get_pages - get temp pages array > + * @gfp: allocation flags passed to the underlying allocator > * > * Returns pointer to array of pointers to struct page which can be indexed > * with pcpu_page_idx(). Note that there is only one array and accesses > @@ -29,7 +30,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk *chunk, > * RETURNS: > * Pointer to temp pages array on success. > */ > -static struct page **pcpu_get_pages(void) > +static struct page **pcpu_get_pages(gfp_t gfp) > { > static struct page **pages; > size_t pages_size = pcpu_nr_units * pcpu_unit_pages * sizeof(pages[0]); > @@ -37,7 +38,7 @@ static struct page **pcpu_get_pages(void) > lockdep_assert_held(&pcpu_alloc_mutex); > > if (!pages) > - pages = pcpu_mem_zalloc(pages_size, GFP_KERNEL); > + pages = pcpu_mem_zalloc(pages_size, gfp); > return pages; > } > > @@ -278,7 +279,7 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, > { > struct page **pages; > > - pages = pcpu_get_pages(); > + pages = pcpu_get_pages(gfp); > if (!pages) > return -ENOMEM; > > @@ -316,7 +317,7 @@ static void pcpu_depopulate_chunk(struct pcpu_chunk *chunk, > * successful population attempt so the temp pages array must > * be available now. > */ > - pages = pcpu_get_pages(); > + pages = pcpu_get_pages(GFP_KERNEL); > BUG_ON(!pages); > > /* unmap and free */ > -- > 2.20.1.415.g653613c723-goog > Sorry, I'm travelling today and was hoping to respond to this later tonight. So percpu memory is a little different as it's an intermediary. When you call __alloc_percpu_gfp() and it does not contain GFP_KERNEL, it is considered atomic. So, we only service requests out of already populated memory. pcpu_get_pages() is only called when we need to populate/depopulate a chunk and will not be called if we need an atomic allocation. Also, in all but the first case, it won't make an allocation as pages is a static variable. Furthermore, percpu only plumbs through certain gfp as not all make sense [1]. [1] https://lore.kernel.org/lkml/cover.1518668149.git.dennisszhou@gmail.com/ Thanks, Dennis
Hi Andrew, On Sat, Dec 29, 2018 at 01:03:52PM -0800, Andrew Morton wrote: > On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt <shakeelb@google.com> wrote: > > > __alloc_percpu_gfp() can be called from atomic context, so, make > > pcpu_get_pages use the gfp provided to the higher layer. > > Does this fix any user-visible issues? Sorry for not getting to this earlier. I'm currently traveling. I respoeded on the patch itself. Do you mind unqueuing? I explain in more detail on the patch, but __alloc_percpu_gfp() will never call pcpu_get_pages() when called as not GFP_KERNEL. Thanks, Dennis
Hi Dennis, On Sat, Dec 29, 2018 at 1:26 PM Dennis Zhou <dennis@kernel.org> wrote: > > Hi Andrew, > > On Sat, Dec 29, 2018 at 01:03:52PM -0800, Andrew Morton wrote: > > On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt <shakeelb@google.com> wrote: > > > > > __alloc_percpu_gfp() can be called from atomic context, so, make > > > pcpu_get_pages use the gfp provided to the higher layer. > > > > Does this fix any user-visible issues? > > Sorry for not getting to this earlier. I'm currently traveling. I > respoeded on the patch itself. Do you mind unqueuing? I explain in more > detail on the patch, but __alloc_percpu_gfp() will never call > pcpu_get_pages() when called as not GFP_KERNEL. > Thanks for the explanation. Andrew, please ignore/drop this patch. thanks, Shakeel
diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c index d8078de912de..4f42c4c5c902 100644 --- a/mm/percpu-vm.c +++ b/mm/percpu-vm.c @@ -21,6 +21,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk *chunk, /** * pcpu_get_pages - get temp pages array + * @gfp: allocation flags passed to the underlying allocator * * Returns pointer to array of pointers to struct page which can be indexed * with pcpu_page_idx(). Note that there is only one array and accesses @@ -29,7 +30,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk *chunk, * RETURNS: * Pointer to temp pages array on success. */ -static struct page **pcpu_get_pages(void) +static struct page **pcpu_get_pages(gfp_t gfp) { static struct page **pages; size_t pages_size = pcpu_nr_units * pcpu_unit_pages * sizeof(pages[0]); @@ -37,7 +38,7 @@ static struct page **pcpu_get_pages(void) lockdep_assert_held(&pcpu_alloc_mutex); if (!pages) - pages = pcpu_mem_zalloc(pages_size, GFP_KERNEL); + pages = pcpu_mem_zalloc(pages_size, gfp); return pages; } @@ -278,7 +279,7 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, { struct page **pages; - pages = pcpu_get_pages(); + pages = pcpu_get_pages(gfp); if (!pages) return -ENOMEM; @@ -316,7 +317,7 @@ static void pcpu_depopulate_chunk(struct pcpu_chunk *chunk, * successful population attempt so the temp pages array must * be available now. */ - pages = pcpu_get_pages(); + pages = pcpu_get_pages(GFP_KERNEL); BUG_ON(!pages); /* unmap and free */
__alloc_percpu_gfp() can be called from atomic context, so, make pcpu_get_pages use the gfp provided to the higher layer. Signed-off-by: Shakeel Butt <shakeelb@google.com> --- mm/percpu-vm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)