Re: [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

2015-08-05 Thread Dmitry Torokhov
On Wed, Aug 05, 2015 at 11:08:55AM +0100, Stefano Stabellini wrote:
> On Tue, 4 Aug 2015, Julien Grall wrote:
> > Based on include/xen/mm.h [1], Linux is mistakenly using MFN when GFN
> > is meant, I suspect this is because the first support for Xen was for
> > PV. This resulted in some misimplementation of helpers on ARM and
> > confused developers about the expected behavior.
> > 
> > For instance, with pfn_to_mfn, we expect to get an MFN based on the name.
> > Although, if we look at the implementation on x86, it's returning a GFN.
> > 
> > For clarity and avoid new confusion, replace any reference to mfn with
> > gfn in any helpers used by PV drivers. The x86 code will still keep some
> > reference of pfn_to_mfn but exclusively for PV (a BUG_ON has been added
> > to ensure this). No changes as been made in the hypercall field, even
> > though they may be invalid, in order to keep the same as the defintion
> > in xen repo.
> > 
> > Take also the opportunity to simplify simple construction such
> > as pfn_to_mfn(page_to_pfn(page)) into page_to_gfn. More complex clean up
> > will come in follow-up patches.
> > 
> > [1] 
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
> > 
> > Signed-off-by: Julien Grall 
> > Cc: Stefano Stabellini 
> > Cc: Russell King 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Boris Ostrovsky 
> > Cc: David Vrabel 
> > Cc: Thomas Gleixner 
> > Cc: Ingo Molnar 
> > Cc: "H. Peter Anvin" 
> > Cc: x...@kernel.org
> > Cc: "Roger Pau Monné" 
> > Cc: Dmitry Torokhov 
> > Cc: Ian Campbell 
> > Cc: Wei Liu 
> > Cc: Juergen Gross 
> > Cc: "James E.J. Bottomley" 
> > Cc: Greg Kroah-Hartman 
> > Cc: Jiri Slaby 
> > Cc: Jean-Christophe Plagniol-Villard 
> > Cc: Tomi Valkeinen 
> > Cc: linux-in...@vger.kernel.org
> > Cc: net...@vger.kernel.org
> > Cc: linux-scsi@vger.kernel.org
> > Cc: linuxppc-...@lists.ozlabs.org
> > Cc: linux-fb...@vger.kernel.org
> > Cc: linux-arm-ker...@lists.infradead.org
> 
> Aside from the x86 bits:
> 
> Reviewed-by: Stefano Stabellini 

Not really important, but just in case anyone waits for my ack on input
bits:

Acked-by: Dmitry Torokhov 

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
On Friday, September 05, 2014 11:12:41 PM Tejun Heo wrote:
> On Fri, Sep 05, 2014 at 12:47:16AM -0700, Luis R. Rodriguez wrote:
> > Ah -- well without it the way we "find" drivers that need this new
> > "async feature" is by a bug report and folks saying their system can't
> > boot, or they say their device doesn't come up. That's all. Tracing
> > this to systemd and a timeout was one of the most ugliest things ever.
> > There two insane bug reports you can go check:
> > 
> > mptsas was the first:
> > 
> > http://article.gmane.org/gmane.linux.kernel/1669550
> > https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1297248
> > 
> > Then cxgb4:
> > 
> > https://bugzilla.novell.com/show_bug.cgi?id=877622
> > 
> > I only had Cc'd you on the newest gem pata_marvell :
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=59581
> > 
> > We can't seriously expect to be doing all this work for every driver.
> > a WARN_ONCE() would enable us to find the drivers that need this new
> > async probe "feature".
> 
> This whole approach of trying to mark specific drivers as needing
> "async probing" is completely broken for the problem at hand.  It
> can't address the problem adequately while breaking backward
> compatibility.  I don't think this makes much sense.
> 

Which problem are we talking about here though? It does solve the slow device
stalling the rest if the kernel booting (non-module case) for me.

I also reject the notion that anyone should be relying on drivers to be fully
bound on module loading. It is not nineties anymore. We have hot pluggable
buses, deferred probing, and even for not hot-pluggable ones the module
providing the device itself might not be yet loaded. Any scripts that expect to
find device 100% ready after module loading are simply broken.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
On Sat, Sep 06, 2014 at 02:49:25AM +0900, Tejun Heo wrote:
> Hello,
> 
> On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
> > Which problem are we talking about here though? It does solve the slow 
> > device
> > stalling the rest if the kernel booting (non-module case) for me.
> 
> The other one.  The one with timeout.  Neither cxgb4 or pata_marvell
> has slow probing stalling boot problem.
> 
> > I also reject the notion that anyone should be relying on drivers to be 
> > fully
> > bound on module loading. It is not nineties anymore. We have hot pluggable
> > buses, deferred probing, and even for not hot-pluggable ones the module
> > providing the device itself might not be yet loaded. Any scripts that 
> > expect to
> > find device 100% ready after module loading are simply broken.
> 
> We've been treating loading + probing as a single operation when
> loading drivers and the assumption has always been that the existing
> devices at the time of loading finished probing by the time insmod
> finishes.  We now need to split loading and probing and wait for each
> of them differently.  The *only* thing we can do is somehow making the
> issuer specify that it's gonna wait for probing separately.  I'm not
> sure this can even be up for discussion.  We're talking about a major
> userland visible behavior change.

I do not agree that it is actually user-visible change: generally speaking you
do not really know if device is there or not. They come and go. Like I said,
consider all permutations, with hot-pluggable buses, deferred probing, etc,
etc.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
On Fri, Sep 05, 2014 at 11:12:17AM -0700, Luis R. Rodriguez wrote:
> On Fri, Sep 5, 2014 at 10:49 AM, Tejun Heo  wrote:
> > Hello,
> >
> > On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
> >> Which problem are we talking about here though? It does solve the slow 
> >> device
> >> stalling the rest if the kernel booting (non-module case) for me.
> >
> > The other one.  The one with timeout.  Neither cxgb4 or pata_marvell
> > has slow probing stalling boot problem.
> >
> >> I also reject the notion that anyone should be relying on drivers to be 
> >> fully
> >> bound on module loading. It is not nineties anymore. We have hot pluggable
> >> buses, deferred probing, and even for not hot-pluggable ones the module
> >> providing the device itself might not be yet loaded. Any scripts that 
> >> expect to
> >> find device 100% ready after module loading are simply broken.
> >
> > We've been treating loading + probing as a single operation when
> > loading drivers and the assumption has always been that the existing
> > devices at the time of loading finished probing by the time insmod
> > finishes.  We now need to split loading and probing and wait for each
> > of them differently.  The *only* thing we can do is somehow making the
> > issuer specify that it's gonna wait for probing separately.  I'm not
> > sure this can even be up for discussion.  We're talking about a major
> > userland visible behavior change.  We simply can't change it
> > underneath the existing users.
> 
> Meanwhile we are allowing a major design consideration such as a 30
> second timeout for both init + probe all of a sudden become a hard
> requirement for device drivers. I see your point but can't also be
> introducing major design changes willy nilly either. We *need* a
> solution for the affected drivers.
> 
> Also what stops drivers from going ahead and just implementing their
> own async probe? 

They already do and the problem is that they do that poorly. One of the issues
is that the device is considered bound and so may attempt to suspend/resume
them, or unbind them, and the driver is not ready for such operations to take
place.

And even though driver is bound "synchronously" it does not help the user in
the slightest and the object that is the result of driver initialization is
still created asynchronously and is not ready (well, it might if drivers use
async_schedule as we are doing asych_sycnhronize_full() in module load.unload).

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 2/6] driver-core: add driver async_probe support

