All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Maurizio Lombardi <mlombard@redhat.com>,
	akpm@linux-foundation.org, tj@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lib/vsprintf.c: increase the size of the field_width variable
Date: Thu, 10 Sep 2015 01:17:36 -0700	[thread overview]
Message-ID: <1441873056.17219.139.camel@perches.com> (raw)
In-Reply-To: <87613ik8r9.fsf@rasmusvillemoes.dk>

On Thu, 2015-09-10 at 09:56 +0200, Rasmus Villemoes wrote:
> On Thu, Sep 10 2015, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2015-09-10 at 09:04 +0200, Maurizio Lombardi wrote:
> >> On 09/09/2015 08:51 PM, Rasmus Villemoes wrote:
> >> > I'm also a little confused; I don't see what printk has to do with the
> >> > reported problem (I'd expect the /sys/... file to be generated by
> >> > something like seq_printf).
> >> 
> >> In the scsi-debug case scnprintf is used, but it doesn't really matter
> >> because the change I made would influence printk and all its friends as
> >> well... everything that will parse "%*pb[l]".
> >> 
> >> > 
> >> >> %*pb is meant for smallish bitmaps, not big ones.
> >> > 
> >> > Yup. And that leads to my other confusion: Given that the expected
> >> > output is given as "0-15", does the bitmap really consist of > S16_MAX
> >> > bits with only the first 16 set?
> >> > 
> >> 
> >> Yes. To be precise, in the example I mentioned in the commit message, a
> >> bitmap of size = 524288 bits is created.
> >> If you assign this number to a s16 variable the result will be zero and
> >> nothing will be printed.
> >
> > Maurizio, did you try the patch I posted?
> > I think it'll work, but it doesn't fix the
> > fundamental issue of %*pbl with large bitmaps.
> 
> It also won't work for the case at hand if/when the actual bitmap ever
> gets a bit set beyond S16_MAX.

But at least it should work for the bitmap sized <= S16_MAX
which should be the majority of uses.

> A (somewhat ugly?) solution might be to teach %pb another flag, say h (for
> huge), meaning that the pointer is actually (struct printf_bitmap*),
> with 
> 
> struct printf_bitmap { unsigned long *bits; unsigned long nbits; }
> 
> Then callers with potentially huge bitmaps would do
> 
> struct printf_bitmap tmp = { my_bitmap, my_size };
> snprintf("%pbhl", &tmp)

Yes, but it still couldn't work without the ability to have
large output buffers and printk doesn't support that.

seq_printf might have some performance issue with it too as
it would repetitively try to emit, fail, and grow the buffer.



  reply	other threads:[~2015-09-10  8:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 10:13 [PATCH] lib/vsprintf.c: increase the size of the field_width variable Maurizio Lombardi
2015-09-09 13:39 ` Tejun Heo
2015-09-09 16:33 ` Joe Perches
2015-09-09 16:36   ` Tejun Heo
2015-09-09 16:55     ` Joe Perches
2015-09-09 18:51   ` Rasmus Villemoes
2015-09-09 19:26     ` Joe Perches
2015-09-10 14:36       ` Tejun Heo
2015-09-10 15:41         ` Joe Perches
2015-09-10 15:47           ` Tejun Heo
2015-09-10  7:04     ` Maurizio Lombardi
2015-09-10  7:13       ` Maurizio Lombardi
2015-09-10  7:38       ` Joe Perches
2015-09-10  7:56         ` Rasmus Villemoes
2015-09-10  8:17           ` Joe Perches [this message]
2015-09-10 14:43             ` Tejun Heo
2015-09-10  8:39         ` Maurizio Lombardi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441873056.17219.139.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mlombard@redhat.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.