All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>,
	NFS list <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] pnfs-obj: Uglify objio_segment allocation for the sake of the principle :-(
Date: Wed, 14 Mar 2012 11:33:00 -0700	[thread overview]
Message-ID: <4F60E45C.4010603@panasas.com> (raw)
In-Reply-To: <CA+55aFzCuDCo1tJvgetOVhwnO_W+FaYBDZiuZTGYD+YA4iwp0Q@mail.gmail.com>

On 03/13/2012 09:37 PM, Linus Torvalds wrote:
> On Tue, Mar 13, 2012 at 8:44 PM, Boaz Harrosh <bharrosh@panasas.com> wrote:
>> Since then any use of "variable length arrays" has become blasphemous.
>> Even in perfectly good, beautiful, perfectly safe code like the one
>> below where the variable length arrays are only used as a sizeof()
>> parameter, for type-safe dynamic structure allocations. GCC is not
>> executing any stack allocation code.
> 
> If it's a compile-time constant, just make it one. Stop the whining.
> No VLA's required. You could, for example, make a trivial wrapper
> macro that just declares that array as a constant array in the caller,
> and it really *is* a constant sized array with no questions asked.
> 

You have not read the code. I do not have a "constant sized array"
I have a function that receives num_entries as parameter.

In turn, the function declares an array *type* of variable size,
just a type, not an actual array. Then uses that type as a
sizeof() parameter to take the size and call kalloc for actual
object allocation.

So all I'm doing is using the compiler generated math to
multiply the record size by num_entries. No other code is
generated.

The reason I like them a lot is because I'm now not doing any
*casting* like I do with the new code. After I did the wrong
math and stumped all over Kernel memory a couple of times I
like the compiler to check me out.

> Instead, you waste more time and effort *whining* than doing that
> technical solution.
> 

Well I also did the change which is not that bad. So since I sent
in a fix, I'm entitled to  a *whine*.

Specially in light of the assembly proof I sent. Which you did
not quote.

> You do realize that VLA's have caused real problems, since you even
> quote some of the background. Not to mention all the problems they
> cause for semantics analysis and static checkers.
> 

Sure, that's true

> VLA's really aren't a very good thing in the kernel. And the
> work-around I suggest above is actually *simpler* than using them and
> then spednign the effort explaining "but they are actually constants
> at compile time after all, and aren't the bad kinds of VLAs".
> 

Again, I am actually using real VLAs but only as a type definition
not as an object declaration. And that part works very well.

(Again see assembler attached)

> Just don't do them. They are a mistake.
> 

Yes! So I've sent in a patch. Assembler wise it's identical.
static checkers should now be happy. So here it is.

Just that I don't have to like it or agree with you.

>                   Linus

Thanks for your reply, always a pleasure
Boaz

  reply	other threads:[~2012-03-14 18:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-14  3:44 [PATCH] pnfs-obj: Uglify objio_segment allocation for the sake of the principle :-( Boaz Harrosh
2012-03-14  4:37 ` Linus Torvalds
2012-03-14 18:33   ` Boaz Harrosh [this message]
2012-03-14  5:15 ` Al Viro
2012-03-14 18:57   ` Boaz Harrosh

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=4F60E45C.4010603@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.