| Message ID | 20230302011035.4928-1-dakr@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | maple_tree: export symbol mas_preallocate() | expand |
Thanks for the patch. This should indeed be exported. Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> * Danilo Krummrich <dakr@redhat.com> [230301 20:10]: > Fix missing EXPORT_SYMBOL_GPL() statement for mas_preallocate(). > > Signed-off-by: Danilo Krummrich <dakr@redhat.com> > --- > lib/maple_tree.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 26e2045d3cda..3bfb8a6f3f6d 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -5733,6 +5733,7 @@ int mas_preallocate(struct ma_state *mas, void *entry, gfp_t gfp) > mas_reset(mas); > return ret; > } > +EXPORT_SYMBOL_GPL(mas_preallocate); > > /* > * mas_destroy() - destroy a maple state. > -- > 2.39.2 >
On 3/2/23 02:52, Liam R. Howlett wrote: > Thanks for the patch. This should indeed be exported. > > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> What uses it? Don't we export only things used by in-kernel modules? > * Danilo Krummrich <dakr@redhat.com> [230301 20:10]: >> Fix missing EXPORT_SYMBOL_GPL() statement for mas_preallocate(). >> >> Signed-off-by: Danilo Krummrich <dakr@redhat.com> >> --- >> lib/maple_tree.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> index 26e2045d3cda..3bfb8a6f3f6d 100644 >> --- a/lib/maple_tree.c >> +++ b/lib/maple_tree.c >> @@ -5733,6 +5733,7 @@ int mas_preallocate(struct ma_state *mas, void *entry, gfp_t gfp) >> mas_reset(mas); >> return ret; >> } >> +EXPORT_SYMBOL_GPL(mas_preallocate); >> >> /* >> * mas_destroy() - destroy a maple state. >> -- >> 2.39.2 >> >
On 3/8/23 11:59, Vlastimil Babka wrote: > On 3/2/23 02:52, Liam R. Howlett wrote: >> Thanks for the patch. This should indeed be exported. >> >> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > > What uses it? Don't we export only things used by in-kernel modules? mas_preallocate() is part of the maple tree's 'Advanced API'. All other functions of this API are exported already. More specifically, e.g. mas_store_prealloc() is exported which doesn't make a lot of sense without mas_preallocate() being available too. I don't think it is used outside of mm yet, but as a generic tree implementation maple tree is pretty likely to be used in code built as module, e.g. drivers. > >> * Danilo Krummrich <dakr@redhat.com> [230301 20:10]: >>> Fix missing EXPORT_SYMBOL_GPL() statement for mas_preallocate(). >>> >>> Signed-off-by: Danilo Krummrich <dakr@redhat.com> >>> --- >>> lib/maple_tree.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >>> index 26e2045d3cda..3bfb8a6f3f6d 100644 >>> --- a/lib/maple_tree.c >>> +++ b/lib/maple_tree.c >>> @@ -5733,6 +5733,7 @@ int mas_preallocate(struct ma_state *mas, void *entry, gfp_t gfp) >>> mas_reset(mas); >>> return ret; >>> } >>> +EXPORT_SYMBOL_GPL(mas_preallocate); >>> >>> /* >>> * mas_destroy() - destroy a maple state. >>> -- >>> 2.39.2 >>> >> >
On Wed, Mar 08, 2023 at 03:34:29PM +0100, Danilo Krummrich wrote: > On 3/8/23 11:59, Vlastimil Babka wrote: >> On 3/2/23 02:52, Liam R. Howlett wrote: >>> Thanks for the patch. This should indeed be exported. >>> >>> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> >> >> What uses it? Don't we export only things used by in-kernel modules? > > mas_preallocate() is part of the maple tree's 'Advanced API'. All other > functions of this API are exported already. > > More specifically, e.g. mas_store_prealloc() is exported which doesn't make > a lot of sense without mas_preallocate() being available too. > > I don't think it is used outside of mm yet, but as a generic tree > implementation maple tree is pretty likely to be used in code built as > module, e.g. drivers. If anyone wants to use they can add the export as needed.
On 3/8/23 15:43, Christoph Hellwig wrote: > On Wed, Mar 08, 2023 at 03:34:29PM +0100, Danilo Krummrich wrote: >> On 3/8/23 11:59, Vlastimil Babka wrote: >>> On 3/2/23 02:52, Liam R. Howlett wrote: >>>> Thanks for the patch. This should indeed be exported. >>>> >>>> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> >>> >>> What uses it? Don't we export only things used by in-kernel modules? >> >> mas_preallocate() is part of the maple tree's 'Advanced API'. All other >> functions of this API are exported already. >> >> More specifically, e.g. mas_store_prealloc() is exported which doesn't make >> a lot of sense without mas_preallocate() being available too. >> >> I don't think it is used outside of mm yet, but as a generic tree >> implementation maple tree is pretty likely to be used in code built as >> module, e.g. drivers. > > If anyone wants to use they can add the export as needed. > Valid perception, though personally I wouldn't share it in this specific case. Anyway, it's just that all other functions of the 'Advanced API' are exported already without them having other users than mm either, e.g. mas_store_prealloc() can't be used without mas_preallocate(), hence exporting one, but not the other doesn't really make sense. Consistency wise I think we should either also export mas_preallocate() or don't export the other ones either.
On Wed, Mar 08, 2023 at 04:18:55PM +0100, Danilo Krummrich wrote: > Consistency wise I think we should either also export mas_preallocate() or > don't export the other ones either. Please send a patch to drop all unused exports.
On Wed, Mar 08, 2023 at 04:29:13PM +0100, Christoph Hellwig wrote: > On Wed, Mar 08, 2023 at 04:18:55PM +0100, Danilo Krummrich wrote: > > Consistency wise I think we should either also export mas_preallocate() or > > don't export the other ones either. > > Please send a patch to drop all unused exports. While that's usually a good rule, it just creates unnecessary friction in this kind of case -- a set of library functions which have only been introduced in the last few months. If they're still unused in a year, sure, let's get rid of the exports.
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 26e2045d3cda..3bfb8a6f3f6d 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -5733,6 +5733,7 @@ int mas_preallocate(struct ma_state *mas, void *entry, gfp_t gfp) mas_reset(mas); return ret; } +EXPORT_SYMBOL_GPL(mas_preallocate); /* * mas_destroy() - destroy a maple state.
Fix missing EXPORT_SYMBOL_GPL() statement for mas_preallocate(). Signed-off-by: Danilo Krummrich <dakr@redhat.com> --- lib/maple_tree.c | 1 + 1 file changed, 1 insertion(+)