2014-09-05 Thread Dmitry Torokhov
Hi Luis,

On Thu, Sep 04, 2014 at 11:37:23PM -0700, Luis R. Rodriguez wrote:
> 1) when a built-in driver takes a few seconds to initialize its
>delays can stall the overall boot process

This patch does not solve the 2nd issue fully as it only calls probe
asynchronously during driver registration (and also only for modules???
- it checks drv->owner in a few places). The device may get created
  after driver is initialized, in this case we still want probe to be
called asynchronously.

I think something like the patch below should work. Note that it uses
async_checdule(), so that will satisy for the moment Tejun's objections
to the behavior with regard to module loading and initialization, but it
does not solve your issue with modules being killed after 30 seconds.

To tell the truth I think systemd should not be doing it; it is not its
place to dictate how long module should take to load. It may print
warnings and we'll work on fixing the drivers, but aborting boot just
because they feel like it took too long is not a good idea.

Thanks.

-- 
Dmitry


driver-core: add driver async_probe support

From: Dmitry Torokhov 

Some devices take a long time when initializing, and not all drivers are
suited to initialize their devices when they are open. For example, input
drivers need to interrogate device in order to publish its capabilities
before userspace will open them. When such drivers are compiled into kernel
they may stall entire kernel initialization.

This change allows drivers request for their probe functions to be called
asynchronously during driver and device registration (manual binding is
still synchronous). Because async_schedule is used to perform asynchronous
calls module loading will still wait for the probing to complete.

This is based on earlier patch by "Luis R. Rodriguez" 

