| Message ID | 20221217001554.554913-1-lstoakes@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: remove unused alloc_pages_bulk_list() | expand |
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 <lstoakes@gmail.com> > --- > 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.
On Mon, Dec 19, 2022 at 2:07 AM Mel Gorman <mgorman@techsingularity.net> 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 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 Tue, Dec 20, 2022 at 09:56:19AM -0800, Yang Shi wrote: > On Mon, Dec 19, 2022 at 2:07 AM Mel Gorman <mgorman@techsingularity.net> 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.
On Mon, 19 Dec 2022 at 10:07, Mel Gorman <mgorman@techsingularity.net> 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/20230522205744.2825689-1-dhowells@redhat.com/ > -- > Mel Gorman > SUSE Labs
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) {
This function is not referenced anywhere else in the kernel, remove it. Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com> --- include/linux/gfp.h | 7 ------- 1 file changed, 7 deletions(-)