[RFC] seq_buf: Export symbols to external modules

[RFC] seq_buf: Export symbols to external modules

[Vaibhav Jain]

'seq_buf' provides a very useful abstraction for writing to a string
buffer without needing to worry about it over-flowing. However even
though the API has been stable for couple of years now its stills not
exported to external modules limiting its usage.

Hence this patch proposes update to 'seq_buf.c' to mark all functions
seq_buf_X() which are part of the seq_seq API to be exported to
external GPL modules.

Earlier work:
There was an earlier proposal by Borislav Petkov <bp@alien8.de> to
export seq_buf_printf() to modules at [1], as part of his EDAC
patch-set "EDAC, mce_amd: Add a tracepoint for the decoded
error". However the proposed patch was never merged and its fate is
unknown as I couldn't locate any subsequent discussion as to why patch
in [1] was dropped.

References:
[1]: https://lore.kernel.org/lkml/20170825102411.8682-5-bp@alien8.de/
[2]: https://lore.kernel.org/lkml/20170825092757.434f1eda@gandalf.local.home/

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 lib/seq_buf.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 4e865d42ab03..3aaa685e902d 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -43,6 +43,7 @@ int seq_buf_print_seq(struct seq_file *m, struct seq_buf *s)
 
 	return seq_write(m, s->buffer, len);
 }
