Re: [PATCH] drm/i915: Rename global i915 to i915_modparams

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 19, 2017 at 04:07:17PM +0300, Joonas Lahtinen wrote:
> On Tue, 2017-09-19 at 13:22 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 19, 2017 at 11:24:41AM +0300, Joonas Lahtinen wrote:
> > > On Mon, 2017-09-18 at 22:07 +0200, Michal Wajdeczko wrote:
> > > > On Mon, 18 Sep 2017 21:11:40 +0200, Jani Nikula <jani.nikula@intel.com>  
> > > > wrote:
> > > > 
> > > > > On Mon, 18 Sep 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> > > > > > Our global struct with params is named exactly the same way
> > > > > > as new preferred name for the drm_i915_private function parameter.
> > > > > > To avoid such name reuse lets use different name for the global.
> > > > > > 
> > > > > > v4: introduction of mkwrite()
> > > > > 
> > > > > Why?
> > > > > 
> > > > > I don't know what you're trying to achieve with the mkwrite() stuff (the
> > > > 
> > > > I was trying to buy at least one more vote, as discussed on IRC
> > > > 
> > > > <quote>
> > > > [14:23:36] <dolphin> I'll be glad to vote for i915_modparams +  
> > > > i915_modparams_mkwrite()
> > > > <quote/>
> > > > 
> > > > > commit message would be the perfect place to explain that) but no matter
> > > > > what it should IMO be a separate patch.
> > > > > 
> > > > > I think the simple s/i915/i915_modparams/ would be fine, and we could
> > > > > move on.
> > > > 
> > > > Note that it all started with this idea.
> > > > See https://patchwork.freedesktop.org/patch/176409/
> > > > 
> > > 
> > > I agree with Jani that the pure rename should be its own patch. That'll
> > > make review much easier. Then have a follow-up that introduces
> > > _mkwrite() and as a bonus makes the struct const or at least makes
> > > sparse complain.
> > 
> > I know we abuse the const+mkwrite type of thing for the device info, but
> > I'm not sure how safe that actually is on account of the compiler being
> > free to assume that const stuff doesn't generally change. I guess if the
> > mkwrite thing happens at some early controlled point it's going to be OK,
> > but if it starts happening at some randomish times we might not be so
> > lucky.
> 
> I see this more as a reason to introduce it.

Introduce what exactly? A bug due to compiler optimizing away some read of
the variable because it can assume that it didn't change?

I think this needs to be well thought out to make sure we don't end up
with some impossible looking bugs.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx