linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Tejun Heo <tj@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	Doug Thompson <dougthompson@xmission.com>,
	linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Olof Johansson <olof@lixom.net>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Luis R . Rodriguez" <mcgrof@suse.com>
Subject: Re: [PATCH 3/3] EDAC: amd64_edac: decide if driver can load successfully early.
Date: Thu, 19 Mar 2015 09:52:31 -0700	[thread overview]
Message-ID: <20150319165231.GG30732@dtor-ws> (raw)
In-Reply-To: <20150319163330.GN25365@htj.duckdns.org>

On Thu, Mar 19, 2015 at 12:33:30PM -0400, Tejun Heo wrote:
> Hello, Borislav.
> 
> On Thu, Mar 19, 2015 at 05:23:02PM +0100, Borislav Petkov wrote:
> > On Thu, Mar 19, 2015 at 09:12:26AM -0700, Dmitry Torokhov wrote:
> > > By the same token it only makes sense to load e1000e when I have e1000e
> > > device loaded, but we allow it to load anyway. Or psmouse. Or pretty
> > > much any other drivers (sans some platform code). The fact is that we
> > > for long time have separated module loading and driver binding. Loading
> > > driver even without the devices is standard behavior.
> > 
> > FWIW, I always hated that.
> 
> You understand that there are technical reasons behind the current
> behavior?  This is not something people just did on a whim.  We used
> to have autounload and all that but over time moved away from it
> because the trade-offs around the behavior shifted.
> 
> I don't get why you don't understand this.  As a general rule, we
> don't go and implement one-off behaviors like this because it's well
> understood that things like this are more costly in the longer term.
> As said multiple times before, if you think this is a class of problem
> worth solving, do so properly.  Please stop shell scripting in your
> ->probe() in kernel.
> 
> > If I detect at least one NB which is ok, I can then continue and do
> > pci_register_driver(). If there are no suitable NBs, I return an error
> > and don't even touch PCI.
> 
> Please don't.  Consider it nacked preemptively.  If you want to solve
> this and can justify the added complexity, solve it in a general way -
> teach it to the driver model.

I do not think you can teach it to the driver model in general: there is
no definite point when probing is "done". The drivers and devices come
and go, at random moments in time.

So here the driver just assumes that all devices will be enumerated
before the driver is loaded and acts upon this knowledge. The whole
schema is fragile (I mean can I compile it in the kernel and see
breaking because of  link order changes and driver is now initialized
before PCI devices are scanned? Possibly...).

Thanks.

-- 
Dmitry

  parent reply	other threads:[~2015-03-19 16:52 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19  0:49 [RFC/RFT] amd64_edac: avoid doing post-probe setup Dmitry Torokhov
2015-03-19  0:49 ` [PATCH 1/3] EDAC: amd64: stop allocating ecc settings separately Dmitry Torokhov
2015-03-19  0:49 ` [PATCH 2/3] EDAC: amd64_edac: clean up remove_one_instance() Dmitry Torokhov
2015-03-19  0:49 ` [PATCH 3/3] EDAC: amd64_edac: decide if driver can load successfully early Dmitry Torokhov
2015-03-19  9:40   ` Borislav Petkov
2015-03-19 15:29     ` Tejun Heo
2015-03-19 15:35       ` Borislav Petkov
2015-03-19 15:52         ` Dmitry Torokhov
2015-03-19 15:59           ` Borislav Petkov
2015-03-19 16:12             ` Dmitry Torokhov
2015-03-19 16:23               ` Borislav Petkov
2015-03-19 16:33                 ` Tejun Heo
2015-03-19 16:45                   ` Borislav Petkov
2015-03-19 16:49                     ` Tejun Heo
2015-03-19 16:56                       ` Borislav Petkov
2015-03-19 17:03                         ` Tejun Heo
2015-03-19 17:04                           ` Borislav Petkov
2015-03-19 17:10                             ` Tejun Heo
2015-03-19 17:15                               ` Tejun Heo
2015-03-19 17:27                                 ` Borislav Petkov
2015-03-19 17:47                                   ` Tejun Heo
2015-03-19 17:54                                     ` Borislav Petkov
2015-03-19 18:05                                       ` Tejun Heo
2015-03-19 16:52                   ` Dmitry Torokhov [this message]
2015-03-19 17:09                     ` Tejun Heo
2015-03-19 15:55         ` Tejun Heo
2015-03-19 16:01           ` Borislav Petkov
2015-03-19 16:12             ` Tejun Heo
2015-03-19 16:57               ` Dmitry Torokhov
2015-03-19 17:22                 ` Tejun Heo

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=20150319165231.GG30732@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dougthompson@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@suse.com \
    --cc=mchehab@osg.samsung.com \
    --cc=olof@lixom.net \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).