Signed-off-by: Dmitry Torokhov 
---
 drivers/base/bus.c |   31 ++
 drivers/base/dd.c  |  106 +++-
 include/linux/device.h |2 +
 3 files changed, 112 insertions(+), 27 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 83e910a..49fe573 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -10,6 +10,7 @@
  *
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -547,15 +548,12 @@ void bus_probe_device(struct device *dev)
 {
struct bus_type *bus = dev->bus;
struct subsys_interface *sif;
-   int ret;
 
if (!bus)
return;
 
-   if (bus->p->drivers_autoprobe) {
-   ret = device_attach(dev);
-   WARN_ON(ret < 0);
-   }
+   if (bus->p->drivers_autoprobe)
+   device_initial_probe(dev);
 
mutex_lock(&bus->p->mutex);
list_for_each_entry(sif, &bus->p->interfaces, node)
@@ -657,6 +655,17 @@ static ssize_t uevent_store(struct device_driver *drv, 
const char *buf,
 }
 static DRIVER_ATTR_WO(uevent);
 
+static void driver_attach_async(void *_drv, async_cookie_t cookie)
+{
+   struct device_driver *drv = _drv;
+   int ret;
+
+   ret = driver_attach(drv);
+
+   pr_debug("bus: '%s': driver %s async attach completed: %d\n",
+drv->bus->name, drv->name, ret);
+}
+
 /**
  * bus_add_driver - Add a driver to the bus.
  * @drv: driver.
@@ -689,9 +698,15 @@ int bus_add_driver(struct device_driver *drv)
 
klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
if (drv->bus->p->drivers_autoprobe) {
-   error = driver_attach(drv);
-   if (error)
-   goto out_unregister;
+   if (drv->async_probe) {
+   pr_debug("bus: '%s': probing driver %s 
asynchronously\n",
+   drv->bus->name, drv->name);
+   async_schedule(driver_attach_async, drv);
+   } else {
+   error = driver_attach(drv);
+   if (error)
+   goto out_unregister;
+   }
}
module_add_driver(drv->owner, drv);
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index e4ffbcf..67a2f85 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -402,31 +402,52 @@ int driver_probe_device(struct device_driver *drv, struct 
device *dev)
return ret;
 }
 
-static int __device_attach(struct device_driver *drv, void *data)
+struct device_attach_data {
+   struct device *dev;
+   bool check_async;
+   bool want_async;
+   bool have_async;
+};
+
+static int __device_attach_driver(struct device_driver *drv, void *_data)
 {
-   struct device *dev = data;
+   struct device_attach_data *data = _data;
+   struct device *dev = data->dev;
 
if (!driver_match_device(drv, dev))
return 0;
 
+   if (drv->async_

Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
On Fri, Sep 05, 2014 at 03:45:08PM -0700, Arjan van de Ven wrote:
> On 9/5/2014 3:29 PM, Tejun Heo wrote:
> >Hello, Dmitry.
> >
> >On Fri, Sep 05, 2014 at 11:10:03AM -0700, Dmitry Torokhov wrote:
> >>I do not agree that it is actually user-visible change: generally speaking 
> >>you
> >>do not really know if device is there or not. They come and go. Like I said,
> >>consider all permutations, with hot-pluggable buses, deferred probing, etc,
> >
> >It is for storage devices which always have guaranteed synchronous
> >probing on module load and well-defined probing order.  Sure, modern
> >setups are a lot more dynamic but I'm quite certain that there are
> >setups in the wild which depend on storage driver loading being
> >synchronous.  We can't simply declare one day that such behavior is
> >broken and break, most likely, their boots.
> 
> we even depend on this in the mount-by-label cases
> 
> many setups assume that the internal storage prevails over the USB stick in 
> the case of conflicts.
> it's a security issue; you don't want the built in secure bootloader that has 
> a kernel root argument
> by label/uuid.
> the security there tends to assume that built-in wins over USB

Ahem... and they sure it works reliably with large storage arrays? With
SCSI doing probing asynchronously already?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
On Fri, Sep 05, 2014 at 04:05:30PM -0700, Arjan van de Ven wrote:
> On 9/5/2014 3:52 PM, Dmitry Torokhov wrote:
> >On Fri, Sep 05, 2014 at 03:45:08PM -0700, Arjan van de Ven wrote:
> >>On 9/5/2014 3:29 PM, Tejun Heo wrote:
> >>>Hello, Dmitry.
> >>>
> >>>On Fri, Sep 05, 2014 at 11:10:03AM -0700, Dmitry Torokhov wrote:
> >>>>I do not agree that it is actually user-visible change: generally 
> >>>>speaking you
> >>>>do not really know if device is there or not. They come and go. Like I 
> >>>>said,
> >>>>consider all permutations, with hot-pluggable buses, deferred probing, 
> >>>>etc,
> >>>
> >>>It is for storage devices which always have guaranteed synchronous
> >>>probing on module load and well-defined probing order.  Sure, modern
> >>>setups are a lot more dynamic but I'm quite certain that there are
> >>>setups in the wild which depend on storage driver loading being
> >>>synchronous.  We can't simply declare one day that such behavior is
> >>>broken and break, most likely, their boots.
> >>
> >>we even depend on this in the mount-by-label cases
> >>
> >>many setups assume that the internal storage prevails over the USB stick in 
> >>the case of conflicts.
> >>it's a security issue; you don't want the built in secure bootloader that 
> >>has a kernel root argument
> >>by label/uuid.
> >>the security there tends to assume that built-in wins over USB
> >
> >Ahem... and they sure it works reliably with large storage arrays? With
> >SCSI doing probing asynchronously already?
> 
> you tend to trust your large storage array
> you tend to not trust the walk up USB stick.

If you allow physical access it does not matter really.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-05 Thread Dmitry Torokhov
Hi Tejun,

On Sat, Sep 06, 2014 at 07:55:33AM +0900, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Fri, Sep 05, 2014 at 03:49:17PM -0700, Dmitry Torokhov wrote:
> > On Sat, Sep 06, 2014 at 07:31:39AM +0900, Tejun Heo wrote:
> > > On Sat, Sep 06, 2014 at 07:29:56AM +0900, Tejun Heo wrote:
> > > > It is for storage devices which always have guaranteed synchronous
> > > > probing on module load and well-defined probing order.
> > 
> > Agree about probing order (IIRC that is why we had to revert the
> > wholesale asynchronous probing a few years back) but totally disagree
> > about synchronous module loading.
> 
> I don't get it.  This is a behavior userland already depends on for
> boots.  What's there to agree or disagree?  This is just a fact that
> we can't do this w/o disturbing some userlands in a major way.

I am just expressing my disbelief that somebody relies on module loading
being synchronous with probing. Out of curiosity, do you have any
pointers?

> 
> > Anyway, I just posted a patch that I think preserves module loading
> > behavior and solves my issue with built-in modules. It does not help
> > Luis' issue though (but then I think the main problem is with systemd
> > being stupid there).
> 
> This sure can be worked around from userland side too by not imposing
> any timeout on module loading but that said for the same reasons that
> you've been arguing until now, I actually do think that it's kinda
> silly to make device probing synchronous to module loading at this
> time and age.  What we disagree on is not that we want to separate
> those waits.  It is about how to achieve it.

Well, there are separate things we want to solve. My main issue is not
with modules, but rather compiled-in drivers that stall kernel boot,
and these particular drivers are just fine if they are probed out of
bound.

> 
> > > To add a bit, if the argument here is that dependency on such behavior
> > > shouldn't exist and module loading and device probing should always be
> > > asynchronous, the right approach is implementing "synchronous_probing"
> > > flag not the other way around.  I actually wouldn't hate to see that
> > > change happening but whoever submits and routes such a change should
> > > be ready for a major shitstorm, I'm afraid.
> > 
> > I think we already had this storm and that is why here we have opt-in
> > behavior for the drivers.
> 
> It's a different shitstorm where we actively break bootings on some
> userlands.  Trust me.  That's gonna be a lot worse.

That did break bootings and that's why we reverted the wholesale async
probing.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-09 Thread Dmitry Torokhov
On Tuesday, September 09, 2014 03:46:23 PM James Bottomley wrote:
> On Wed, 2014-09-10 at 07:41 +0900, Tejun Heo wrote:
> > 
> > The thing is that we have to have dynamic mechanism to listen for
> > device attachments no matter what and such mechanism has been in place
> > for a long time at this point.  The synchronous wait simply doesn't
> > serve any purpose anymore and kinda gets in the way in that it makes
> > it a possibly extremely slow process to tell whether loading of a
> > module succeeded or not because the wait for the initial round of
> > probe is piggybacked.
> 
> OK, so we just fire and forget in userland ... why bother inventing an
> elaborate new infrastructure in the kernel to do exactly what
> 
> modprobe  &
> 
> would do?

Just so we do not forget: we also want the no-modules case to also be able
to probe asynchronously so that a slow device does not stall kernel booting.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-11 Thread Dmitry Torokhov
On Thu, Sep 11, 2014 at 12:59:25PM -0700, James Bottomley wrote:
> 
> On Tue, 2014-09-09 at 16:01 -0700, Dmitry Torokhov wrote:
> > On Tuesday, September 09, 2014 03:46:23 PM James Bottomley wrote:
> > > On Wed, 2014-09-10 at 07:41 +0900, Tejun Heo wrote:
> > > > 
> > > > The thing is that we have to have dynamic mechanism to listen for
> > > > device attachments no matter what and such mechanism has been in place
> > > > for a long time at this point.  The synchronous wait simply doesn't
> > > > serve any purpose anymore and kinda gets in the way in that it makes
> > > > it a possibly extremely slow process to tell whether loading of a
> > > > module succeeded or not because the wait for the initial round of
> > > > probe is piggybacked.
> > > 
> > > OK, so we just fire and forget in userland ... why bother inventing an
> > > elaborate new infrastructure in the kernel to do exactly what
> > > 
> > > modprobe  &
> > > 
> > > would do?
> > 
> > Just so we do not forget: we also want the no-modules case to also be able
> > to probe asynchronously so that a slow device does not stall kernel booting.
> 
> Yes, but we mostly do this anyway.  SCSI for instance does asynchronous
> scanning of attached devices (once the cards are probed)

What would it do it card was a bit slow to probe?

> but has a sync
> point for ordering.

Quite often we do not really care about ordering of devices. I mean,
does it matter if your mouse is discovered before your keyboard or
after?

>
> The problem of speeding up boot is different from the one of init
> processes killing modprobes.

Right. One is systemd doing stupid things, another is kernel could be
smarter.

>  There are elements in common, but by and
> large the biggest headaches at least in large device number boots have
> already been tackled by the enterprise crowd (they don't like their
> S390's or 1024 core NUMA systems taking half an hour to come up).

Please do not position this as a mostly solved large systems problem,
For us it is touchpad detection stalling kernel for 0.5-1 sec. Which is
a lot given that we boot in seconds.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-11 Thread Dmitry Torokhov
On Thu, Sep 11, 2014 at 01:42:20PM -0700, Luis R. Rodriguez wrote:
> On Thu, Sep 11, 2014 at 1:23 PM, Dmitry Torokhov
>  wrote:
> >
> >>  There are elements in common, but by and
> >> large the biggest headaches at least in large device number boots have
> >> already been tackled by the enterprise crowd (they don't like their
> >> S390's or 1024 core NUMA systems taking half an hour to come up).
> >
> > Please do not position this as a mostly solved large systems problem,
> > For us it is touchpad detection stalling kernel for 0.5-1 sec. Which is
> > a lot given that we boot in seconds.
> 
> Dmitry, would working on top of the aysnc series be reasonable? Then
> we could address these as separate things which we'd build on top of.
> The one aspect I see us needing to share is the "async probe universe
> is OK" flag.

Sure. Are you planning on refreshing your series? I think the
code-related discussion kind of stalled...

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-22 Thread Dmitry Torokhov
On Monday, September 22, 2014 09:49:06 PM Pavel Machek wrote:
> On Thu 2014-09-11 13:23:54, Dmitry Torokhov wrote:
> > On Thu, Sep 11, 2014 at 12:59:25PM -0700, James Bottomley wrote:
> > > On Tue, 2014-09-09 at 16:01 -0700, Dmitry Torokhov wrote:
> > > > On Tuesday, September 09, 2014 03:46:23 PM James Bottomley wrote:
> > > > > On Wed, 2014-09-10 at 07:41 +0900, Tejun Heo wrote:
> > > > > > The thing is that we have to have dynamic mechanism to listen for
> > > > > > device attachments no matter what and such mechanism has been in
> > > > > > place
> > > > > > for a long time at this point.  The synchronous wait simply
> > > > > > doesn't
> > > > > > serve any purpose anymore and kinda gets in the way in that it
> > > > > > makes
> > > > > > it a possibly extremely slow process to tell whether loading of a
> > > > > > module succeeded or not because the wait for the initial round of
> > > > > > probe is piggybacked.
> > > > > 
> > > > > OK, so we just fire and forget in userland ... why bother inventing
> > > > > an
> > > > > elaborate new infrastructure in the kernel to do exactly what
> > > > > 
> > > > > modprobe  &
> > > > > 
> > > > > would do?
> > > > 
> > > > Just so we do not forget: we also want the no-modules case to also be
> > > > able
> > > > to probe asynchronously so that a slow device does not stall kernel
> > > > booting.> > 
> > > Yes, but we mostly do this anyway.  SCSI for instance does asynchronous
> > > scanning of attached devices (once the cards are probed)
> > 
> > What would it do it card was a bit slow to probe?
> > 
> > > but has a sync
> > > point for ordering.
> > 
> > Quite often we do not really care about ordering of devices. I mean,
> > does it matter if your mouse is discovered before your keyboard or
> > after?
> 
> Actually yes, I suspect it does.
> 
> I do evtest /dev/input/eventX by hand, occassionaly. It would be
> annoying if they moved between reboots.

I am sorry but you will have to cope with such annoyances. It' snot like we 
fail to boot the box here.

The systems are now mostly hot-pluggable and userland is supposed to
handle it, and it does, at least for input devices. If you want stable naming
use udev facilities to rename devices as needed or add needed symlinks (by-id, 
etc.).

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support

2014-09-28 Thread Dmitry Torokhov
Hi Luis,

On Fri, Sep 26, 2014 at 02:57:17PM -0700, Luis R. Rodriguez wrote:
> +static bool drv_enable_async_probe(struct device_driver *drv,
> +struct bus_type *bus)
> +{
> + struct module *mod;
> +
> + if (!drv->owner || drv->sync_probe)
> + return false;

This bit is one of the biggest issues I have with the patch set. Why async
probing is limited to modules only? I mentioned several times that we need
async probing for built-in drivers and the way you are structuring the flags
(async by default for modules, possibly opt-out of async for modules, forcibly
sync for built-in) it is hard to extend the infrastructure for built-in case.

Also, as far as I can see, you are only considering the case where driver is
being bound to already registered devices. If you have a module that creates a
device for a driver that is already loaded and takes long time to probe you
would still be probing synchronously even if driver/module requested async
behavior.

So for me it is NAK in the current form.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

2014-09-30 Thread Dmitry Torokhov
On Tue, Sep 30, 2014 at 11:06:34PM +0200, Pavel Machek wrote:
> 
> On Mon 2014-09-22 13:23:54, Dmitry Torokhov wrote:
> > On Monday, September 22, 2014 09:49:06 PM Pavel Machek wrote:
> > > On Thu 2014-09-11 13:23:54, Dmitry Torokhov wrote:
> > > > On Thu, Sep 11, 2014 at 12:59:25PM -0700, James Bottomley wrote:
> 
> > > > > Yes, but we mostly do this anyway.  SCSI for instance does 
> > > > > asynchronous
> > > > > scanning of attached devices (once the cards are probed)
> > > > 
> > > > What would it do it card was a bit slow to probe?
> > > > 
> > > > > but has a sync
> > > > > point for ordering.
> > > > 
> > > > Quite often we do not really care about ordering of devices. I mean,
> > > > does it matter if your mouse is discovered before your keyboard or
> > > > after?
> > > 
> > > Actually yes, I suspect it does.
> > > 
> > > I do evtest /dev/input/eventX by hand, occassionaly. It would be
> > > annoying if they moved between reboots.
> > 
> > I am sorry but you will have to cope with such annoyances. It' snot like we 
> > fail to boot the box here.
> > 
> > The systems are now mostly hot-pluggable and userland is supposed to
> > handle it, and it does, at least for input devices. If you want stable 
> > naming
> > use udev facilities to rename devices as needed or add needed symlinks 
> > (by-id, 
> > etc.).
> 
> Well, it would be nice if udev was not mandatory. Do the sync points
> for ordering actually cost us something?

Yes, boot time. We can save a second or two off the boot time if we probe
several devices/drivers simultaneously.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules

2014-10-06 Thread Dmitry Torokhov
Hi Luis,

On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" 
> 
> At times we may wish to express the desire to prefer to have
> a device driver probe asynchronously by default. We cannot
> simply enable all device drivers to do this without vetting
> that userspace is prepared for this though given that some
> old userspace is expected to exist which is not equipped to
> deal with broad async probe support. This defines a new kernel
> parameter, bus.enable_kern_async=1, to help address this both to
> help enable async probe support for built-in drivers and to
> enable drivers to specify a preference to opt in for async
> probe support.
> 
> If you have a device driver that should use async probe
> support when possible enable the prefer_async_probe bool.
> 
> Folks wishing to test enabling async probe for all built-in
> drivers can enable bus.__DEBUG__kernel_force_mod_async_probe=1,
> if you use that though you are on your own.

Thank you for working on this. However there are still couple of issues
with the async probe.

1. As far as I can see you only handle the case when device is already
present and you load a driver. In this case we will do either async or
sync probing, depending on the driver/module settings. However if driver
has already been loaded/registered and we are adding a new device
(another module load, for example you load i2c controller module and it
enumerates its children, or driver signalled deferral during binding)
the probe will be synchronous regardless of the settings.

2. I thin kin the current implementation deferred binding process is
still single-threaded and basically synchronous.

Both of these issues stem form the fact that you only plugging into
bus_add_driver(), but you also need to plug into bus_probe_device(). I
believe I handled these 2 cases properly in the version of patch I sent
a couple of weeks ago so if you could incorporate that in your work that
would be great.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Pv-drivers] [PATCH 097/493] scsi: remove use of __devexit_p

2012-11-26 Thread Dmitry Torokhov
On Mon, Nov 19, 2012 at 01:20:46PM -0500, Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> needed.
> 

...

>  drivers/scsi/vmw_pvscsi.c | 2 +-

For vmw_pvscsi:
 
Acked-by: Dmitry Torokhov 

Thanks,
Dmitry

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Pv-drivers] [PATCH 192/493] scsi: remove use of __devinit

2012-11-26 Thread Dmitry Torokhov
On Mon, Nov 19, 2012 at 01:22:21PM -0500, Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option so __devinit is no longer
> needed.
> 

...

>  drivers/scsi/vmw_pvscsi.c |  6 +-

For vmw_pvscsi:

Acked-by: Dmitry Torokhov 

Thanks,
Dmitry


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] kfifo: log based kfifo API

2013-01-08 Thread Dmitry Torokhov
Hi Yuanhan,

On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
> The current kfifo API take the kfifo size as input, while it rounds
>  _down_ the size to power of 2 at __kfifo_alloc. This may introduce
> potential issue.
> 
> Take the code at drivers/hid/hid-logitech-dj.c as example:
> 
>   if (kfifo_alloc(&djrcv_dev->notif_fifo,
>DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report),
>GFP_KERNEL)) {
> 
> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct dj_report)
> is 15.
> 
> Which means it wants to allocate a kfifo buffer which can store 8
> dj_report entries at once. The expected kfifo buffer size would be
> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
> size to rounddown_power_of_2(120) =  64, and then allocate a buf
> with 64 bytes, which I don't think this is the original author want.
> 
> With the new log API, we can do like following:
> 
>   int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
>   sizeof(struct dj_report));
> 
>   if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, GFP_KERNEL)) {
> 
> This make sure we will allocate enough kfifo buffer for holding
> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.

Why don't you simply change __kfifo_alloc to round the allocation up
instead of down?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Remove pointless casts from void pointers,

2007-10-26 Thread Dmitry Torokhov
On 10/26/07, Jeff Garzik <[EMAIL PROTECTED]> wrote:
>  drivers/input/touchscreen/h3600_ts_input.c |4 ++--

Acked-by: Dmitry Torokhov <[EMAIL PROTECTED]>

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] add transport class symlink to device object

2005-08-18 Thread Dmitry Torokhov
On 8/18/05, Greg KH <[EMAIL PROTECTED]> wrote:
> @@ -500,9 +519,13 @@ int class_device_add(struct class_device
>}
> 
>class_device_add_attrs(class_dev);
> -   if (class_dev->dev)
> +   if (class_dev->dev) {
> +   class_name = make_class_name(class_dev);
>sysfs_create_link(&class_dev->kobj,
>  &class_dev->dev->kobj, "device");
> +   sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
> + class_name);
> +   }
> 

I wonder if we need to grab a reference to class_dev->dev here:

dev = device_get(class_dev->dev);
if (dev) {
 
}

Otherwise, if device gets unregistered/deleted before class device is
deleted we'll get into trouble when removing the link since
class_dev->dev will be garbage.

.. But grabbing that reference will cause pains in SCSI system which,
when I looked, removed class devices from device's release function.

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] add transport class symlink to device object

2005-08-31 Thread Dmitry Torokhov
On Wednesday 31 August 2005 16:43, Greg KH wrote:
> On Thu, Aug 18, 2005 at 02:50:19PM -0500, Dmitry Torokhov wrote:
> > On 8/18/05, Greg KH <[EMAIL PROTECTED]> wrote:
> > > @@ -500,9 +519,13 @@ int class_device_add(struct class_device
> > >}
> > > 
> > >class_device_add_attrs(class_dev);
> > > -   if (class_dev->dev)
> > > +   if (class_dev->dev) {
> > > +   class_name = make_class_name(class_dev);
> > >sysfs_create_link(&class_dev->kobj,
> > >  &class_dev->dev->kobj, "device");
> > > +   sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
> > > + class_name);
> > > +   }
> > > 
> > 
> > I wonder if we need to grab a reference to class_dev->dev here:
> > 
> > dev = device_get(class_dev->dev);
> > if (dev) {
> >  
> > }
> > 
> > Otherwise, if device gets unregistered/deleted before class device is
> > deleted we'll get into trouble when removing the link since
> > class_dev->dev will be garbage.
> > 
> > .. But grabbing that reference will cause pains in SCSI system which,
> > when I looked, removed class devices from device's release function.
> 
> No the sysfs_create_link() call increments the kobject reference on the
> target of the symlink.  See sysfs_add_link() for details.  So this
> should be just fine, right?
>

Yes, you are right. Sorry for the moise.

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFD driver-core] Lifetime problems of the current driver model

2007-03-30 Thread Dmitry Torokhov

Hi James,

On 3/30/07, James Bottomley <[EMAIL PROTECTED]> wrote:

On Fri, 2007-03-30 at 18:43 +0900, Tejun Heo wrote:
> Orphaning sysfs nodes on unregistration is a big step in this
> direction.  With sysfs reference counting out of the picture,
> implementing 'disconnect immediate' interface only on a few components
> (including request_queue) should suffice for most block device
> drivers.  I'm not familiar with other drivers but I don't think
> they'll be very different.

I agree with this statement.  The big question in my mind is how do we
do it?

The essential problem, and the reason why the lifetime rules are
entangled is that fundamentally, sysfs entries are managed by kobjects.
The things the device drivers are interested in is struct device, which
is usually embedded in driver data structures.  Unfortunately, the
required kobject is usually embedded in struct device meaning that the
relevant driver structure cannot be freed while the sysfs entry still
exists.

It seems to me that the only way to Orphan the sysfs entries is to
separate the kobject and the struct device so their lifetime rules are
no longer entangled.  Then we can free the driver structure with the
embedded struct device while still keeping the necessary kobject around
to perform orphaned sysfs operations.

So it seems to me that what we need to do is to give struct device its
own kref and a pointer to a kobject.  Then we can manage the separate
lifetimes independently.  The device would basically allocate and take a
reference to the kobject on device_add() and relinquish it again (and
null out the kobject pointer) on device_del().  The complexity here is
that we now have to allocate the kobject somewhere else ... probably in
device_add() (it doesn't come for free with the device structures).



If you want to manage lifetime rules independently you might want to
not embed struct device into you subsystems objects but attach them
via pointers and use device_create(). Now that we orphan sysfs access
upon unregistering device this will severe all ties from driver core
to your system once you start teardown of a device and you should be
in clear.

However there are simpler subsystems (input, serio, etc.) where there
is only one core module which provides services to device drivers and
handles registration and deregistration. For such substustems it makes
sense to embed struct devices and manage lifetime for all components
at once.

--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFD driver-core] Lifetime problems of the current driver model

2007-03-30 Thread Dmitry Torokhov

On 3/30/07, James Bottomley <[EMAIL PROTECTED]> wrote:

On Fri, 2007-03-30 at 09:15 -0400, Dmitry Torokhov wrote:
> If you want to manage lifetime rules independently you might want to
> not embed struct device into you subsystems objects but attach them
> via pointers and use device_create(). Now that we orphan sysfs access
> upon unregistering device this will severe all ties from driver core
> to your system once you start teardown of a device and you should be
> in clear.

But that wouldn't really help ... all objects have lifetimes.  What
Tejun is pointing out is that we have two different lifetime
requirements:  driver internal (and subsystem) objects and sysfs
objects ...


Exactly. If driver-private data structures have different lifetime
than device abstractions (like they seem to have with scsi and ide)
then you split them and refcount separately. We are just arguing where
to put the line. You want to split devices and kobjects and rework
infrastructure and I am saying that we already have infrastructure we
need and that split shoud be between generic device structure and
driver-managed device structure.

--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] driver model/scsi: synchronize pm calls with probe/remove

2005-03-21 Thread Dmitry Torokhov
On Mon, 21 Mar 2005 18:18:46 +0900, Tejun Heo <[EMAIL PROTECTED]> wrote:
> Hello, Dmitry, Mochel and James.
> 
> I've been looking at sd code and found seemingly bogus 'if (!sdkp)'
> tests with /* this can happen */ comment.  I've digged changelog and
> found out that this was to prevent oops which occurs if some driver
> gets stuck inside ->probe and the machine goes down and calls back
> ->remove.  IMHO, we should avoid this problem by fixing driver ->probe
> or ->remove callbacks instead of detecting and bypassing
> half-initialized/destroyed devices in pm callbacks.
> 
> This patch read-locks a device's bus using device_pm_down_read_bus()
> before invoking any pm callback.

Hi Tejun,

There are talks about getting rid of bus's rwsem and replacing it with
a per-device semaphore to serialize probe, remove, suspend and resume.
This should resolve entire host of problems including this one, if I
unrerstand it correctly.

Please take a look here:
http://seclists.org/lists/linux-kernel/2005/Mar/5847.html

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] scsi: remove incorrect __exit markups

