This function is not referenced anywhere else in the kernel, remove it.
Signed-off-by: Lorenzo Stoakes <[email protected]>
---
include/linux/gfp.h | 7 -------
1 file changed, 7 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 65a78773dcca..67c5164f4758 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -188,13 +188,6 @@ unsigned long alloc_pages_bulk_array_mempolicy(gfp_t gfp,
unsigned long nr_pages,
struct page **page_array);
-/* Bulk allocate order-0 pages */
-static inline unsigned long
-alloc_pages_bulk_list(gfp_t gfp, unsigned long nr_pages, struct list_head *list)
-{
- return __alloc_pages_bulk(gfp, numa_mem_id(), NULL, nr_pages, list, NULL);
-}
-
static inline unsigned long
alloc_pages_bulk_array(gfp_t gfp, unsigned long nr_pages, struct page **page_array)
{
--
2.38.1
On Sat, Dec 17, 2022 at 12:15:54AM +0000, Lorenzo Stoakes wrote:
> This function is not referenced anywhere else in the kernel, remove it.
I think we're waiting to see if any users show up. It's only been 18
months or so, so perhaps wait a little longer?
Also, if we do get rid of this, then __alloc_pages_bulk() can lose
the "list" argument.
> Signed-off-by: Lorenzo Stoakes <[email protected]>
> ---
> include/linux/gfp.h | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 65a78773dcca..67c5164f4758 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -188,13 +188,6 @@ unsigned long alloc_pages_bulk_array_mempolicy(gfp_t gfp,
> unsigned long nr_pages,
> struct page **page_array);
>
> -/* Bulk allocate order-0 pages */
> -static inline unsigned long
> -alloc_pages_bulk_list(gfp_t gfp, unsigned long nr_pages, struct list_head *list)
> -{
> - return __alloc_pages_bulk(gfp, numa_mem_id(), NULL, nr_pages, list, NULL);
> -}
> -
> static inline unsigned long
> alloc_pages_bulk_array(gfp_t gfp, unsigned long nr_pages, struct page **page_array)
> {
> --
> 2.38.1
>
+ Mel
On Sat, Dec 17, 2022 at 04:56:03AM +0000, Matthew Wilcox wrote:
> I think we're waiting to see if any users show up. It's only been 18
> months or so, so perhaps wait a little longer?
Apologies, I should have researched the background of this further. I see it was
added speculatively in 0f87d9d30f.
> Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> the "list" argument.
Ack. Will do a v2 if Mel feels it's appropriate to remove at this stage,
otherwise we can drop the idea and wait for users to emerge.
On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> + Mel
>
> On Sat, Dec 17, 2022 at 04:56:03AM +0000, Matthew Wilcox wrote:
> > I think we're waiting to see if any users show up. It's only been 18
> > months or so, so perhaps wait a little longer?
>
> Apologies, I should have researched the background of this further. I see it was
> added speculatively in 0f87d9d30f.
>
> > Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> > the "list" argument.
>
> Ack. Will do a v2 if Mel feels it's appropriate to remove at this stage,
> otherwise we can drop the idea and wait for users to emerge.
I reckon we should give it another few months until May. There has been
one user recently that tried to use list but it turned out arrays were
more appropriate.
--
Mel Gorman
SUSE Labs
On Mon, Dec 19, 2022 at 2:07 AM Mel Gorman <[email protected]> wrote:
>
> On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > + Mel
> >
> > On Sat, Dec 17, 2022 at 04:56:03AM +0000, Matthew Wilcox wrote:
> > > I think we're waiting to see if any users show up. It's only been 18
> > > months or so, so perhaps wait a little longer?
> >
> > Apologies, I should have researched the background of this further. I see it was
> > added speculatively in 0f87d9d30f.
> >
> > > Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> > > the "list" argument.
> >
> > Ack. Will do a v2 if Mel feels it's appropriate to remove at this stage,
> > otherwise we can drop the idea and wait for users to emerge.
>
> I reckon we should give it another few months until May. There has been
> one user recently that tried to use list but it turned out arrays were
> more appropriate.
Another recent try was the patches for dm-crypt. However Mel suggested
adding a callback interface instead of using list since list is not
necessary for that usecase. Or are we talking about the same one?
I didn't get time to implement Mel's suggestion yet due to my
employment change. Hopefully I can find some time after the new year.
>
> --
> Mel Gorman
> SUSE Labs
>
On Sat, Dec 17, 2022 at 04:56:03AM +0000, Matthew Wilcox wrote:
> On Sat, Dec 17, 2022 at 12:15:54AM +0000, Lorenzo Stoakes wrote:
> > This function is not referenced anywhere else in the kernel, remove it.
>
> I think we're waiting to see if any users show up. It's only been 18
> months or so, so perhaps wait a little longer?
Is that being snarky? :) We generally don't add dead code at all, and
certaintly don't leave it around for a year and a half.
If anyone really needs this we can add it back.
> Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> the "list" argument.
Yepp.
On Tue, Dec 20, 2022 at 09:56:19AM -0800, Yang Shi wrote:
> On Mon, Dec 19, 2022 at 2:07 AM Mel Gorman <[email protected]> wrote:
> >
> > On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > > + Mel
> > >
> > > On Sat, Dec 17, 2022 at 04:56:03AM +0000, Matthew Wilcox wrote:
> > > > I think we're waiting to see if any users show up. It's only been 18
> > > > months or so, so perhaps wait a little longer?
> > >
> > > Apologies, I should have researched the background of this further. I see it was
> > > added speculatively in 0f87d9d30f.
> > >
> > > > Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> > > > the "list" argument.
> > >
> > > Ack. Will do a v2 if Mel feels it's appropriate to remove at this stage,
> > > otherwise we can drop the idea and wait for users to emerge.
> >
> > I reckon we should give it another few months until May. There has been
> > one user recently that tried to use list but it turned out arrays were
> > more appropriate.
>
> Another recent try was the patches for dm-crypt. However Mel suggested
> adding a callback interface instead of using list since list is not
> necessary for that usecase. Or are we talking about the same one?
>
That was the one I was thinking of but forgot the specifics so the array
comment was wrong. I only remembered that there was an attempted list user
but it wasn't the best solution.
> I didn't get time to implement Mel's suggestion yet due to my
> employment change. Hopefully I can find some time after the new year.
>
No rush.
--
Mel Gorman
SUSE Labs
On Thu, Dec 22, 2022 at 02:03:46AM -0800, Christoph Hellwig wrote:
> Is that being snarky? :) We generally don't add dead code at all, and
> certaintly don't leave it around for a year and a half.
>
> If anyone really needs this we can add it back.
>
Mel has replied elsewhere on this thread and we have a potential user so I think
the lowest-impact approach is to go with what he suggested - wait until May and
see if we have any takers.
I have put a reminder in my calendar for then, I'll check to see if it's being
used anywhere, if not I'll respin this patch. So problem solved for everyone :)
> > Also, if we do get rid of this, then __alloc_pages_bulk() can lose
> > the "list" argument.
>
> Yepp.
Yeah the respin will take this into account, if the function remains unused in
May.
On Mon, 19 Dec 2022 at 10:07, Mel Gorman <[email protected]> wrote:
>
> On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> I reckon we should give it another few months until May. There has been
> one user recently that tried to use list but it turned out arrays were
> more appropriate.
>
It being May 31st, it feels appropriate to chase this up :)
David's series at [0] did initially use this function, before
switching to the _array() variant. Other than that it seems that it
remains unused.
Cheers, Lorenzo
[0]:https://lore.kernel.org/all/[email protected]/
> --
> Mel Gorman
> SUSE Labs
--
Lorenzo Stoakes
https://ljs.io
On Wed, May 31, 2023 at 8:24 AM Lorenzo Stoakes <[email protected]> wrote:
>
> On Mon, 19 Dec 2022 at 10:07, Mel Gorman <[email protected]> wrote:
> >
> > On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > I reckon we should give it another few months until May. There has been
> > one user recently that tried to use list but it turned out arrays were
> > more appropriate.
> >
>
> It being May 31st, it feels appropriate to chase this up :)
>
> David's series at [0] did initially use this function, before
> switching to the _array() variant. Other than that it seems that it
> remains unused.
Thanks for following this up. My patchset (use bulk allocator for
dm-crypt) also switched from list API to a new callback API suggested
by Mel. But I haven't heard anything back from Mel about it yet. I
forgot to follow up due to too many distractions.
>
> Cheers, Lorenzo
>
> [0]:https://lore.kernel.org/all/[email protected]/
>
> > --
> > Mel Gorman
> > SUSE Labs
>
>
>
> --
> Lorenzo Stoakes
> https://ljs.io
>
On Wed, May 31, 2023 at 02:29:01PM -0700, Yang Shi wrote:
> On Wed, May 31, 2023 at 8:24???AM Lorenzo Stoakes <[email protected]> wrote:
> >
> > On Mon, 19 Dec 2022 at 10:07, Mel Gorman <[email protected]> wrote:
> > >
> > > On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > > I reckon we should give it another few months until May. There has been
> > > one user recently that tried to use list but it turned out arrays were
> > > more appropriate.
> > >
> >
> > It being May 31st, it feels appropriate to chase this up :)
> >
> > David's series at [0] did initially use this function, before
> > switching to the _array() variant. Other than that it seems that it
> > remains unused.
>
> Thanks for following this up. My patchset (use bulk allocator for
> dm-crypt) also switched from list API to a new callback API suggested
> by Mel. But I haven't heard anything back from Mel about it yet. I
> forgot to follow up due to too many distractions.
>
It needs a follow-up. My various inboxes that I get cc'd on currently exceed
1000 mails due to being offline a lot during the last few months so there
could be a lot hiding in there. Conceptually at least I don't recall having
any problem with the callback patches as long as the dm-crypt people are
happy. I vaguely recall a review disappeared off into the weeds talking
about maybe using physically contiguous pages (missing the point of using
the bulk allocator at all and ignoring fragmentation concerns affecting
allocation success rates) but I'm not certain.
--
Mel Gorman
SUSE Labs
On Thu, Jun 01, 2023 at 10:10:02AM +0100, Mel Gorman wrote:
> On Wed, May 31, 2023 at 02:29:01PM -0700, Yang Shi wrote:
> > On Wed, May 31, 2023 at 8:24???AM Lorenzo Stoakes <[email protected]> wrote:
> > >
> > > On Mon, 19 Dec 2022 at 10:07, Mel Gorman <[email protected]> wrote:
> > > >
> > > > On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > > > I reckon we should give it another few months until May. There has been
> > > > one user recently that tried to use list but it turned out arrays were
> > > > more appropriate.
> > > >
> > >
> > > It being May 31st, it feels appropriate to chase this up :)
> > >
> > > David's series at [0] did initially use this function, before
> > > switching to the _array() variant. Other than that it seems that it
> > > remains unused.
> >
> > Thanks for following this up. My patchset (use bulk allocator for
> > dm-crypt) also switched from list API to a new callback API suggested
> > by Mel. But I haven't heard anything back from Mel about it yet. I
> > forgot to follow up due to too many distractions.
> >
>
> It needs a follow-up. My various inboxes that I get cc'd on currently exceed
> 1000 mails due to being offline a lot during the last few months so there
> could be a lot hiding in there. Conceptually at least I don't recall having
> any problem with the callback patches as long as the dm-crypt people are
> happy. I vaguely recall a review disappeared off into the weeds talking
> about maybe using physically contiguous pages (missing the point of using
> the bulk allocator at all and ignoring fragmentation concerns affecting
> allocation success rates) but I'm not certain.
Entirely understandable re: this (rather trivial) removal, and obviously
this is far from high priority so no rush.
However, if you feel Yang's series ought to use a different interface and
given no other users of alloc_pages_bulk_list() have emerged, I am happy to
go ahead and respin the patch. If you want to wait absolutely no problem
either! :)
Cheers, Lorenzo
>
> --
> Mel Gorman
> SUSE Labs
On Thu, Jun 1, 2023 at 2:10 AM Mel Gorman <[email protected]> wrote:
>
> On Wed, May 31, 2023 at 02:29:01PM -0700, Yang Shi wrote:
> > On Wed, May 31, 2023 at 8:24???AM Lorenzo Stoakes <[email protected]> wrote:
> > >
> > > On Mon, 19 Dec 2022 at 10:07, Mel Gorman <[email protected]> wrote:
> > > >
> > > > On Sat, Dec 17, 2022 at 08:34:58AM +0000, Lorenzo Stoakes wrote:
> > > > I reckon we should give it another few months until May. There has been
> > > > one user recently that tried to use list but it turned out arrays were
> > > > more appropriate.
> > > >
> > >
> > > It being May 31st, it feels appropriate to chase this up :)
> > >
> > > David's series at [0] did initially use this function, before
> > > switching to the _array() variant. Other than that it seems that it
> > > remains unused.
> >
> > Thanks for following this up. My patchset (use bulk allocator for
> > dm-crypt) also switched from list API to a new callback API suggested
> > by Mel. But I haven't heard anything back from Mel about it yet. I
> > forgot to follow up due to too many distractions.
> >
>
> It needs a follow-up. My various inboxes that I get cc'd on currently exceed
> 1000 mails due to being offline a lot during the last few months so there
> could be a lot hiding in there. Conceptually at least I don't recall having
> any problem with the callback patches as long as the dm-crypt people are
> happy. I vaguely recall a review disappeared off into the weeds talking
> about maybe using physically contiguous pages (missing the point of using
> the bulk allocator at all and ignoring fragmentation concerns affecting
> allocation success rates) but I'm not certain.
Yes, other than that someone also complained the changeset is too big,
and suggested just try to allocate compound page and fallback to the
mempool allocator if that fails instead of creating a dedicated
mempool API.
Do you prefer follow up in that v2 thread or I send a new v3
(basically same with v2, just rebased on the latest mm-unstable)?
>
> --
> Mel Gorman
> SUSE Labs