[v2,01/39] lib/string_helpers: Add flags param to string_get_size()
Commit Message
From: Kent Overstreet <kent.overstreet@linux.dev>
The new flags parameter allows controlling
- Whether or not the units suffix is separated by a space, for
compatibility with sort -h
- Whether or not to append a B suffix - we're not always printing
bytes.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
drivers/block/virtio_blk.c | 4 ++--
drivers/gpu/drm/gud/gud_drv.c | 2 +-
drivers/mmc/core/block.c | 4 ++--
drivers/mtd/spi-nor/debugfs.c | 6 ++---
.../ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 4 ++--
drivers/scsi/sd.c | 8 +++----
include/linux/string_helpers.h | 13 +++++-----
lib/string_helpers.c | 24 +++++++++++++------
lib/test-string_helpers.c | 4 ++--
mm/hugetlb.c | 8 +++----
11 files changed, 44 insertions(+), 35 deletions(-)
Comments
(Minimized the list of people for my review / comments)
On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
> From: Kent Overstreet <kent.overstreet@linux.dev>
>
> The new flags parameter allows controlling
> - Whether or not the units suffix is separated by a space, for
> compatibility with sort -h
> - Whether or not to append a B suffix - we're not always printing
> bytes.
...
> string_get_size(nblocks, queue_logical_block_size(q),
> - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> + 0, cap_str_10, sizeof(cap_str_10));
This doesn't seem right (even if it works). We shouldn't rely on the
implementation details.
...
> - string_get_size(sdkp->capacity, sector_size,
> - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> + string_get_size(sdkp->capacity, sector_size, 0,
> + cap_str_10, sizeof(cap_str_10));
Neither this.
...
> -/* Descriptions of the types of units to
> - * print in */
> -enum string_size_units {
> - STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> - STRING_UNITS_2, /* use binary powers of 2^10 */
> +enum string_size_flags {
So, please add UNITS_10 as it is now. It will help if anybody in the future
wants to add, e.g., 8-base numbers.
> + STRING_SIZE_BASE2 = (1 << 0),
> + STRING_SIZE_NOSPACE = (1 << 1),
> + STRING_SIZE_NOBYTES = (1 << 2),
> };
Please, add necessary comments.
...
> +enum string_size_units {
> + STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> + STRING_UNITS_2, /* use binary powers of 2^10 */
> +};
And what a point now in having these?
I assume you need to split this to a few patches:
1) rename parameter to be a flags without renaming the definitions (this will
touch only string_helpers part);
2) do the end job by renaming it all over the drivers;
3) add the other flags one-by-one (each in a separate change);
4) use new flags where it's needed;
Also see below.
...
> static const char *const units_10[] = {
> - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> + "", "k", "M", "G", "T", "P", "E", "Z", "Y"
> };
> static const char *const units_2[] = {
> - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
> };
Ouch, instead of leaving this and actually "cutting the letter" with NO* flags,
you did something different.
...
Now the main part. Since in 50+% cases (I briefly estimated, it may be more)
this is used in printf() why not introducing a new pointer extension for that?
Yes, it may be done separately, but it will look like a double effort to me.
Instead it might give us a possibility to scale w/o touching users each time
we want to do something and at the same time hide this complete API under
printf() implementation.
On Tue, Oct 24, 2023 at 7:26 AM Andy Shevchenko <andy@kernel.org> wrote:
>
> (Minimized the list of people for my review / comments)
Thanks! I'll let Kent address your feedback.
>
> On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
> > From: Kent Overstreet <kent.overstreet@linux.dev>
> >
> > The new flags parameter allows controlling
> > - Whether or not the units suffix is separated by a space, for
> > compatibility with sort -h
> > - Whether or not to append a B suffix - we're not always printing
> > bytes.
>
> ...
>
> > string_get_size(nblocks, queue_logical_block_size(q),
> > - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> > + 0, cap_str_10, sizeof(cap_str_10));
>
> This doesn't seem right (even if it works). We shouldn't rely on the
> implementation details.
>
> ...
>
> > - string_get_size(sdkp->capacity, sector_size,
> > - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
>
> > + string_get_size(sdkp->capacity, sector_size, 0,
> > + cap_str_10, sizeof(cap_str_10));
>
> Neither this.
>
> ...
>
> > -/* Descriptions of the types of units to
> > - * print in */
> > -enum string_size_units {
> > - STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > - STRING_UNITS_2, /* use binary powers of 2^10 */
> > +enum string_size_flags {
>
> So, please add UNITS_10 as it is now. It will help if anybody in the future
> wants to add, e.g., 8-base numbers.
>
> > + STRING_SIZE_BASE2 = (1 << 0),
> > + STRING_SIZE_NOSPACE = (1 << 1),
> > + STRING_SIZE_NOBYTES = (1 << 2),
> > };
>
> Please, add necessary comments.
>
> ...
>
> > +enum string_size_units {
> > + STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > + STRING_UNITS_2, /* use binary powers of 2^10 */
> > +};
>
> And what a point now in having these?
>
> I assume you need to split this to a few patches:
>
> 1) rename parameter to be a flags without renaming the definitions (this will
> touch only string_helpers part);
> 2) do the end job by renaming it all over the drivers;
> 3) add the other flags one-by-one (each in a separate change);
> 4) use new flags where it's needed;
>
> Also see below.
>
> ...
>
> > static const char *const units_10[] = {
> > - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> > + "", "k", "M", "G", "T", "P", "E", "Z", "Y"
> > };
> > static const char *const units_2[] = {
> > - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> > + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
> > };
>
> Ouch, instead of leaving this and actually "cutting the letter" with NO* flags,
> you did something different.
>
> ...
>
> Now the main part. Since in 50+% cases (I briefly estimated, it may be more)
> this is used in printf() why not introducing a new pointer extension for that?
>
> Yes, it may be done separately, but it will look like a double effort to me.
> Instead it might give us a possibility to scale w/o touching users each time
> we want to do something and at the same time hide this complete API under
> printf() implementation.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Tue, Oct 24, 2023 at 05:26:18PM +0300, Andy Shevchenko wrote:
> (Minimized the list of people for my review / comments)
>
> On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
> > From: Kent Overstreet <kent.overstreet@linux.dev>
> >
> > The new flags parameter allows controlling
> > - Whether or not the units suffix is separated by a space, for
> > compatibility with sort -h
> > - Whether or not to append a B suffix - we're not always printing
> > bytes.
>
> ...
>
> > string_get_size(nblocks, queue_logical_block_size(q),
> > - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> > + 0, cap_str_10, sizeof(cap_str_10));
>
> This doesn't seem right (even if it works). We shouldn't rely on the
> implementation details.
It's now a flags parameter: passing an empty set of flags is not
"relying on an implementation detail".
> > -/* Descriptions of the types of units to
> > - * print in */
> > -enum string_size_units {
> > - STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > - STRING_UNITS_2, /* use binary powers of 2^10 */
> > +enum string_size_flags {
>
> So, please add UNITS_10 as it is now. It will help if anybody in the future
> wants to add, e.g., 8-base numbers.
Octal human readable numbers? No, no one's wanted that so far and I
very much doubt anyone will want that in the future.
> > + STRING_SIZE_BASE2 = (1 << 0),
> > + STRING_SIZE_NOSPACE = (1 << 1),
> > + STRING_SIZE_NOBYTES = (1 << 2),
> > };
>
> Please, add necessary comments.
That I can do.
> > +enum string_size_units {
> > + STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > + STRING_UNITS_2, /* use binary powers of 2^10 */
> > +};
>
> And what a point now in having these?
Minimizing the size of the diff and making it more reviewable. It's fine
as an internal implementation thing.
>
> I assume you need to split this to a few patches:
>
> 1) rename parameter to be a flags without renaming the definitions (this will
> touch only string_helpers part);
> 2) do the end job by renaming it all over the drivers;
> 3) add the other flags one-by-one (each in a separate change);
> 4) use new flags where it's needed;
No, those would not be atomic changes. In particular changing the
parameter to a flags without changing the callers - that's not how we do
things.
We're currently working towards _better_ type safety for enums, fyi.
The new flags _could_ be a separate patch, but since it would be
touching much the same code as the previous patch I don't see the point
in splitting it.
> > static const char *const units_10[] = {
> > - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> > + "", "k", "M", "G", "T", "P", "E", "Z", "Y"
> > };
> > static const char *const units_2[] = {
> > - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> > + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
> > };
>
> Ouch, instead of leaving this and actually "cutting the letter" with NO* flags,
> you did something different.
Not sure I understand your complaint? Were you attached to the redundant
Bs?
> Now the main part. Since in 50+% cases (I briefly estimated, it may be more)
> this is used in printf() why not introducing a new pointer extension for that?
>
> Yes, it may be done separately, but it will look like a double effort to me.
> Instead it might give us a possibility to scale w/o touching users each time
> we want to do something and at the same time hide this complete API under
> printf() implementation.
No, I would not be in favor of another %p extension: in particular,
since this takes integer inputs the lack of type safety for %p
extensions coupled with C's very relaxed approach to integer type
conversion would be a really nasty footgun.
On Tue, Oct 24, 2023 at 03:46:53PM -0400, Kent Overstreet wrote:
> On Tue, Oct 24, 2023 at 05:26:18PM +0300, Andy Shevchenko wrote:
> > On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
...
> > > string_get_size(nblocks, queue_logical_block_size(q),
> > > - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> > > + 0, cap_str_10, sizeof(cap_str_10));
> >
> > This doesn't seem right (even if it works). We shouldn't rely on the
> > implementation details.
>
> It's now a flags parameter: passing an empty set of flags is not
> "relying on an implementation detail".
0 is the "default" flag which is definitely relies on the "implementation
detail". And I think that it's better that caller will explicitly tell what
they want.
...
> > > -/* Descriptions of the types of units to
> > > - * print in */
> > > -enum string_size_units {
> > > - STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > > - STRING_UNITS_2, /* use binary powers of 2^10 */
> > > +enum string_size_flags {
> >
> > So, please add UNITS_10 as it is now. It will help if anybody in the future
> > wants to add, e.g., 8-base numbers.
>
> Octal human readable numbers? No, no one's wanted that so far and I
> very much doubt anyone will want that in the future.
I also in doubt, but still, the explicit is better than implicit in this case
in my opinion.
> > > + STRING_SIZE_BASE2 = (1 << 0),
> > > + STRING_SIZE_NOSPACE = (1 << 1),
> > > + STRING_SIZE_NOBYTES = (1 << 2),
> > > };
...
> > > +enum string_size_units {
> > > + STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > > + STRING_UNITS_2, /* use binary powers of 2^10 */
> > > +};
> >
> > And what a point now in having these?
>
> Minimizing the size of the diff and making it more reviewable. It's fine
> as an internal implementation thing.
It's not an issue to rename these all over the places as you already did that
for most of the users. And minimizing diff could be done better with
--histogram algorithm or so. Otherwise it is not an objective here, right?
...
> > I assume you need to split this to a few patches:
> >
> > 1) rename parameter to be a flags without renaming the definitions (this will
> > touch only string_helpers part);
> > 2) do the end job by renaming it all over the drivers;
> > 3) add the other flags one-by-one (each in a separate change);
> > 4) use new flags where it's needed;
>
> No, those would not be atomic changes. In particular changing the
> parameter to a flags without changing the callers - that's not how we do
> things.
> We're currently working towards _better_ type safety for enums, fyi.
>
> The new flags _could_ be a separate patch, but since it would be
> touching much the same code as the previous patch I don't see the point
> in splitting it.
Individual flags can be discussed, objected or approved and won't affect the
rest of the changes. That's why I highly recommend to reconsider splitting of
this change.
It would be possible to squash back if maintainer wants this, but from review
perspective you are adding more burden to the reviewer's shoulders is not good.
...
> > > static const char *const units_10[] = {
> > > - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> > > + "", "k", "M", "G", "T", "P", "E", "Z", "Y"
> > > };
> > > static const char *const units_2[] = {
> > > - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> > > + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
> > > };
> >
> > Ouch, instead of leaving this and actually "cutting the letter" with NO* flags,
> > you did something different.
>
> Not sure I understand your complaint? Were you attached to the redundant
> Bs?
Flag means "cutting" while in the code you "adding" (doing the opposite). Why
not do exactly "cutting" without touching there. Or since you mentioned changes
across the all callers, make them explicitly tell that they want Bytes suffix.
On Thu, Oct 26, 2023 at 04:12:52PM +0300, Andy Shevchenko wrote:
> On Tue, Oct 24, 2023 at 03:46:53PM -0400, Kent Overstreet wrote:
> > On Tue, Oct 24, 2023 at 05:26:18PM +0300, Andy Shevchenko wrote:
> > > On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
>
> ...
>
> > > > string_get_size(nblocks, queue_logical_block_size(q),
> > > > - STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> > > > + 0, cap_str_10, sizeof(cap_str_10));
> > >
> > > This doesn't seem right (even if it works). We shouldn't rely on the
> > > implementation details.
> >
> > It's now a flags parameter: passing an empty set of flags is not
> > "relying on an implementation detail".
>
> 0 is the "default" flag which is definitely relies on the "implementation
> detail". And I think that it's better that caller will explicitly tell what
> they want.
>
> ...
>
> > > > -/* Descriptions of the types of units to
> > > > - * print in */
> > > > -enum string_size_units {
> > > > - STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > > > - STRING_UNITS_2, /* use binary powers of 2^10 */
> > > > +enum string_size_flags {
> > >
> > > So, please add UNITS_10 as it is now. It will help if anybody in the future
> > > wants to add, e.g., 8-base numbers.
> >
> > Octal human readable numbers? No, no one's wanted that so far and I
> > very much doubt anyone will want that in the future.
>
> I also in doubt, but still, the explicit is better than implicit in this case
> in my opinion.
>
> > > > + STRING_SIZE_BASE2 = (1 << 0),
> > > > + STRING_SIZE_NOSPACE = (1 << 1),
> > > > + STRING_SIZE_NOBYTES = (1 << 2),
> > > > };
>
> ...
>
> > > > +enum string_size_units {
> > > > + STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> > > > + STRING_UNITS_2, /* use binary powers of 2^10 */
> > > > +};
> > >
> > > And what a point now in having these?
> >
> > Minimizing the size of the diff and making it more reviewable. It's fine
> > as an internal implementation thing.
>
> It's not an issue to rename these all over the places as you already did that
> for most of the users. And minimizing diff could be done better with
> --histogram algorithm or so. Otherwise it is not an objective here, right?
>
> ...
>
> > > I assume you need to split this to a few patches:
> > >
> > > 1) rename parameter to be a flags without renaming the definitions (this will
> > > touch only string_helpers part);
> > > 2) do the end job by renaming it all over the drivers;
> > > 3) add the other flags one-by-one (each in a separate change);
> > > 4) use new flags where it's needed;
> >
> > No, those would not be atomic changes. In particular changing the
> > parameter to a flags without changing the callers - that's not how we do
> > things.
>
> > We're currently working towards _better_ type safety for enums, fyi.
> >
> > The new flags _could_ be a separate patch, but since it would be
> > touching much the same code as the previous patch I don't see the point
> > in splitting it.
>
> Individual flags can be discussed, objected or approved and won't affect the
> rest of the changes. That's why I highly recommend to reconsider splitting of
> this change.
>
> It would be possible to squash back if maintainer wants this, but from review
> perspective you are adding more burden to the reviewer's shoulders is not good.
>
> ...
>
> > > > static const char *const units_10[] = {
> > > > - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> > > > + "", "k", "M", "G", "T", "P", "E", "Z", "Y"
> > > > };
> > > > static const char *const units_2[] = {
> > > > - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> > > > + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
> > > > };
> > >
> > > Ouch, instead of leaving this and actually "cutting the letter" with NO* flags,
> > > you did something different.
> >
> > Not sure I understand your complaint? Were you attached to the redundant
> > Bs?
>
> Flag means "cutting" while in the code you "adding" (doing the opposite). Why
> not do exactly "cutting" without touching there. Or since you mentioned changes
> across the all callers, make them explicitly tell that they want Bytes suffix.
Andy: to be blunt, you've been pretty hostile and hysterical ("breaking
the kernel!" over debug statements? really?) and the bikeshedding is
getting to be too much - I'm just going to drop this patch from the
series and we'll post process the output as needed.
On Thu, Oct 26, 2023 at 9:45 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
> On Thu, Oct 26, 2023 at 04:12:52PM +0300, Andy Shevchenko wrote:
> > On Tue, Oct 24, 2023 at 03:46:53PM -0400, Kent Overstreet wrote:
> > > On Tue, Oct 24, 2023 at 05:26:18PM +0300, Andy Shevchenko wrote:
> > > > On Tue, Oct 24, 2023 at 06:45:58AM -0700, Suren Baghdasaryan wrote:
...
> Andy: to be blunt, you've been pretty hostile and hysterical ("breaking
> the kernel!" over debug statements? really?)
Where did I say that?
And instead of judging me, look from the reviewer's and maintainer's
point of view. The patch is unreviewable and has not good
maintainability in this form as it does a few things at once.
Also the commit message is poorly written as it has no mention about use cases.
> and the bikeshedding is
> getting to be too much - I'm just going to drop this patch from the
> series and we'll post process the output as needed.
I don't think I bikeshed here too much. This is the regular asking the
reviewers and/or maintainers usually do, so is it too hard to split?
On Mon, Oct 30, 2023 at 10:07:43PM +0200, Andy Shevchenko wrote:
> I don't think I bikeshed here too much. This is the regular asking the
> reviewers and/or maintainers usually do, so is it too hard to split?
Excuse me?
You were objecting to the new approach making it impossible to add
_octal_ support, and your proposed patch split was completely broken.
Are you trying to be more reasonable now...?
@@ -260,7 +260,7 @@ print_mapping(unsigned long start, unsigned long end, unsigned long size, bool e
if (end <= start)
return;
- string_get_size(size, 1, STRING_UNITS_2, buf, sizeof(buf));
+ string_get_size(size, 1, STRING_SIZE_BASE2, buf, sizeof(buf));
pr_info("Mapped 0x%016lx-0x%016lx with %s pages%s\n", start, end, buf,
exec ? " (exec)" : "");
@@ -986,9 +986,9 @@ static void virtblk_update_capacity(struct virtio_blk *vblk, bool resize)
nblocks = DIV_ROUND_UP_ULL(capacity, queue_logical_block_size(q) >> 9);
string_get_size(nblocks, queue_logical_block_size(q),
- STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
+ STRING_SIZE_BASE2, cap_str_2, sizeof(cap_str_2));
string_get_size(nblocks, queue_logical_block_size(q),
- STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
+ 0, cap_str_10, sizeof(cap_str_10));
dev_notice(&vdev->dev,
"[%s] %s%llu %d-byte logical blocks (%s/%s)\n",
@@ -329,7 +329,7 @@ static int gud_stats_debugfs(struct seq_file *m, void *data)
struct gud_device *gdrm = to_gud_device(entry->dev);
char buf[10];
- string_get_size(gdrm->bulk_len, 1, STRING_UNITS_2, buf, sizeof(buf));
+ string_get_size(gdrm->bulk_len, 1, STRING_SIZE_BASE2, buf, sizeof(buf));
seq_printf(m, "Max buffer size: %s\n", buf);
seq_printf(m, "Number of errors: %u\n", gdrm->stats_num_errors);
@@ -2511,7 +2511,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
blk_queue_write_cache(md->queue.queue, cache_enabled, fua_enabled);
- string_get_size((u64)size, 512, STRING_UNITS_2,
+ string_get_size((u64)size, 512, STRING_SIZE_BASE2,
cap_str, sizeof(cap_str));
pr_info("%s: %s %s %s%s\n",
md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
@@ -2707,7 +2707,7 @@ static int mmc_blk_alloc_rpmb_part(struct mmc_card *card,
list_add(&rpmb->node, &md->rpmbs);
- string_get_size((u64)size, 512, STRING_UNITS_2,
+ string_get_size((u64)size, 512, STRING_SIZE_BASE2,
cap_str, sizeof(cap_str));
pr_info("%s: %s %s %s, chardev (%d:%d)\n",
@@ -85,7 +85,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
seq_printf(s, "name\t\t%s\n", info->name);
seq_printf(s, "id\t\t%*ph\n", SPI_NOR_MAX_ID_LEN, nor->id);
- string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf));
+ string_get_size(params->size, 1, STRING_SIZE_BASE2, buf, sizeof(buf));
seq_printf(s, "size\t\t%s\n", buf);
seq_printf(s, "write size\t%u\n", params->writesize);
seq_printf(s, "page size\t%u\n", params->page_size);
@@ -130,14 +130,14 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
struct spi_nor_erase_type *et = &erase_map->erase_type[i];
if (et->size) {
- string_get_size(et->size, 1, STRING_UNITS_2, buf,
+ string_get_size(et->size, 1, STRING_SIZE_BASE2, buf,
sizeof(buf));
seq_printf(s, " %02x (%s) [%d]\n", et->opcode, buf, i);
}
}
if (!(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
- string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf));
+ string_get_size(params->size, 1, STRING_SIZE_BASE2, buf, sizeof(buf));
seq_printf(s, " %02x (%s)\n", SPINOR_OP_CHIP_ERASE, buf);
}
@@ -3457,8 +3457,8 @@ static void mem_region_show(struct seq_file *seq, const char *name,
{
char buf[40];
- string_get_size((u64)to - from + 1, 1, STRING_UNITS_2, buf,
- sizeof(buf));
+ string_get_size((u64)to - from + 1, 1, STRING_SIZE_BASE2,
+ buf, sizeof(buf));
seq_printf(seq, "%-15s %#x-%#x [%s]\n", name, from, to, buf);
}
@@ -2689,10 +2689,10 @@ sd_print_capacity(struct scsi_disk *sdkp,
if (!sdkp->first_scan && old_capacity == sdkp->capacity)
return;
- string_get_size(sdkp->capacity, sector_size,
- STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
- string_get_size(sdkp->capacity, sector_size,
- STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
+ string_get_size(sdkp->capacity, sector_size, STRING_SIZE_BASE2,
+ cap_str_2, sizeof(cap_str_2));
+ string_get_size(sdkp->capacity, sector_size, 0,
+ cap_str_10, sizeof(cap_str_10));
sd_printk(KERN_NOTICE, sdkp,
"%llu %d-byte logical blocks: (%s/%s)\n",
@@ -17,15 +17,14 @@ static inline bool string_is_terminated(const char *s, int len)
return memchr(s, '\0', len) ? true : false;
}
-/* Descriptions of the types of units to
- * print in */
-enum string_size_units {
- STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
- STRING_UNITS_2, /* use binary powers of 2^10 */
+enum string_size_flags {
+ STRING_SIZE_BASE2 = (1 << 0),
+ STRING_SIZE_NOSPACE = (1 << 1),
+ STRING_SIZE_NOBYTES = (1 << 2),
};
-void string_get_size(u64 size, u64 blk_size, enum string_size_units units,
- char *buf, int len);
+int string_get_size(u64 size, u64 blk_size, enum string_size_flags flags,
+ char *buf, int len);
int parse_int_array_user(const char __user *from, size_t count, int **array);
@@ -19,11 +19,17 @@
#include <linux/string.h>
#include <linux/string_helpers.h>
+enum string_size_units {
+ STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
+ STRING_UNITS_2, /* use binary powers of 2^10 */
+};
+
/**
* string_get_size - get the size in the specified units
* @size: The size to be converted in blocks
* @blk_size: Size of the block (use 1 for size in bytes)
- * @units: units to use (powers of 1000 or 1024)
+ * @flags: units to use (powers of 1000 or 1024), whether to include space
+ * separator
* @buf: buffer to format to
* @len: length of buffer
*
@@ -32,14 +38,16 @@
* at least 9 bytes and will always be zero terminated.
*
*/
-void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
- char *buf, int len)
+int string_get_size(u64 size, u64 blk_size, enum string_size_flags flags,
+ char *buf, int len)
{
+ enum string_size_units units = flags & flags & STRING_SIZE_BASE2
+ ? STRING_UNITS_2 : STRING_UNITS_10;
static const char *const units_10[] = {
- "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
+ "", "k", "M", "G", "T", "P", "E", "Z", "Y"
};
static const char *const units_2[] = {
- "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
+ "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"
};
static const char *const *const units_str[] = {
[STRING_UNITS_10] = units_10,
@@ -126,8 +134,10 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
else
unit = units_str[units][i];
- snprintf(buf, len, "%u%s %s", (u32)size,
- tmp, unit);
+ return snprintf(buf, len, "%u%s%s%s%s", (u32)size, tmp,
+ (flags & STRING_SIZE_NOSPACE) ? "" : " ",
+ unit,
+ (flags & STRING_SIZE_NOBYTES) ? "" : "B");
}
EXPORT_SYMBOL(string_get_size);
@@ -507,8 +507,8 @@ static __init void __test_string_get_size(const u64 size, const u64 blk_size,
char buf10[string_get_size_maxbuf];
char buf2[string_get_size_maxbuf];
- string_get_size(size, blk_size, STRING_UNITS_10, buf10, sizeof(buf10));
- string_get_size(size, blk_size, STRING_UNITS_2, buf2, sizeof(buf2));
+ string_get_size(size, blk_size, 0, buf10, sizeof(buf10));
+ string_get_size(size, blk_size, STRING_SIZE_BASE2, buf2, sizeof(buf2));
test_string_get_size_check("STRING_UNITS_10", exp_result10, buf10,
size, blk_size);
@@ -3228,7 +3228,7 @@ static void __init hugetlb_hstate_alloc_pages_onenode(struct hstate *h, int nid)
if (i == h->max_huge_pages_node[nid])
return;
- string_get_size(huge_page_size(h), 1, STRING_UNITS_2, buf, 32);
+ string_get_size(huge_page_size(h), 1, STRING_SIZE_BASE2, buf, 32);
pr_warn("HugeTLB: allocating %u of page size %s failed node%d. Only allocated %lu hugepages.\n",
h->max_huge_pages_node[nid], buf, nid, i);
h->max_huge_pages -= (h->max_huge_pages_node[nid] - i);
@@ -3290,7 +3290,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
if (i < h->max_huge_pages) {
char buf[32];
- string_get_size(huge_page_size(h), 1, STRING_UNITS_2, buf, 32);
+ string_get_size(huge_page_size(h), 1, STRING_SIZE_BASE2, buf, 32);
pr_warn("HugeTLB: allocating %lu of page size %s failed. Only allocated %lu hugepages.\n",
h->max_huge_pages, buf, i);
h->max_huge_pages = i;
@@ -3336,7 +3336,7 @@ static void __init report_hugepages(void)
for_each_hstate(h) {
char buf[32];
- string_get_size(huge_page_size(h), 1, STRING_UNITS_2, buf, 32);
+ string_get_size(huge_page_size(h), 1, STRING_SIZE_BASE2, buf, 32);
pr_info("HugeTLB: registered %s page size, pre-allocated %ld pages\n",
buf, h->free_huge_pages);
pr_info("HugeTLB: %d KiB vmemmap can be freed for a %s page\n",
@@ -4227,7 +4227,7 @@ static int __init hugetlb_init(void)
char buf[32];
string_get_size(huge_page_size(&default_hstate),
- 1, STRING_UNITS_2, buf, 32);
+ 1, STRING_SIZE_BASE2, buf, 32);
pr_warn("HugeTLB: Ignoring hugepages=%lu associated with %s page size\n",
default_hstate.max_huge_pages, buf);
pr_warn("HugeTLB: Using hugepages=%lu for number of default huge pages\n",