2017-03-01 Thread Dmitry Torokhov
Even if bus is not hot-pluggable, devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov 
---
 drivers/scsi/sgiwd93.c| 2 +-
 drivers/scsi/sni_53c710.c | 2 +-
 drivers/scsi/zalon.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sgiwd93.c b/drivers/scsi/sgiwd93.c
index 6d215e2fb46d..71b4b91d2215 100644
--- a/drivers/scsi/sgiwd93.c
+++ b/drivers/scsi/sgiwd93.c
@@ -297,7 +297,7 @@ static int sgiwd93_probe(struct platform_device *pdev)
return err;
 }
 
-static int __exit sgiwd93_remove(struct platform_device *pdev)
+static int sgiwd93_remove(struct platform_device *pdev)
 {
struct Scsi_Host *host = platform_get_drvdata(pdev);
struct ip22_hostdata *hdata = (struct ip22_hostdata *) host->hostdata;
diff --git a/drivers/scsi/sni_53c710.c b/drivers/scsi/sni_53c710.c
index 76278072147e..1f9a087daf69 100644
--- a/drivers/scsi/sni_53c710.c
+++ b/drivers/scsi/sni_53c710.c
@@ -117,7 +117,7 @@ static int snirm710_probe(struct platform_device *dev)
return -ENODEV;
 }
 
-static int __exit snirm710_driver_remove(struct platform_device *dev)
+static int snirm710_driver_remove(struct platform_device *dev)
 {
struct Scsi_Host *host = dev_get_drvdata(&dev->dev);
struct NCR_700_Host_Parameters *hostdata =
diff --git a/drivers/scsi/zalon.c b/drivers/scsi/zalon.c
index 97ccb0383539..b2cf1faa819d 100644
--- a/drivers/scsi/zalon.c
+++ b/drivers/scsi/zalon.c
@@ -167,7 +167,7 @@ static struct parisc_device_id zalon_tbl[] = {
 
 MODULE_DEVICE_TABLE(parisc, zalon_tbl);
 
-static int __exit zalon_remove(struct parisc_device *dev)
+static int zalon_remove(struct parisc_device *dev)
 {
struct Scsi_Host *host = dev_get_drvdata(&dev->dev);
 
-- 
2.12.0.rc1.440.g5b76565f74-goog


-- 
Dmitry


Re: [PATCH 3/3] __device_release_driver(): Do not wait for asynchronous probing

2018-10-15 Thread Dmitry Torokhov
On Tue, Oct 02, 2018 at 02:52:23PM -0700, Bart Van Assche wrote:
> Since __device_release_driver() is called with the device lock held
> and since the same device lock is obtained by the functions that
> perform asynchronous probing (driver_attach_async() and
> __device_attach_async_helper()), asynchronous probing is already
> serialized against __device_release_driver(). Remove the
> async_synchronize_full() call from __device_release_driver() to
> avoid that a deadlock is triggered.

Hmm, I see. Well, the idea was to let all outstanding async probes
(scheduled but not yet running) to complete before releasing the driver.
I am not sure if anyone actually needs this behavior and implementing it
in deadlock free way is nigh impossible, so

Acked-by: Dmitry Torokhov 

> 
> See also commit 765230b5f084 ("driver-core: add asynchronous probing
> support for drivers").

Should probably be "Fixes: 765230b5f084 ..."

> 
> Signed-off-by: Bart Van Assche 
> Cc: Dmitry Torokhov 
> Cc: Lee Duncan 
> Cc: Hannes Reinecke 
> Cc: Luis R. Rodriguez 
> Cc: Johannes Thumshirn 
> Cc: Christoph Hellwig 
> Cc: Greg Kroah-Hartman 
> ---
>  drivers/base/dd.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index edfc9f0b1180..d6520de659bd 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -926,9 +926,6 @@ static void __device_release_driver(struct device *dev, 
> struct device *parent)
>  
>   drv = dev->driver;
>   if (drv) {
> - if (driver_allows_async_probing(drv))
> - async_synchronize_full();
> -
>   while (device_links_busy(dev)) {
>   device_unlock(dev);
>   if (parent)
> -- 
> 2.19.0.605.g01d371f741-goog
> 

Thanks.

-- 
Dmitry