This introduces a new helper and switches current users to use it.
parisc and s390 weren't tested anyhow, the other are compile tested.
Andy Shevchenko (5):
seq_file: provide an analogue of print_hex_dump()
saa7164: convert to seq_hex_dump()
crypto: qat - use seq_hex_dump() to dump buffers
parisc: use seq_hex_dump() to dump buffers
[S390] zcrypt: use seq_hex_dump() to dump buffers
.../crypto/qat/qat_common/adf_transport_debug.c | 16 ++--------
drivers/media/pci/saa7164/saa7164-core.c | 31 +++----------------
drivers/parisc/ccio-dma.c | 14 ++-------
drivers/parisc/sba_iommu.c | 11 ++-----
drivers/s390/crypto/zcrypt_api.c | 10 +------
fs/seq_file.c | 35 ++++++++++++++++++++++
include/linux/seq_file.h | 4 +++
7 files changed, 52 insertions(+), 69 deletions(-)
--
2.0.1
The new seq_hex_dump() is a complete analogue of print_hex_dump().
We have few users of this functionality already. It allows to reduce their
codebase.
Signed-off-by: Andy Shevchenko <[email protected]>
---
fs/seq_file.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/seq_file.h | 4 ++++
2 files changed, 39 insertions(+)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3857b72..fec4a6b 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/cred.h>
#include <linux/mm.h>
+#include <linux/printk.h>
#include <asm/uaccess.h>
#include <asm/page.h>
@@ -794,6 +795,40 @@ void seq_pad(struct seq_file *m, char c)
}
EXPORT_SYMBOL(seq_pad);
+/* Analogue of print_hex_dump() */
+void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type,
+ int rowsize, int groupsize, const void *buf, size_t len,
+ bool ascii)
+{
+ const u8 *ptr = buf;
+ int i, linelen, remaining = len;
+ unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+
+ if (rowsize != 16 && rowsize != 32)
+ rowsize = 16;
+
+ for (i = 0; i < len; i += rowsize) {
+ linelen = min(remaining, rowsize);
+ remaining -= rowsize;
+
+ hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
+ linebuf, sizeof(linebuf), ascii);
+
+ switch (prefix_type) {
+ case DUMP_PREFIX_ADDRESS:
+ seq_printf(m, "%s%p: %s\n", prefix_str, ptr + i, linebuf);
+ break;
+ case DUMP_PREFIX_OFFSET:
+ seq_printf(m, "%s%.8x: %s\n", prefix_str, i, linebuf);
+ break;
+ default:
+ seq_printf(m, "%s%s\n", prefix_str, linebuf);
+ break;
+ }
+ }
+}
+EXPORT_SYMBOL(seq_hex_dump);
+
struct list_head *seq_list_start(struct list_head *head, loff_t pos)
{
struct list_head *lh;
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 52e0097..6a8be4c 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -107,6 +107,10 @@ int seq_write(struct seq_file *seq, const void *data, size_t len);
__printf(2, 3) int seq_printf(struct seq_file *, const char *, ...);
__printf(2, 0) int seq_vprintf(struct seq_file *, const char *, va_list args);
+void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type,
+ int rowsize, int groupsize, const void *buf, size_t len,
+ bool ascii);
+
int seq_path(struct seq_file *, const struct path *, const char *);
int seq_dentry(struct seq_file *, struct dentry *, const char *);
int seq_path_root(struct seq_file *m, const struct path *path,
--
2.0.1
Instead of custom approach let's use recently introduced seq_hex_dump() helper.
In this case it slightly changes the output, namely the four tetrads will be
output on one line.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/s390/crypto/zcrypt_api.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index 0e18c5d..d1f9983 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -1203,16 +1203,8 @@ static void sprinthx(unsigned char *title, struct seq_file *m,
static void sprinthx4(unsigned char *title, struct seq_file *m,
unsigned int *array, unsigned int len)
{
- int r;
-
seq_printf(m, "\n%s\n", title);
- for (r = 0; r < len; r++) {
- if ((r % 8) == 0)
- seq_printf(m, " ");
- seq_printf(m, "%08X ", array[r]);
- if ((r % 8) == 7)
- seq_putc(m, '\n');
- }
+ seq_hex_dump(m, " ", DUMP_PREFIX_NONE, 32, 4, array, len, false);
seq_putc(m, '\n');
}
--
2.0.1
Instead of custom approach let's use recently added seq_hex_dump() helper.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/media/pci/saa7164/saa7164-core.c | 31 ++++---------------------------
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/media/pci/saa7164/saa7164-core.c b/drivers/media/pci/saa7164/saa7164-core.c
index 1bf0697..6f81584 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -1065,7 +1065,6 @@ static int saa7164_proc_show(struct seq_file *m, void *v)
struct saa7164_dev *dev;
struct tmComResBusInfo *b;
struct list_head *list;
- int i, c;
if (saa7164_devcount == 0)
return 0;
@@ -1089,35 +1088,13 @@ static int saa7164_proc_show(struct seq_file *m, void *v)
seq_printf(m, " .m_pdwGetReadPos = 0x%x (0x%08x)\n",
b->m_dwGetWritePos, saa7164_readl(b->m_dwGetWritePos));
- c = 0;
seq_printf(m, "\n Set Ring:\n");
- seq_printf(m, "\n addr 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f\n");
- for (i = 0; i < b->m_dwSizeSetRing; i++) {
- if (c == 0)
- seq_printf(m, " %04x:", i);
+ seq_hex_dump(m, " ", DUMP_PREFIX_OFFSET, 16, 1,
+ b->m_pdwSetRing, b->m_dwSizeSetRing, false);
- seq_printf(m, " %02x", *(b->m_pdwSetRing + i));
-
- if (++c == 16) {
- seq_printf(m, "\n");
- c = 0;
- }
- }
-
- c = 0;
seq_printf(m, "\n Get Ring:\n");
- seq_printf(m, "\n addr 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f\n");
- for (i = 0; i < b->m_dwSizeGetRing; i++) {
- if (c == 0)
- seq_printf(m, " %04x:", i);
-
- seq_printf(m, " %02x", *(b->m_pdwGetRing + i));
-
- if (++c == 16) {
- seq_printf(m, "\n");
- c = 0;
- }
- }
+ seq_hex_dump(m, " ", DUMP_PREFIX_OFFSET, 16, 1,
+ b->m_pdwGetRing, b->m_dwSizeGetRing, false);
mutex_unlock(&b->lock);
--
2.0.1
Instead of custom approach let's use recently introduced seq_hex_dump() helper.
In this case it slightly changes the output, namely the four tetrads will be
output on one line.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/crypto/qat/qat_common/adf_transport_debug.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/qat/qat_common/adf_transport_debug.c b/drivers/crypto/qat/qat_common/adf_transport_debug.c
index 6b69745..8b103cf 100644
--- a/drivers/crypto/qat/qat_common/adf_transport_debug.c
+++ b/drivers/crypto/qat/qat_common/adf_transport_debug.c
@@ -86,9 +86,7 @@ static int adf_ring_show(struct seq_file *sfile, void *v)
{
struct adf_etr_ring_data *ring = sfile->private;
struct adf_etr_bank_data *bank = ring->bank;
- uint32_t *msg = v;
void __iomem *csr = ring->bank->csr_addr;
- int i, x;
if (v == SEQ_START_TOKEN) {
int head, tail, empty;
@@ -111,18 +109,8 @@ static int adf_ring_show(struct seq_file *sfile, void *v)
seq_puts(sfile, "----------- Ring data ------------\n");
return 0;
}
- seq_printf(sfile, "%p:", msg);
- x = 0;
- i = 0;
- for (; i < (ADF_MSG_SIZE_TO_BYTES(ring->msg_size) >> 2); i++) {
- seq_printf(sfile, " %08X", *(msg + i));
- if ((ADF_MSG_SIZE_TO_BYTES(ring->msg_size) >> 2) != i + 1 &&
- (++x == 8)) {
- seq_printf(sfile, "\n%p:", msg + i + 1);
- x = 0;
- }
- }
- seq_puts(sfile, "\n");
+ seq_hex_dump(sfile, "", DUMP_PREFIX_ADDRESS, 16, 4,
+ v, ADF_MSG_SIZE_TO_BYTES(ring->msg_size), false);
return 0;
}
--
2.0.1
Instead of custom approach let's use recently introduced seq_hex_dump() helper.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/parisc/ccio-dma.c | 14 +++-----------
drivers/parisc/sba_iommu.c | 11 +++--------
2 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index 8b490d7..9d353d2 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -1101,20 +1101,12 @@ static const struct file_operations ccio_proc_info_fops = {
static int ccio_proc_bitmap_info(struct seq_file *m, void *p)
{
- int len = 0;
struct ioc *ioc = ioc_list;
while (ioc != NULL) {
- u32 *res_ptr = (u32 *)ioc->res_map;
- int j;
-
- for (j = 0; j < (ioc->res_size / sizeof(u32)); j++) {
- if ((j & 7) == 0)
- len += seq_puts(m, "\n ");
- len += seq_printf(m, "%08x", *res_ptr);
- res_ptr++;
- }
- len += seq_puts(m, "\n\n");
+ seq_hex_dump(m, " ", DUMP_PREFIX_NONE, 32, 4, ioc->res_map,
+ ioc->res_size, false);
+ seq_putc(m, '\n');
ioc = ioc->next;
break; /* XXX - remove me */
}
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index 1ff1b67..fbc4db9 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -1857,15 +1857,10 @@ sba_proc_bitmap_info(struct seq_file *m, void *p)
{
struct sba_device *sba_dev = sba_list;
struct ioc *ioc = &sba_dev->ioc[0]; /* FIXME: Multi-IOC support! */
- unsigned int *res_ptr = (unsigned int *)ioc->res_map;
- int i, len = 0;
- for (i = 0; i < (ioc->res_size/sizeof(unsigned int)); ++i, ++res_ptr) {
- if ((i & 7) == 0)
- len += seq_printf(m, "\n ");
- len += seq_printf(m, " %08x", *res_ptr);
- }
- len += seq_printf(m, "\n");
+ seq_hex_dump(m, " ", DUMP_PREFIX_NONE, 32, 4, ioc->res_map,
+ ioc->res_size, false);
+ seq_printf(m, "\n");
return 0;
}
--
2.0.1
On Wed, Jul 9, 2014 at 11:24 AM, Andy Shevchenko
<[email protected]> wrote:
> Instead of custom approach let's use recently added seq_hex_dump() helper.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
ack
Reviewed-by: Steven Toth <[email protected]>
--
Steven Toth - Kernel Labs
http://www.kernellabs.com
On Wed, 2014-07-09 at 18:24 +0300, Andy Shevchenko wrote:
> This introduces a new helper and switches current users to use it.
While seq_print_hex_dump seems useful, I'm not sure
existing forms can be changed to use it if any output
content changes.
seq_ is supposed to be a stable API.
On Wed, 2014-07-09 at 18:24 +0300, Andy Shevchenko wrote:
> Instead of custom approach let's use recently introduced seq_hex_dump() helper.
Doesn't this also change the output from
1111111122222222333333334444444455555555666666667777777788888888
to
11111111 22222222 33333333 44444444 55555555 66666666 77777777 88888888
In one case indeed it does, in another - no, though it seems it prints
same data (by meaning) in both cases. I would like driver maintainer
to say a word what they think about it.
On Wed, Jul 9, 2014 at 9:26 PM, Joe Perches <[email protected]> wrote:
> On Wed, 2014-07-09 at 18:24 +0300, Andy Shevchenko wrote:
>> Instead of custom approach let's use recently introduced seq_hex_dump() helper.
>
> Doesn't this also change the output from
> 1111111122222222333333334444444455555555666666667777777788888888
> to
> 11111111 22222222 33333333 44444444 55555555 66666666 77777777 88888888
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
With Best Regards,
Andy Shevchenko
On Wednesday, July 09, 2014 at 05:24:26 PM, Andy Shevchenko wrote:
> The new seq_hex_dump() is a complete analogue of print_hex_dump().
>
> We have few users of this functionality already. It allows to reduce their
> codebase.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> fs/seq_file.c | 35 +++++++++++++++++++++++++++++++++++
> include/linux/seq_file.h | 4 ++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 3857b72..fec4a6b 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -12,6 +12,7 @@
> #include <linux/slab.h>
> #include <linux/cred.h>
> #include <linux/mm.h>
> +#include <linux/printk.h>
>
> #include <asm/uaccess.h>
> #include <asm/page.h>
> @@ -794,6 +795,40 @@ void seq_pad(struct seq_file *m, char c)
> }
> EXPORT_SYMBOL(seq_pad);
>
> +/* Analogue of print_hex_dump() */
> +void seq_hex_dump(struct seq_file *m, const char *prefix_str, int
> prefix_type, + int rowsize, int groupsize, const void *buf,
size_t len,
> + bool ascii)
> +{
> + const u8 *ptr = buf;
> + int i, linelen, remaining = len;
> + unsigned char linebuf[32 * 3 + 2 + 32 + 1];
> +
> + if (rowsize != 16 && rowsize != 32)
> + rowsize = 16;
> +
> + for (i = 0; i < len; i += rowsize) {
> + linelen = min(remaining, rowsize);
> + remaining -= rowsize;
> +
> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> + linebuf, sizeof(linebuf), ascii);
> +
> + switch (prefix_type) {
> + case DUMP_PREFIX_ADDRESS:
> + seq_printf(m, "%s%p: %s\n", prefix_str, ptr + i,
linebuf);
> + break;
> + case DUMP_PREFIX_OFFSET:
> + seq_printf(m, "%s%.8x: %s\n", prefix_str, i, linebuf);
> + break;
> + default:
> + seq_printf(m, "%s%s\n", prefix_str, linebuf);
> + break;
> + }
> + }
> +}
> +EXPORT_SYMBOL(seq_hex_dump);
The above function looks like almost verbatim copy of print_hex_dump(). The only
difference I can spot is that it's calling seq_printf() instead of printk(). Can
you not instead generalize print_hex_dump() and based on it's invocation, make
it call either seq_printf() or printk() ?
Best regards,
On Wed, 2014-07-09 at 22:39 +0200, Marek Vasut wrote:
> The above function looks like almost verbatim copy of print_hex_dump(). The only
> difference I can spot is that it's calling seq_printf() instead of printk(). Can
> you not instead generalize print_hex_dump() and based on it's invocation, make
> it call either seq_printf() or printk() ?
How do you propose doing that given any seq_<foo> call
requires a struct seq_file * and print_hex_dump needs
a KERN_<LEVEL>.
Is there an actual value to it?
On Wednesday, July 09, 2014 at 11:21:08 PM, Joe Perches wrote:
> On Wed, 2014-07-09 at 22:39 +0200, Marek Vasut wrote:
> > The above function looks like almost verbatim copy of print_hex_dump().
> > The only difference I can spot is that it's calling seq_printf() instead
> > of printk(). Can you not instead generalize print_hex_dump() and based
> > on it's invocation, make it call either seq_printf() or printk() ?
>
> How do you propose doing that given any seq_<foo> call
> requires a struct seq_file * and print_hex_dump needs
> a KERN_<LEVEL>.
I can imagine a rather nasty way, I can't say I would like it myself tho. The
general idea would be to pull out the entire switch {} statement into a separate
functions , one for printk() and one for seq_printf() cases. Then, have a
generic do_hex_dump() call which would take as an argument a pointer to either
of those functions and a void * to either the seq_file or level . Finally, there
would have to be a wrapper to call the do_hex_dump() with the correct function
pointer and it's associated arg.
Nasty? Yes ... Ineffective? Most likely.
> Is there an actual value to it?
Reducing the code duplication, but I wonder if there is a smarter solution than
the horrid one above.
Best regards,
Marek Vasut
On Thu, 2014-07-10 at 09:58 +0200, Marek Vasut wrote:
> On Wednesday, July 09, 2014 at 11:21:08 PM, Joe Perches wrote:
> > On Wed, 2014-07-09 at 22:39 +0200, Marek Vasut wrote:
> > > The above function looks like almost verbatim copy of print_hex_dump().
> > > The only difference I can spot is that it's calling seq_printf() instead
> > > of printk(). Can you not instead generalize print_hex_dump() and based
> > > on it's invocation, make it call either seq_printf() or printk() ?
> >
> > How do you propose doing that given any seq_<foo> call
> > requires a struct seq_file * and print_hex_dump needs
> > a KERN_<LEVEL>.
>
> I can imagine a rather nasty way, I can't say I would like it myself tho. The
> general idea would be to pull out the entire switch {} statement into a separate
> functions , one for printk() and one for seq_printf() cases. Then, have a
> generic do_hex_dump() call which would take as an argument a pointer to either
> of those functions and a void * to either the seq_file or level . Finally, there
> would have to be a wrapper to call the do_hex_dump() with the correct function
> pointer and it's associated arg.
>
> Nasty? Yes ... Ineffective? Most likely.
It looks not good idea, yeah.
> > Is there an actual value to it?
>
> Reducing the code duplication, but I wonder if there is a smarter solution than
> the horrid one above.
I have considered to modify hex_dump_to_buffer() to return how many
bytes it actually proceed to the buffer. In that case we can directly
print to m->buf like other seq_<foo> calls do.
But I still have doubts about it. Any opinion?
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Wed, 2014-07-09 at 18:24 +0300, Andy Shevchenko wrote:
> Instead of custom approach let's use recently introduced seq_hex_dump() helper.
>
> In this case it slightly changes the output, namely the four tetrads will be
> output on one line.
The above paragraph is not true and will be removed in v2.
The output is kept the same as in original code.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/s390/crypto/zcrypt_api.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index 0e18c5d..d1f9983 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -1203,16 +1203,8 @@ static void sprinthx(unsigned char *title, struct seq_file *m,
> static void sprinthx4(unsigned char *title, struct seq_file *m,
> unsigned int *array, unsigned int len)
> {
> - int r;
> -
> seq_printf(m, "\n%s\n", title);
> - for (r = 0; r < len; r++) {
> - if ((r % 8) == 0)
> - seq_printf(m, " ");
> - seq_printf(m, "%08X ", array[r]);
> - if ((r % 8) == 7)
> - seq_putc(m, '\n');
> - }
> + seq_hex_dump(m, " ", DUMP_PREFIX_NONE, 32, 4, array, len, false);
> seq_putc(m, '\n');
> }
>
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Thu, 2014-07-10 at 12:50 +0300, Andy Shevchenko wrote:
> I have considered to modify hex_dump_to_buffer() to return how many
> bytes it actually proceed to the buffer. In that case we can directly
> print to m->buf like other seq_<foo> calls do.
>
> But I still have doubts about it. Any opinion?
Simpler is better.
On Wed, 2014-07-09 at 11:31 -0700, Tadeusz Struk wrote:
> On 07/09/2014 08:24 AM, Andy Shevchenko wrote:
>
> > In this case it slightly changes the output, namely the four tetrads will be
> > output on one line.
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> It's ok, I can still read it.
It seems I miscalculated the tetrads. It should be 8 per line, correct?
In that case we could easily use 32 bytes per line and thus remove that
paragraph from commit message.
> Acked-by: Tadeusz Struk <[email protected]>
Thanks!
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
Em Wed, 9 Jul 2014 14:24:29 -0400
Steven Toth <[email protected]> escreveu:
> On Wed, Jul 9, 2014 at 11:24 AM, Andy Shevchenko
> <[email protected]> wrote:
> > Instead of custom approach let's use recently added seq_hex_dump() helper.
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> ack
>
> Reviewed-by: Steven Toth <[email protected]>
>
Acked-by: Mauro Carvalho Chehab <[email protected]>
Regards,
Mauro