All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sasha Levin <levinsasha928@gmail.com>,
	penberg@kernel.org, john@jfloren.net, kvm@vger.kernel.org,
	asias.hejun@gmail.com, gorcunov@gmail.com,
	prasadjoshi124@gmail.com
Subject: Re: [PATCH 5/5 V2] kvm tools: Initialize and use VESA and VNC
Date: Wed, 25 May 2011 12:17:44 +0200	[thread overview]
Message-ID: <20110525101744.GA30983@elte.hu> (raw)
In-Reply-To: <4DDCD364.9090903@redhat.com>


* Paolo Bonzini <pbonzini@redhat.com> wrote:

> > so what's your point?
> 
> Using this kind of trick makes it harder to share code with other 
> libraries that may require a higher standard of portability (not 
> "better" or "worse", just "higher"). [...]

That's an complication but should be fixable, should it ever happen.

As things stand today we:

 - Are *already* using an ELF linker script, see tools/kvm/bios/rom.ld.S

 - Have multiple valid reasons not to use ((constructor))

 - Want to use sections to implement other useful features as well

If the *only* linker script use would be the init facility then you'd 
probably have a valid point - although the possible code flow 
fragility with ((constructor)) is still a problem: we still would 
want to know when no constructors were executed.

Also it's not clear why ((constructor)) was written in the way it 
was: why apparently no access is given to the array of init functions 
and why it's not possible to turn the auto-execution off but still 
have the array generated, for legitimate cases that want to use data 
driven constructor execution.

> >>>>>> I know portability is not relevant to tools/kvm/, but using 
> >>>>>> unportable tricks for the sake of using them is a direct way 
> >>>>>> to NIH. But oh well all of tools/kvm/ is NIH after all. :)
> >
> > Btw., that NIH claim was rather unfair and uncalled for as well.
> 
> Hey hey I put a smiley for a reason!

Well after two insults in a single paragraph you need to put in at 
least two smileys! Or not write the insults in a technical discssion 
to begin with, especially if you are criticising a patch rather 
forcefully. It will be easily misunderstood as a real insult, despite 
the smiley ;-)

> Anyway I think we both agree that this debate is pointless.  I 
> learnt something (I wasn't aware of interaction between 
> ((constructor)) and static libraries), you learnt something (it's 
> the same with ((section)), and it's intrinsic in how static 
> libraries work).

While i did not know whether static libraries would work with a 
linker script (never tried it - and your experiment suggests that 
they wont), the ((section)) approach we could create a clear runtime 
BUG_ON() assert for a zero-sized array of init function pointers, 
while ((constructor)) will silently not execute initialization 
functions.

Thanks,

	Ingo

  reply	other threads:[~2011-05-25 10:17 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-23 11:19 [PATCH 1/5 V2] kvm tools: Add BIOS INT10 handler Sasha Levin
2011-05-23 11:19 ` [PATCH 2/5 V2] kvm tools: Add video mode to kernel initialization Sasha Levin
2011-05-23 11:30   ` Ingo Molnar
2011-05-23 11:19 ` [PATCH 3/5 V2] kvm tools: Add VESA device Sasha Levin
2011-05-23 11:32   ` Ingo Molnar
2011-05-23 11:19 ` [PATCH 4/5 V2] kvm tools: Update makefile and feature tests Sasha Levin
2011-05-23 11:19 ` [PATCH 5/5 V2] kvm tools: Initialize and use VESA and VNC Sasha Levin
2011-05-23 11:38   ` Ingo Molnar
2011-05-23 11:45     ` Pekka Enberg
2011-05-24  8:37     ` Paolo Bonzini
2011-05-24  8:50       ` Ingo Molnar
2011-05-24  9:10         ` Paolo Bonzini
2011-05-24  9:55           ` Pekka Enberg
2011-05-24 11:22             ` Avi Kivity
2011-05-24 11:26               ` Pekka Enberg
2011-05-24 11:30                 ` Sasha Levin
2011-05-24 11:30                 ` Avi Kivity
2011-05-24 11:38                   ` Pekka Enberg
2011-05-24 11:41                     ` Avi Kivity
2011-05-24 11:56                       ` Pekka Enberg
2011-05-24 12:27                         ` Paolo Bonzini
2011-05-24 14:38                           ` Avi Kivity
2011-05-24 14:37                       ` Avi Kivity
2011-05-24 14:54                         ` Pekka Enberg
2011-05-24 19:03                           ` Ingo Molnar
2011-05-24 19:00                     ` Ingo Molnar
2011-05-24 19:16           ` Ingo Molnar
2011-05-24  9:18         ` Paolo Bonzini
2011-05-24 19:40           ` Ingo Molnar
2011-05-25  8:21             ` Paolo Bonzini
2011-05-25  8:32               ` Ingo Molnar
2011-05-25  9:15                 ` Paolo Bonzini
2011-05-25  9:36                   ` Ingo Molnar
2011-05-25 10:01                     ` Paolo Bonzini
2011-05-25 10:17                       ` Ingo Molnar [this message]
2011-05-25 10:44                         ` Paolo Bonzini
2011-05-25 12:53                           ` Ingo Molnar
2011-05-25 15:37                             ` Paolo Bonzini
2011-05-25  9:49                   ` Ingo Molnar
2011-05-25  8:38               ` Ingo Molnar
2011-05-24  8:51       ` Cyrill Gorcunov
2011-05-23 14:10   ` Pekka Enberg
2011-05-23 11:29 ` [PATCH 1/5 V2] kvm tools: Add BIOS INT10 handler Ingo Molnar

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=20110525101744.GA30983@elte.hu \
    --to=mingo@elte.hu \
    --cc=asias.hejun@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=john@jfloren.net \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=penberg@kernel.org \
    --cc=prasadjoshi124@gmail.com \
    /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.