+EXPORT_SYMBOL_GPL(seq_buf_print_seq);
 
 /**
  * seq_buf_vprintf - sequence printing of information.
@@ -70,6 +71,7 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)
 	seq_buf_set_overflow(s);
 	return -1;
 }
+EXPORT_SYMBOL_GPL(seq_buf_vprintf);
 
 /**
  * seq_buf_printf - sequence printing of information
@@ -91,6 +93,7 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(seq_buf_printf);
 
 #ifdef CONFIG_BINARY_PRINTF
 /**
@@ -127,6 +130,7 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)
 	seq_buf_set_overflow(s);
 	return -1;
 }
+EXPORT_SYMBOL_GPL(seq_buf_bprintf);
 #endif /* CONFIG_BINARY_PRINTF */
 
 /**
@@ -156,6 +160,7 @@ int seq_buf_puts(struct seq_buf *s, const char *str)
 	seq_buf_set_overflow(s);
 	return -1;
 }
+EXPORT_SYMBOL_GPL(seq_buf_puts);
 
 /**
  * seq_buf_putc - sequence printing of simple character
@@ -177,6 +182,7 @@ int seq_buf_putc(struct seq_buf *s, unsigned char c)
 	seq_buf_set_overflow(s);
 	return -1;
 }
+EXPORT_SYMBOL_GPL(seq_buf_putc);
 
 /**
  * seq_buf_putmem - write raw data into the sequenc buffer
@@ -202,6 +208,7 @@ int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len)
 	seq_buf_set_overflow(s);
 	return -1;
 }
+EXPORT_SYMBOL_GPL(seq_buf_putmem);
 
 #define MAX_MEMHEX_BYTES	8U
 #define HEX_CHARS		(MAX_MEMHEX_BYTES*2 + 1)
@@ -251,6 +258,7 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(seq_buf_putmem_hex);
 
 /**
  * seq_buf_path - copy a path into the sequence buffer
@@ -282,6 +290,7 @@ int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc)
 
 	return res;
 }
+EXPORT_SYMBOL_GPL(seq_buf_path);
 
 /**
  * seq_buf_to_user - copy the squence buffer to user space
@@ -328,6 +337,7 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf, int cnt)
 	s->readpos += cnt;
 	return cnt;
 }
+EXPORT_SYMBOL_GPL(seq_buf_to_user);
 
 /**
  * seq_buf_hex_dump - print formatted hex dump into the sequence buffer
@@ -390,3 +400,4 @@ int seq_buf_hex_dump(struct seq_buf *s, const char *prefix_str, int prefix_type,
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(seq_buf_hex_dump);
-- 
2.25.2

Re: [RFC] seq_buf: Export symbols to external modules

[Steven Rostedt]

On Thu, 16 Apr 2020 09:21:24 +0530
Vaibhav Jain <vaibhav@linux.ibm.com> wrote:

> 'seq_buf' provides a very useful abstraction for writing to a string
> buffer without needing to worry about it over-flowing. However even
> though the API has been stable for couple of years now its stills not
> exported to external modules limiting its usage.
> 
> Hence this patch proposes update to 'seq_buf.c' to mark all functions
> seq_buf_X() which are part of the seq_seq API to be exported to
> external GPL modules.
> 
> Earlier work:
> There was an earlier proposal by Borislav Petkov <bp@alien8.de> to
> export seq_buf_printf() to modules at [1], as part of his EDAC
> patch-set "EDAC, mce_amd: Add a tracepoint for the decoded
> error". However the proposed patch was never merged and its fate is
> unknown as I couldn't locate any subsequent discussion as to why patch
> in [1] was dropped.
> 
> References:
> [1]: https://lore.kernel.org/lkml/20170825102411.8682-5-bp@alien8.de/
> [2]: https://lore.kernel.org/lkml/20170825092757.434f1eda@gandalf.local.home/
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
> Cc: Cezary Rojewski <cezary.rojewski@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
>  lib/seq_buf.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
>

I'm perfectly fine with this change, but recently there's been a lot of
discussion about doing something like this for out-of-tree modules. Is
there going to be a use case for in tree modules for this? It will make the
case much easier to get this accepted.

-- Steve

Re: [RFC] seq_buf: Export symbols to external modules

[Vaibhav Jain]

Thanks for looking into this patch Steven,

Steven Rostedt <rostedt@goodmis.org> writes:

> On Thu, 16 Apr 2020 09:21:24 +0530
> Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
>> 'seq_buf' provides a very useful abstraction for writing to a string
>> buffer without needing to worry about it over-flowing. However even
>> though the API has been stable for couple of years now its stills not
>> exported to external modules limiting its usage.
>> 
>> Hence this patch proposes update to 'seq_buf.c' to mark all functions
>> seq_buf_X() which are part of the seq_seq API to be exported to
>> external GPL modules.
>> 
>> Earlier work:
>> There was an earlier proposal by Borislav Petkov <bp@alien8.de> to
>> export seq_buf_printf() to modules at [1], as part of his EDAC
>> patch-set "EDAC, mce_amd: Add a tracepoint for the decoded
>> error". However the proposed patch was never merged and its fate is
>> unknown as I couldn't locate any subsequent discussion as to why patch
>> in [1] was dropped.
>> 
>> References:
>> [1]: https://lore.kernel.org/lkml/20170825102411.8682-5-bp@alien8.de/
>> [2]: https://lore.kernel.org/lkml/20170825092757.434f1eda@gandalf.local.home/
>> 
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
>> Cc: Cezary Rojewski <cezary.rojewski@intel.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>>  lib/seq_buf.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>>
>
> I'm perfectly fine with this change, but recently there's been a lot of
> discussion about doing something like this for out-of-tree modules. Is
> there going to be a use case for in tree modules for this? It will make the
> case much easier to get this accepted.

Having these symbols exported to modules should simplify generating file
content for pseudo file systems like sysfs or procfs. Many of the in
kernel modules export atleast one such attribute file. Using seq_buf
api provides a safe way to populate the read buffers for these attrs
as these string buffers are PAGE_SIZE in length and a buggy module can
easily cause an overflow.

My specific use-case is exporting a set of nvdimm specific flags from
papr_scm kernel module [1] via sysfs through a patch proposed at [2] and
using seq_buf should considerably simply my code as suggested by Mpe
at [3].

[1] arch/powerpc/platforms/pseries/papr_scm.c
[2] https://lore.kernel.org/linux-nvdimm/20200331143229.306718-2-vaibhav@linux.ibm.com
[3] https://lore.kernel.org/linux-nvdimm/878sjetcis.fsf@mpe.ellerman.id.au

>
> -- Steve

-- 
Vaibhav

Re: [RFC] seq_buf: Export symbols to external modules

[Steven Rostedt]

On Fri, 17 Apr 2020 14:47:48 +0530
Vaibhav Jain <vaibhav@linux.ibm.com> wrote:

> Having these symbols exported to modules should simplify generating file
> content for pseudo file systems like sysfs or procfs. Many of the in
> kernel modules export atleast one such attribute file. Using seq_buf
> api provides a safe way to populate the read buffers for these attrs
> as these string buffers are PAGE_SIZE in length and a buggy module can
> easily cause an overflow.
> 
> My specific use-case is exporting a set of nvdimm specific flags from
> papr_scm kernel module [1] via sysfs through a patch proposed at [2] and
> using seq_buf should considerably simply my code as suggested by Mpe
> at [3].
> 
> [1] arch/powerpc/platforms/pseries/papr_scm.c
> [2] https://lore.kernel.org/linux-nvdimm/20200331143229.306718-2-vaibhav@linux.ibm.com
> [3] https://lore.kernel.org/linux-nvdimm/878sjetcis.fsf@mpe.ellerman.id.au

This patch should be added to a patch series that needs it. Then I'll give
my ack to it. That way, there's a reason to export them.

-- Steve

Re: [RFC] seq_buf: Export symbols to external modules

[Vaibhav Jain]

Steven Rostedt <rostedt@goodmis.org> writes:

> On Fri, 17 Apr 2020 14:47:48 +0530
> Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
>> Having these symbols exported to modules should simplify generating file
>> content for pseudo file systems like sysfs or procfs. Many of the in
>> kernel modules export atleast one such attribute file. Using seq_buf
>> api provides a safe way to populate the read buffers for these attrs
>> as these string buffers are PAGE_SIZE in length and a buggy module can
>> easily cause an overflow.
>> 
>> My specific use-case is exporting a set of nvdimm specific flags from
>> papr_scm kernel module [1] via sysfs through a patch proposed at [2] and
>> using seq_buf should considerably simply my code as suggested by Mpe
>> at [3].
>> 
>> [1] arch/powerpc/platforms/pseries/papr_scm.c
>> [2] https://lore.kernel.org/linux-nvdimm/20200331143229.306718-2-vaibhav@linux.ibm.com
>> [3] https://lore.kernel.org/linux-nvdimm/878sjetcis.fsf@mpe.ellerman.id.au
>
> This patch should be added to a patch series that needs it. Then I'll give
> my ack to it. That way, there's a reason to export them.
Thanks Steve,

I have posted a patch series at 
https://lore.kernel.org/linux-nvdimm/20200508104922.72565-1-vaibhav@linux.ibm.com/
titled "powerpc/papr_scm: Add support for reporting nvdimm health" that
contains a patch to export seq_buf_printf() viz patch
https://lore.kernel.org/linux-nvdimm/20200508104922.72565-3-vaibhav@linux.ibm.com/
titled "seq_buf: Export seq_buf_printf() to external modules"

~ Vaibhav
>
> -- Steve