RE: Flaw in the driver-model implementation of attributes

RE: Flaw in the driver-model implementation of attributes

[Perez-Gonzalez, Inaky]

> From: viro@parcelfarce.linux.theplanet.co.uk
> On Tue, Jun 17, 2003 at 08:44:50PM -0700, Perez-Gonzalez, Inaky wrote:
> 
> > Maybe this is going to kill my argument as an analogy, but think
> > about a C++ class hierarchy, where belonging to a class means
> > ...
> 
> But there is no inheritance here.  Block device and IDE disk are
> different objects and relation is not "A is B with <...>", it's
> "among other things, A happens to use B in a way <...>".

Well, the device is an IDE disk that is linked through an IDE 
controller that is linked through a PCI controller to the system bus. 

That IDE device presents an interface. The block layer just presents 
the common interface that all block devices present (and that IDE
and SCSI disks are able to provide) - there is no inheritance, but
the concept is the same.
 
> Moreover, there is no such thing as "physical device 
> of that block device". There might be many.  There 
> might be none.  IOW, we have a bunch of constructors 
> for class "block device" and some of them happen to have
> some kinds of physical devices among their arguments. 

[I happen not to know the block layer as well as you and many others 
do, so please correct me where I am wrong ...]

So what? _every_ block device will have some form of physical 
back-up that can be linked back into sysfs.

In cases like this, doesn't it make sense to have some 
/sys/devices/SOMETHING/ hierarchy for those  "logical" or "virtual" 
devices that back-up those block devices? 

You could even say that RAID and ramdisks -as used in the example 
above- would belong to /sys/devices/"virtual"/raid/ and 
...../ramdisks/; after all, you have to create those devices before
being able to attach them (last time I checked):

(using subdirs for each layer for clarity)

/sys/devices/"virtual"/raid/0
  attr1
  attr2     ...   sysfs specific attrs 
  block/    ... block layer specific attrs
    attr1
    attr2
    component1 -> /sys/devices/pci/FOO/block/part1  
    component1 -> /sys/devices/pci/BAR/block/part4  

(I would also love to see the block device node being dropped in 
the corresponding "block" directory, but that's another story).

And extrapolating even more, I'd expect to see something 
like this for the block devices that are part of a physical device 
(partitions, I mean):

/sys/blabla/pci/3.14159/ide0/0.0
  attr1
  attr2     ...   sysfs specific attrs 
  block/    ... block layer specific attrs
    attr1
    attr2  
    part0/  ... partition specific stuff
      attr1
      attr...
    part1/
      attr1
      attr2 ...
  ide/      ... ide layer specific attrs
    attr1
    attr2 

In the tree structure it makes sense, because each block
device, at the end is or a partition (and thus is embedded
in a "true" block device) or a true block device on a 1:1
relationship with a physical device.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

Re: Flaw in the driver-model implementation of attributes

[viro]

On Wed, Jun 18, 2003 at 12:48:59AM -0700, Perez-Gonzalez, Inaky wrote:
 
> [I happen not to know the block layer as well as you and many others 
> do, so please correct me where I am wrong ...]
> 
> So what? _every_ block device will have some form of physical 
> back-up that can be linked back into sysfs.

... except ones that will not.  Wonderful.  I bow to that logics - there
is nothing it wouldn't cover.
 
> In cases like this, doesn't it make sense to have some 
> /sys/devices/SOMETHING/ hierarchy for those  "logical" or "virtual" 
> devices that back-up those block devices? 
> 
> You could even say that RAID and ramdisks -as used in the example 
> above- would belong to /sys/devices/"virtual"/raid/ and 
> ...../ramdisks/; after all, you have to create those devices before
> being able to attach them (last time I checked):
 
> In the tree structure it makes sense, because each block
> device, at the end is or a partition (and thus is embedded
> in a "true" block device) or a true block device on a 1:1
> relationship with a physical device.

BS.  There is nothing to stop you from having a block device that talks
to userland process instead of any form of hardware.  As the matter of
fact, we already have such a beast - nbd.  There is also RAID - where
there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
that sit on several disks - where do you see 1:1 or 1:n or n:1?
There is such thing as e.g. encrypted loop over NFS.  There are all
sorts of interesting things, with all sorts of interesting relationship
to some pieces of hardware.

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 18 Jun 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:

> BS.  There is nothing to stop you from having a block device that talks
> to userland process instead of any form of hardware.  As the matter of
> fact, we already have such a beast - nbd.  There is also RAID - where
> there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
> that sit on several disks - where do you see 1:1 or 1:n or n:1?
> There is such thing as e.g. encrypted loop over NFS.  There are all
> sorts of interesting things, with all sorts of interesting relationship
> to some pieces of hardware.

This is the sort of thing that bothers me.  Block devices deserve their 
own "view", so we have /sys/block/ -- perhaps to be renamed 
/sys/class/block/.  Fine.

But what other sorts of things deserve their own "view" as well?  Some
are already established, maybe others aren't.  How's a developer supposed
to know whether the driver he's working on deserves its own entry in
/sys/class/ or not?  How's a user supposed to know where in the hierarchy
to look for a particular device?

Here's a suggestion for something that would definitely help.  Create a
listing (maybe in Documentation/driver-model/) of all the major kernel
subsystems that deserve to have their own entries in /sys/class/ (or the
equivalent).  Explain clearly that any device driver that registers with
one of those subsystems will receive a directory in the /sys/class/
hierarchy where it can register its class devices, and say what the name
of that directory will be.  Explain that a driver that doesn't register
with one of these subsystems will simply have to create its own entry in
/sys/devices/ under its parent node.

Not all this infrastructure has been created yet.  For instance, there 
isn't at the moment any place under /sys/class/usb/ for a USB host 
controller driver to register its class device.  But if these ideas were 
formalized and written down, it would be straightforward to fill in the 
missing pieces.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Wed, Jun 18, 2003 at 10:32:22AM -0400, Alan Stern wrote:
> On Wed, 18 Jun 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> > BS.  There is nothing to stop you from having a block device that talks
> > to userland process instead of any form of hardware.  As the matter of
> > fact, we already have such a beast - nbd.  There is also RAID - where
> > there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
> > that sit on several disks - where do you see 1:1 or 1:n or n:1?
> > There is such thing as e.g. encrypted loop over NFS.  There are all
> > sorts of interesting things, with all sorts of interesting relationship
> > to some pieces of hardware.
> 
> This is the sort of thing that bothers me.  Block devices deserve their 
> own "view", so we have /sys/block/ -- perhaps to be renamed 
> /sys/class/block/.  Fine.
> 
> But what other sorts of things deserve their own "view" as well?  Some
> are already established, maybe others aren't.  How's a developer supposed
> to know whether the driver he's working on deserves its own entry in
> /sys/class/ or not?  How's a user supposed to know where in the hierarchy
> to look for a particular device?

Ok, have you _read_ the documentation on the driver model?  In it,
classes and devices are clearly spelled out as to what the differences
are, and what shows up where.

See Pat's linux.conf.au 2003 paper for much more detail.

And yes, I need to fix up the Documentation/driver_model/class.txt with
the most recent info...

In short, devices describe physical things that are present in the
computer system.  Classes describe a type of device, be it virtual or
physical.  Almost always, classes refer to something a user uses through
the /dev filesystem (like mice, tty, block, audio, etc.)

So no, there is not always a 1:1 mapping from classes to devices, that
is why the driver model does not enforce such a mapping at all.  You can
have multiple "struct class_device" structures that point to the same
"struct device" or no "struct device" at all.

Hope this helps to clear up the confusion that seems to be happening.

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 18 Jun 2003, Greg KH wrote:

> Ok, have you _read_ the documentation on the driver model?  In it,
> classes and devices are clearly spelled out as to what the differences
> are, and what shows up where.

Yes, of course I've read it.  It's lacking a number of important details.

For example, nowhere in devices.txt does it say that a device driver
should not create attributes in the struct device's directory since that
directory is owned by the bus driver.  (That's a very easy mistake to
make; at first sight it appears to be the natural way for a driver to
expose details of how it controls a device.)  In fact, nowhere in the
documentation does it say that you shouldn't attach an attribute to an 
object unless you own that object.  Maybe that seems obvious, but when a 
driver is bound to a device can't it be said to "own" that device in some 
sense?

The class.txt document does _not_ explain clearly what the differences
between devices and classes (or more properly, class devices) are.  It
merely says that "A device class describes a type of device..."  It
doesn't say what sorts of devices get to belong to a class; it doesn't
even list the existing classes yet!  (As you are obviously aware.)

Let me ask you this:  Given a device that doesn't fit clearly into any of 
the existing classes, how would you decide whether or not to create a new 
class for it?

> See Pat's linux.conf.au 2003 paper for much more detail.
> 
> And yes, I need to fix up the Documentation/driver_model/class.txt with
> the most recent info...
> 
> In short, devices describe physical things that are present in the
> computer system.  Classes describe a type of device, be it virtual or
> physical.  Almost always, classes refer to something a user uses through
> the /dev filesystem (like mice, tty, block, audio, etc.)

Yes, but _which_ physical things correspond to devices?  And how should 
the parent-child relationships be decided?

Consider a concrete example: a USB host controller.  Let's say that on my
system /sys/devices/pci0/0000:00:07.2 is an OHCI HC.  That particular
object is created by the PCI bus driver, and directly below it is
/sys/devices/pci0/0000:00:07.2/usb1 -- what physical thing does that
correspond to?  Is it the virtual root hub?  It's created by the USB core;
where does the object created by the HC driver belong?

Or have I misunderstood, and was it intended from the start that _all_ the
objects under /sys/devices/ should be created by bus drivers, while _all_
the objects created by device drivers belong somewhere else?  Is that
somewhere else always under /sys/class/ (or /sys/block/)?  And where in
the documentation is this spelled out?

> So no, there is not always a 1:1 mapping from classes to devices, that
> is why the driver model does not enforce such a mapping at all.  You can
> have multiple "struct class_device" structures that point to the same
> "struct device" or no "struct device" at all.
> 
> Hope this helps to clear up the confusion that seems to be happening.

I'm still struggling... :-)

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

Here's another way to express some of my questions.

Consider that many of the buses in a computer system have interconnects 
that are sufficiently complicated to warrant their own driver.  For 
example, a SCSI adapter card can be regarded as an interconnect between 
a PCI bus and a SCSI bus.  A USB host controller is an interconnect 
between a PCI bus and a USB bus.  Each of these requires its own driver.

Should the interconnect's driver add a device under /sys/devices/, and if 
so, where?

Right now they don't.  For example, I have a SCSI card in my system.  It
shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
view of the card.  It also shows up as
/sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
the same card.  So there are two devices representing the same physical 
object, and neither of them is created by the aic7xxx driver.

The same thing happens with USB.  /sys/devices/pci0/0000:00:07.2/ and 
/sys/devices/pci0/0000:00:07.2/usb1/ are two views of the same host 
controller, created by the PCI and USB bus drivers.  There's no entry 
created by the host controller driver.

Now presumably this fits the driver model okay; I haven't seen any 
requirement that _every_ device/driver combination must have an entry 
under /sys/devices/.  Still, supposing the driver _did_ want to create its 
own object, where would it go?

Would the SCSI host adapter device show up as 
/sys/devices/pci0/0000:00:0d.0/aic7xxx/host1/ -- intermediate between the 
PCI and SCSI bus driver entries?  Should it appear as 
/sys/devices/pci0/0000:00:0d.0/host1/aic7xxx/ -- under the SCSI bus entry?  
Or should it not appear under /sys/devices/ at all but somewhere else 
instead, such as /sys/class/scsi_host/host1/aic7xxx/ ?

The same questions apply to the USB host controller example.

Bear in mind that these interconnects are not the same manner of device as
a disk or a mouse.  They're not things the user normally manipulates and
they aren't accessed through /dev/.  Does that mean that they don't
deserve to go under /sys/classes/ ?


To change the topic slightly, consider this small inconsistency in sysfs.  
The IDE bus driver creates entries for each IDE channel.  So for example, 
/sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
slave device on channel 1.  The SCSI bus driver, on the other hand, does 
not create intermediate levels in the hierarchy for channels or targets.  
So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
host level and the LUN level.

Is that the sort of decision that's left up to the bus driver author?  
Should there be any sort of enforced consistency, or doesn't it matter?

Still trying to grasp the fundamental principles at work here...

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> > Ok, have you _read_ the documentation on the driver model?  In it,
> > classes and devices are clearly spelled out as to what the differences
> > are, and what shows up where.
> 
> Yes, of course I've read it.  It's lacking a number of important details.

Hey, we've tried. I realize it's missing details, and though I know it's 
important to keep it updated, many other things take precedence. 

> For example, nowhere in devices.txt does it say that a device driver
> should not create attributes in the struct device's directory since that
> directory is owned by the bus driver.  (That's a very easy mistake to
> make; at first sight it appears to be the natural way for a driver to
> expose details of how it controls a device.)  In fact, nowhere in the
> documentation does it say that you shouldn't attach an attribute to an 
> object unless you own that object.  Maybe that seems obvious, but when a 
> driver is bound to a device can't it be said to "own" that device in some 
> sense?

It's "bound" to the device, but it does not own the object. 

Note that only recently have we realized the full importance of creating 
attributes _only_ for objects that you own. It's exposed bugs recently, 
and hasn't had a chance to make it in to the documentation. 

> Let me ask you this:  Given a device that doesn't fit clearly into any of 
> the existing classes, how would you decide whether or not to create a new 
> class for it?

If it does not fit into the existing classes, then there is probably a new 
class that needs to be created for it. While you're at it, please update 
the documentation and set a good example for the rest of us ;)

> Yes, but _which_ physical things correspond to devices?  And how should 
> the parent-child relationships be decided?
> 
> Consider a concrete example: a USB host controller.  Let's say that on my
> system /sys/devices/pci0/0000:00:07.2 is an OHCI HC.  That particular
> object is created by the PCI bus driver, and directly below it is
> /sys/devices/pci0/0000:00:07.2/usb1 -- what physical thing does that
> correspond to?  Is it the virtual root hub?  It's created by the USB core;
> where does the object created by the HC driver belong?
> 
> Or have I misunderstood, and was it intended from the start that _all_ the
> objects under /sys/devices/ should be created by bus drivers, while _all_
> the objects created by device drivers belong somewhere else?  

Yes.

> Is that
> somewhere else always under /sys/class/ (or /sys/block/)?  

Yes, /sys/class

> And where in
> the documentation is this spelled out?

It should be in the first sections of each driver model paper - The 
hierarchy under /sys/devices/ represents the physical hierarchy of the 
system itself. Each object represented in it is intended to map directly 
to a physical device. Physical devices are discovered and registered by 
bus drivers in the system. 


	-pat

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> Consider that many of the buses in a computer system have interconnects 
> that are sufficiently complicated to warrant their own driver.  For 
> example, a SCSI adapter card can be regarded as an interconnect between 
> a PCI bus and a SCSI bus.  A USB host controller is an interconnect 
> between a PCI bus and a USB bus.  Each of these requires its own driver.
> 
> Should the interconnect's driver add a device under /sys/devices/, and if 
> so, where?

No, it's already created for them. The USB controller you speak of is a 
PCI device. The driver is a PCI driver. There is already an object that 
represents the physical device in the hierarchy; you do not need to create 
a virtual one to represent it. 

> Right now they don't.  For example, I have a SCSI card in my system.  It
> shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
> view of the card.  It also shows up as
> /sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
> the same card.  So there are two devices representing the same physical 
> object, and neither of them is created by the aic7xxx driver.
> 
> The same thing happens with USB.  /sys/devices/pci0/0000:00:07.2/ and 
> /sys/devices/pci0/0000:00:07.2/usb1/ are two views of the same host 
> controller, created by the PCI and USB bus drivers.  There's no entry 
> created by the host controller driver.

SCSI copied USB in this respect. I've always been skeptical about the
representation, though Greg had good reason to initially do this. I wonder 
if that object could be moved over /sys/class/usb-host/ these days.. 

> Now presumably this fits the driver model okay; I haven't seen any 
> requirement that _every_ device/driver combination must have an entry 
> under /sys/devices/.  Still, supposing the driver _did_ want to create its 
> own object, where would it go?

Well, what does the object represent? A per-device driver-specific object? 
Is it really a separate object, or does it have a SCSI host object 
embedded in it? Usually, from what I've seen, if there is a per-device 
driver-specific object, it contains an embedded (class) object. This 
object is what the driver registers with the infrastructure for that 
device type.

That embedded structure should contain an embedded struct class_device, 
which is registered with the class (when the driver registers it's 
object). That gives it presence under /sys/class/whatever/

It also gives the driver an object that it owns, via embedding, which it 
is free to export attributes for.

> Bear in mind that these interconnects are not the same manner of device as
> a disk or a mouse.  They're not things the user normally manipulates and
> they aren't accessed through /dev/.  Does that mean that they don't
> deserve to go under /sys/classes/ ?

No, they should still go under /sys/class. 

> To change the topic slightly, consider this small inconsistency in sysfs.  
> The IDE bus driver creates entries for each IDE channel.  So for example, 
> /sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
> channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
> slave device on channel 1.  The SCSI bus driver, on the other hand, does 
> not create intermediate levels in the hierarchy for channels or targets.  
> So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
> for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
> host level and the LUN level.
> 
> Is that the sort of decision that's left up to the bus driver author?  
> Should there be any sort of enforced consistency, or doesn't it matter?

Yes. :) 

There should be consistency, but it is left up to the authors. We don't 
have the time to babysit each subsystem's sysfs exports. We anticipate a 
de facto standard to arise and everyone to conform to it, especially when 
users point out inconsistencies in them.. 


	-pat

Re: Flaw in the driver-model implementation of attributes

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> Right now they don't.  For example, I have a SCSI card in my system.  It
> shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
> view of the card.  It also shows up as
> /sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
> the same card.  So there are two devices representing the same physical 
> object, and neither of them is created by the aic7xxx driver.

Actually host1 is created by the aic7xxx driver through registration
with the SCSI subsystem. The aic7xxx allocates the structure, registers
it, unregisters it, and drops the ref on it when it is done with it.

The SCSI subsystems view of hosts that have registered with the subsystem
is through /class/scsi_host.

> To change the topic slightly, consider this small inconsistency in sysfs.  
> The IDE bus driver creates entries for each IDE channel.  So for example, 
> /sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
> channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
> slave device on channel 1.  The SCSI bus driver, on the other hand, does 
> not create intermediate levels in the hierarchy for channels or targets.  
> So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
> for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
> host level and the LUN level.
> 

Currently the scsi device in the SCSI subsystem is LUN.  While the
future may lead to having a having a target as a child of the host it
would be a large change for 2.5 / 2.6. 

> Is that the sort of decision that's left up to the bus driver author?  
> Should there be any sort of enforced consistency, or doesn't it matter?

Each transport / bus may have a unique nexus between a parent and child.
Interpretation of a name outside of the name space registering or
owning the name would lead to bad info (i.e.  interpretation of these
two child nodes using the same policy "pci0/0000:00:07.1"
"host1/1:0:5:0/")  Enforcing a overall consistency could lead to
creating pseudo hierarchies just to fit a consistency model.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Patrick Mochel wrote:

> SCSI copied USB in this respect. I've always been skeptical about the
> representation, though Greg had good reason to initially do this. I wonder 
> if that object could be moved over /sys/class/usb-host/ these days.. 

I wonder about the apparent proliferation of entries under /sys/class/.  
If people continue to add things like /sys/class/usb-storage/ right at the
top level, won't it become rather unwieldy?  What would be a good place to
put something like that?

----

> Well, what does the object represent? A per-device driver-specific object? 
> Is it really a separate object, or does it have a SCSI host object 
> embedded in it? Usually, from what I've seen, if there is a per-device 
> driver-specific object, it contains an embedded (class) object. This 
> object is what the driver registers with the infrastructure for that 
> device type.
> 
> That embedded structure should contain an embedded struct class_device, 
> which is registered with the class (when the driver registers it's 
> object). That gives it presence under /sys/class/whatever/
> 
> It also gives the driver an object that it owns, via embedding, which it 
> is free to export attributes for.

On Thu, 19 Jun 2003, Mike Anderson wrote:

> Actually host1 is created by the aic7xxx driver through registration
> with the SCSI subsystem. The aic7xxx allocates the structure, registers
> it, unregisters it, and drops the ref on it when it is done with it.

[Two different replies to related questions of mine.]

This makes sense.  But it doesn't match my experience working with the 
SCSI core.  (That experience is connected with usb-storage, not aic7xxx.)

When a host driver registers a newly-detected host adapter with the core,
it doesn't provide a structure representing that adapter.  It provides a
Scsi Host Template, but that's not the same thing.  Instead, the core
creates a struct Scsi_Host on the driver's behalf.  Embedded in it is a
struct device, which is registered under /sys/devices/, and a struct
class_device, which is registered under /sys/class/scsi_host/.  The driver
doesn't own these structures; the SCSI core does.

Furthermore, there is no release() for the embedded struct class_device,
so there's no way for the driver to know when it goes away.  (That lack is
liable to cause a problem at some point.  I noticed that until very
recently there was no release() method for struct class_device at all.)  
There _is_ a release notification for the overall struct Scsi_Host in
scsi_remove_host(), but it occurs at the wrong time.  It is called right
after the embedded struct device has been deregistered, not when its
reference count drops to 0.

So what Mike says the SCSI core does, and what Pat says it ought to do, 
isn't what actually happens.  Is this simply an indication that the work 
of completing the SCSI code to the driver model isn't yet complete?  It 
seems as though a very large change would be needed to make things work as 
Pat described.

In the meantime, where should I register my class device for the 
usb-storage driver?  Should it go in 
/sys/class/scsi_host/host1/usb-storage/, under the existing class device?  
That might be simplest.  Or should there be a new class just for it?

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Patrick Mochel wrote:

> > > Ok, have you _read_ the documentation on the driver model?  In it,
> > > classes and devices are clearly spelled out as to what the differences
> > > are, and what shows up where.
> > 
> > Yes, of course I've read it.  It's lacking a number of important details.
> 
> Hey, we've tried. I realize it's missing details, and though I know it's 
> important to keep it updated, many other things take precedence. 

Believe me, I understand how hard it is to keep documentation in sync with 
a developing system.

> > Let me ask you this:  Given a device that doesn't fit clearly into any of 
> > the existing classes, how would you decide whether or not to create a new 
> > class for it?
> 
> If it does not fit into the existing classes, then there is probably a new 
> class that needs to be created for it. While you're at it, please update 
> the documentation and set a good example for the rest of us ;)

I'll offer a deal.  When you and Greg have gotten the current set of 
updates into the documentation, let me know and I'll add in explanations 
of all the stuff that has puzzled me and been thrashed out in this thread.

At least I seem to be making progress.  Thanks for the help.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Thu, Jun 19, 2003 at 05:14:42PM -0400, Alan Stern wrote:
> 
> So what Mike says the SCSI core does, and what Pat says it ought to do, 
> isn't what actually happens.  Is this simply an indication that the work 
> of completing the SCSI code to the driver model isn't yet complete?  It 
> seems as though a very large change would be needed to make things work as 
> Pat described.

I think it's more of the matter that Mike is still changing the way the
SCSI core works with the class and driver model.  I suggest you take
this to the linux-scsi list to hash out the way scsi should and does
work in order to find the best solution for usb-storage, as I guess that
it also would be the best solution for all other SCSI host drivers.

> In the meantime, where should I register my class device for the 
> usb-storage driver?

Why not hold off until the scsi people are finished with their changes?
If after that, you _really_ need to be putting usb-storage only
attributes in the sysfs tree somewhere, we can talk again.

thanks,

greg "scsi makes my head hurt" k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Greg KH wrote:

> I think it's more of the matter that Mike is still changing the way the
> SCSI core works with the class and driver model.  I suggest you take
> this to the linux-scsi list to hash out the way scsi should and does
> work in order to find the best solution for usb-storage, as I guess that
> it also would be the best solution for all other SCSI host drivers.

Okay.

> > In the meantime, where should I register my class device for the 
> > usb-storage driver?
> 
> Why not hold off until the scsi people are finished with their changes?
> If after that, you _really_ need to be putting usb-storage only
> attributes in the sysfs tree somewhere, we can talk again.

Sounds reasonable.  By the way, do you plan to create a 
/sys/class/usb_host/ class for the OHCI and ECHI drivers?

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Fri, Jun 20, 2003 at 10:22:21AM -0400, Alan Stern wrote:
> > Why not hold off until the scsi people are finished with their changes?
> > If after that, you _really_ need to be putting usb-storage only
> > attributes in the sysfs tree somewhere, we can talk again.
> 
> Sounds reasonable.  By the way, do you plan to create a 
> /sys/class/usb_host/ class for the OHCI and ECHI drivers?

Yes, and for the UHCI driver too :)

Unless someone else wants to send me a patch...

thanks,

greg k-h

Host drivers and conversion of SCSI to the driver model

[Alan Stern]

On Thu, 19 Jun 2003, Greg KH wrote:

> I think it's more of the matter that Mike is still changing the way the
> SCSI core works with the class and driver model.  I suggest you take
> this to the linux-scsi list to hash out the way scsi should and does
> work in order to find the best solution for usb-storage, as I guess that
> it also would be the best solution for all other SCSI host drivers.

Mike:

What does the future for this progression look like?  Will there be some
standard place under /sys/class/scsi??? where a host driver like
usb-storage can register its class device?

And while I'm at it, let me remind you that the class_dev element of
struct Scsi_Host needs to have a release() method.  Probably the easiest 
way to make it work right is to do a "get_device(&shost->gendev)" when 
shost->class_dev is registered and have the release routine do 
"put_device(&shost->gendev)".

Alan Stern

Re: Host drivers and conversion of SCSI to the driver model

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> On Thu, 19 Jun 2003, Greg KH wrote:
> 
> > I think it's more of the matter that Mike is still changing the way the
> > SCSI core works with the class and driver model.  I suggest you take
> > this to the linux-scsi list to hash out the way scsi should and does
> > work in order to find the best solution for usb-storage, as I guess that
> > it also would be the best solution for all other SCSI host drivers.
> 
> Mike:
> 
> What does the future for this progression look like?  Will there be some
> standard place under /sys/class/scsi??? where a host driver like
> usb-storage can register its class device?

Did you need your own class device or can you use the shost_attrs
capability that James added?

> 
> And while I'm at it, let me remind you that the class_dev element of
> struct Scsi_Host needs to have a release() method.  Probably the easiest 
> way to make it work right is to do a "get_device(&shost->gendev)" when 
> shost->class_dev is registered and have the release routine do 
> "put_device(&shost->gendev)".
> 

Yes I will add a class release function and the order you
describe sounds the easiest to make it work.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Host drivers and conversion of SCSI to the driver model

[Alan Stern]

On Fri, 20 Jun 2003, Mike Anderson wrote:

> Alan Stern [stern@rowland.harvard.edu] wrote:
> > 
> > Mike:
> > 
> > What does the future for this progression look like?  Will there be some
> > standard place under /sys/class/scsi??? where a host driver like
> > usb-storage can register its class device?
> 
> Did you need your own class device or can you use the shost_attrs
> capability that James added?

The shost_attrs stuff looks fine, expect for two points.

	1. The scsi_sysfs_modify_shost_attribute() and 
scsi_sysfs_modify_sdev_attribute() functions appear to be written a bit 
carelessly.  Below is a patch that: permits modification of the first 
attribute in the list, allocates a new list with entries having the 
correct size, copies the correct number of entries from the old list, and 
wraps excessively long source lines.

	2. More importantly, the current organization of the code has a
serious problem.  The SCSI core does not modify the host driver when the
reference count for either shost->class_dev or shost->host_gendev drops to
0.  Without knowing that, it is unsafe for the driver ever to deallocate a
private host data structure, since a user process may continue to hold a 
reference to an open attribute file indefinitely, even after 
scsi_unregister() has returned.

Alan Stern


===== scsi_sysfs.c 1.12 vs edited =====
--- 1.12/drivers/scsi/scsi_sysfs.c	Mon Jun  9 13:37:14 2003
+++ edited/drivers/scsi/scsi_sysfs.c	Mon Jun 23 10:43:49 2003
@@ -388,30 +388,36 @@
  *
  * returns zero if successful or error if not
  **/
-int scsi_sysfs_modify_shost_attribute(struct class_device_attribute ***class_attrs,
-				      struct class_device_attribute *attr)
+int scsi_sysfs_modify_shost_attribute(
+			struct class_device_attribute ***class_attrs,
+			struct class_device_attribute *attr)
 {
-	int modify = 0;
+	int modify = -1;
 	int num_attrs;
 
 	if(*class_attrs == NULL)
 		*class_attrs = scsi_sysfs_shost_attrs;
 
 	for(num_attrs=0; (*class_attrs)[num_attrs] != NULL; num_attrs++)
-		if(strcmp((*class_attrs)[num_attrs]->attr.name, attr->attr.name) == 0)
+		if(strcmp((*class_attrs)[num_attrs]->attr.name,
+				attr->attr.name) == 0)
 			modify = num_attrs;
 
-	if(*class_attrs == scsi_sysfs_shost_attrs || !modify) {
+	if(*class_attrs == scsi_sysfs_shost_attrs || modify < 0) {
 		/* note: need space for null at the end as well */
-		struct class_device_attribute **tmp_attrs = kmalloc(sizeof(struct class_device_attribute)*(num_attrs + (modify ? 1 : 2)), GFP_KERNEL);
+		struct class_device_attribute **tmp_attrs =
+				kmalloc(sizeof(*tmp_attrs) *
+					  (num_attrs + (modify >= 0 ? 1 : 2)),
+					GFP_KERNEL);
 		if(tmp_attrs == NULL)
 			return -ENOMEM;
-		memcpy(tmp_attrs, *class_attrs, sizeof(struct class_device_attribute)*num_attrs);
+		memcpy(tmp_attrs, *class_attrs, sizeof(*tmp_attrs) *
+				(num_attrs + 1));
 		if(*class_attrs != scsi_sysfs_shost_attrs)
 			kfree(*class_attrs);
 		*class_attrs = tmp_attrs;
 	}
-	if(modify) {
+	if(modify >= 0) {
 		/* spare the caller from having to copy things it's
 		 * not interested in */
 		struct class_device_attribute *old_attr =
@@ -444,27 +450,32 @@
 int scsi_sysfs_modify_sdev_attribute(struct device_attribute ***dev_attrs,
 				     struct device_attribute *attr)
 {
-	int modify = 0;
+	int modify = -1;
 	int num_attrs;
 
 	if(*dev_attrs == NULL)
 		*dev_attrs = scsi_sysfs_sdev_attrs;
 
 	for(num_attrs=0; (*dev_attrs)[num_attrs] != NULL; num_attrs++)
-		if(strcmp((*dev_attrs)[num_attrs]->attr.name, attr->attr.name) == 0)
+		if(strcmp((*dev_attrs)[num_attrs]->attr.name,
+				attr->attr.name) == 0)
 			modify = num_attrs;
 
-	if(*dev_attrs == scsi_sysfs_sdev_attrs || !modify) {
+	if(*dev_attrs == scsi_sysfs_sdev_attrs || modify < 0) {
 		/* note: need space for null at the end as well */
-		struct device_attribute **tmp_attrs = kmalloc(sizeof(struct device_attribute)*(num_attrs + (modify ? 1 : 2)), GFP_KERNEL);
+		struct device_attribute **tmp_attrs =
+				kmalloc(sizeof(*tmp_attrs) *
+					  (num_attrs + (modify >= 0 ? 1 : 2)),
+					GFP_KERNEL);
 		if(tmp_attrs == NULL)
 			return -ENOMEM;
-		memcpy(tmp_attrs, *dev_attrs, sizeof(struct device_attribute)*num_attrs);
+		memcpy(tmp_attrs, *dev_attrs, sizeof(*tmp_attrs) *
+				(num_attrs + 1));
 		if(*dev_attrs != scsi_sysfs_sdev_attrs)
 			kfree(*dev_attrs);
 		*dev_attrs = tmp_attrs;
 	}
-	if(modify) {
+	if(modify >= 0) {
 		/* spare the caller from having to copy things it's
 		 * not interested in */
 		struct device_attribute *old_attr =

Re: Host drivers and conversion of SCSI to the driver model

[Christoph Hellwig]

On Mon, Jun 23, 2003 at 10:57:22AM -0400, Alan Stern wrote:
> 	2. More importantly, the current organization of the code has a
> serious problem.  The SCSI core does not modify the host driver when the
> reference count for either shost->class_dev or shost->host_gendev drops to
> 0.

The host driver shouldn't know.

> Without knowing that, it is unsafe for the driver ever to deallocate a
> private host data structure, since a user process may continue to hold a 
> reference to an open attribute file indefinitely, even after 
> scsi_unregister() has returned.

Right,  And this issue has been discussed on netdev for a long time.
The solution that they came up with and I plan to implement in scsi
aswell as soon as their code actually shows up is to have a mark this
device gone function that makes the struct device never call back
into driver code.

If you think a bit about this this is what's really needed - even with
your notification you'd still have module unload races.  With the above
fix you can just free your private data in your ->remove method
(or wherever you want) after you marked the struct device / class_device
gone and be sure it'll never be used again.

Of course for this to fully work with scsi we need a proper
scsi_set_host_offline..

Re: Host drivers and conversion of SCSI to the driver model

[Alan Stern]

On Fri, 27 Jun 2003, Christoph Hellwig wrote:

> On Mon, Jun 23, 2003 at 10:57:22AM -0400, Alan Stern wrote:
> > 	2. More importantly, the current organization of the code has a
> > serious problem.  The SCSI core does not modify the host driver when the
> > reference count for either shost->class_dev or shost->host_gendev drops to
> > 0.
> 
> The host driver shouldn't know.
> 
> > Without knowing that, it is unsafe for the driver ever to deallocate a
> > private host data structure, since a user process may continue to hold a 
> > reference to an open attribute file indefinitely, even after 
> > scsi_unregister() has returned.
> 
> Right,  And this issue has been discussed on netdev for a long time.
> The solution that they came up with and I plan to implement in scsi
> aswell as soon as their code actually shows up is to have a mark this
> device gone function that makes the struct device never call back
> into driver code.
> 
> If you think a bit about this this is what's really needed - even with
> your notification you'd still have module unload races.  With the above
> fix you can just free your private data in your ->remove method
> (or wherever you want) after you marked the struct device / class_device
> gone and be sure it'll never be used again.
> 
> Of course for this to fully work with scsi we need a proper
> scsi_set_host_offline..

I don't see how you can make that work without modifying parts of the 
driver model core.  After all, calls to a class_device attribute's show() 
and store() methods don't go through the device driver or the bus driver; 
they are demultiplexed within drivers/base/class.c.

I do agree that something like your proposal would solve the problem.  I
made a similar proposal to Pat Mochel a couple of weeks ago.  My
suggestion was to arrange some sort of mutex on the sysfs file's dentry,
so that calling the show() and store() methods would be atomic with
respect to unlinking the file.  That way the driver could be certain that
after class_device_remove_file() had returned, no one would ever call the
show() or store() functions.  My feeling is that this would be more
generally useful, not just within the SCSI realm.

Alan Stern

Re: Host drivers and conversion of SCSI to the driver model

[Christoph Hellwig]

On Fri, Jun 27, 2003 at 01:56:59PM -0400, Alan Stern wrote:
> I don't see how you can make that work without modifying parts of the 
> driver model core.  After all, calls to a class_device attribute's show() 
> and store() methods don't go through the device driver or the bus driver; 
> they are demultiplexed within drivers/base/class.c.

Right, the intention is to change the device model cope.  See the dicussion
about netdev refcounting on linux-netdev a while ago.  

Re: Host drivers and conversion of SCSI to the driver model

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> On Fri, Jun 27, 2003 at 01:56:59PM -0400, Alan Stern wrote:
> > I don't see how you can make that work without modifying parts of the 
> > driver model core.  After all, calls to a class_device attribute's show() 
> > and store() methods don't go through the device driver or the bus driver; 
> > they are demultiplexed within drivers/base/class.c.
> 
> Right, the intention is to change the device model cope.  See the dicussion
> about netdev refcounting on linux-netdev a while ago.  
> 

I looked over linux-netdev and can not pick out a thread on this
subject. Could you point to the thread or at least the subject of the
thread. I was wanting to see the consensus of the core change.

I have a set of patch running right now on top of scsi-misc-2.5
(1:a:stern_present 2:a:shost_present 3:a:shost_state 4:a:sdev_state)
that fix some release issues and adds a state attribute to shost and
sdev. The patch set does still have the show / store issue, but does
seem to cleanup better post a surprise removal call to scsi_remove_host.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Host drivers and conversion of SCSI to the driver model

[Christoph Hellwig]

On Fri, Jun 27, 2003 at 12:23:21PM -0700, Mike Anderson wrote:
> I looked over linux-netdev and can not pick out a thread on this
> subject. Could you point to the thread or at least the subject of the
> thread.

It's spread over lots of thread.  Two big relevant threads I could find
easily are:

dev->destructor

and

[PATCH 2.5.70] Add release_netdev -- hook for sysfs/net device cleanup


> I was wanting to see the consensus of the core change.

The folks doing the netdev refocunting changes (davem, viro, jgarzik, sch)
seem to agree on it :)  But IMHO it's the only way to go anyway - every
subsystem with sysfs exposure will need something similar.

> I have a set of patch running right now on top of scsi-misc-2.5
> (1:a:stern_present 2:a:shost_present 3:a:shost_state 4:a:sdev_state)
> that fix some release issues and adds a state attribute to shost and
> sdev. The patch set does still have the show / store issue, but does
> seem to cleanup better post a surprise removal call to scsi_remove_host.

Cool.

Re: Host drivers and conversion of SCSI to the driver model

[Jeff Garzik]

The future netdev api could be summarized as

	alloc_<foo>dev -- allocate structure, init
	register_netdev[ice] -- register
	unregister_netdev[ice] -- unregister
	release_netdev -- unref

We actually have this now, once you do s/kfree(dev)/release_netdev(dev)/

	Jeff

Re: Host drivers and conversion of SCSI to the driver model

[Christoph Hellwig]

On Sat, Jun 28, 2003 at 11:08:36AM -0400, Jeff Garzik wrote:
> The future netdev api could be summarized as
> 
> 	alloc_<foo>dev -- allocate structure, init
> 	register_netdev[ice] -- register
> 	unregister_netdev[ice] -- unregister

Yeah, this maps nicely to

	scsi_host_alloc, scsi_add_host, scsi_remove_host

> 	release_netdev -- unref

This is the final scsi_host_put.

> We actually have this now, once you do s/kfree(dev)/release_netdev(dev)/

But what's more interesting are the driver model core changes to mark
a struct (class_)device dead so it won't ever call back into the driver.

IIRC DaveM's words correctly this was the central part of the netdev
refcounting changes.

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Fri, Jun 20, 2003 at 11:32:51AM -0700, Greg KH wrote:
> On Fri, Jun 20, 2003 at 10:22:21AM -0400, Alan Stern wrote:
> > > Why not hold off until the scsi people are finished with their changes?
> > > If after that, you _really_ need to be putting usb-storage only
> > > attributes in the sysfs tree somewhere, we can talk again.
> > 
> > Sounds reasonable.  By the way, do you plan to create a 
> > /sys/class/usb_host/ class for the OHCI and ECHI drivers?
> 
> Yes, and for the UHCI driver too :)
> 
> Unless someone else wants to send me a patch...

This is now in 2.5.74.  Combined with the module reference patch for
attributes that I just posted I now think that the problems discussed in
this thread are solved, right?

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 2 Jul 2003, Greg KH wrote:

> This is now in 2.5.74.  Combined with the module reference patch for
> attributes that I just posted I now think that the problems discussed in
> this thread are solved, right?

Yes.  Any remaining difficulties are more-or-less internal SCSI matters.
I'm glad that, annoying though it may have been for some people, this
conversation had some positive results.

And by the way, I was serious about that offer of contributing to the 
documentation once you and Pat have added your current updates.

Alan Stern

Re: Host drivers and conversion of SCSI to the driver model

[Alan Stern]

On Fri, 27 Jun 2003, Christoph Hellwig wrote:

> Right,  And this issue has been discussed on netdev for a long time.
> The solution that they came up with and I plan to implement in scsi
> aswell as soon as their code actually shows up is to have a mark this
> device gone function that makes the struct device never call back
> into driver code.
> 
> If you think a bit about this this is what's really needed - even with
> your notification you'd still have module unload races.  With the above
> fix you can just free your private data in your ->remove method
> (or wherever you want) after you marked the struct device / class_device
> gone and be sure it'll never be used again.

How will this interact with the module reference-counting just added to 
sysfs by Greg K-H?  Maybe with that new feature, none of this will be 
needed.

Alan Stern

SCSI documentation in scsi_mid_low_api.txt

[Alan Stern]

Mike:

Here's a suggestion to improve the documentation in scsi_mid_low_api.txt.  
In the sections that list the API functions and give their descriptions, 
add a line mentioning whether the function might block.  Right now there's 
no easy way to tell which ones can be called in interrupt context or with 
a spinlock held and which ones can't.

Alan Stern

RE: aic7xxx driver schedules() while holding spinlock

[Tony Battersby]

While on the subject, a while ago I discovered that sym53c8xx_2 in 2.4.x
holds a spinlock (io_request_lock) while calling ioremap() and
iounmap(), which can lead to deadlock on certain hardware.  If anyone is
doing a driver audit, it might be worth checking for this in other
drivers too.

Anthony J. Battersby
Cybernetics

scsi_forget_host() and scsi_remove_device()

[Alan Stern]

There's a real problem about the way scsi_forget_host() calls
scsi_remove_device() for each device on the host's bus.  The problem is
that scsi_remove_device() unregisters the device in sysfs, which unbinds
the device's driver.  This happens immediately, without waiting for the
reference count to be 0.  So if the device is open (mounted, for example)  
when the host is unplugged, the filesystem will have a dangling reference
to the unbound driver.  Of course this will most likely cause a segfault
when the user attempts to unmount the device.

I don't know what the right way is to attack this problem, or what you're
planning to do about it.  One approach would be somehow to prevent any new
references to the device from being created while waiting for all the
existing references to go away.  But that doesn't seem feasible.

How are you going to address this problem?

Alan Stern

Re: scsi_forget_host() and scsi_remove_device()

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> There's a real problem about the way scsi_forget_host() calls
> scsi_remove_device() for each device on the host's bus.  The problem is
> that scsi_remove_device() unregisters the device in sysfs, which unbinds
> the device's driver.  This happens immediately, without waiting for the
> reference count to be 0.  So if the device is open (mounted, for example)  
> when the host is unplugged, the filesystem will have a dangling reference
> to the unbound driver.  Of course this will most likely cause a segfault
> when the user attempts to unmount the device.
> 
> I don't know what the right way is to attack this problem, or what you're
> planning to do about it.  One approach would be somehow to prevent any new
> references to the device from being created while waiting for all the
> existing references to go away.  But that doesn't seem feasible.
> 
> How are you going to address this problem?
> 
> Alan Stern
> 

Yes, this is an issue. I became more of issue one we started using the
LDM driver probe / remove functions.

I am still cleaning up a patch set for hosts and devices so it is not
quite ready for review yet (I got pulled off on to some other issues for
a few days).

For scsi devices here is a quick snap shot of what I trying to test.
	scsi_device_register:
		calls device_initialize on sdev_driverfs_dev
		calls class_device_initialize on sdev_classdev
		calls device_add on sdev_driverfs_dev
		calls class_device_add on sdev_classdev
		calls get_device on sdev_driverfs_dev

	scsi_device_unregister:
		sets a state the scsi_device indicating "delete"
		calls class_device_unregister on sdev_classdev

	scsi_device_cls_release:
		calls put_device on sdev_driverfs_dev

	scsi_device_dev_release:
		calls device_del sdev_driverfs_dev (I have a hack in here to ensure
		device_add was previously called).	
		calls scsi_free_sdev

	scsi_device_get:
		if device state says get ok
			calls get_device on sdev_driverfs_dev
	(I still have access_count on try_module_get in the function).

	scsi_device_put:
		calls put_device on sdev_driverfs_dev
	(I still have access_count on try_module_put in the function).

The gets are only stopped for callers of scsi_device_get. If a
get_device is called from outside the subsystem I cannot stop this
though it would not cause bad things to happen just a longer time to
cleanup. A core flag would most likely be needed to stop this from
happening.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: scsi_forget_host() and scsi_remove_device()

[Alan Stern]

On Thu, 3 Jul 2003, Mike Anderson wrote:

> Alan Stern [stern@rowland.harvard.edu] wrote:
> > There's a real problem about the way scsi_forget_host() calls
> > scsi_remove_device() for each device on the host's bus.  The problem is
> > that scsi_remove_device() unregisters the device in sysfs, which unbinds
> > the device's driver.  This happens immediately, without waiting for the
> > reference count to be 0.  So if the device is open (mounted, for example)  
> > when the host is unplugged, the filesystem will have a dangling reference
> > to the unbound driver.  Of course this will most likely cause a segfault
> > when the user attempts to unmount the device.
> > 
> > I don't know what the right way is to attack this problem, or what you're
> > planning to do about it.  One approach would be somehow to prevent any new
> > references to the device from being created while waiting for all the
> > existing references to go away.  But that doesn't seem feasible.
> > 
> > How are you going to address this problem?
> > 
> > Alan Stern
> > 
> 
> Yes, this is an issue. I became more of issue one we started using the
> LDM driver probe / remove functions.
> 
> I am still cleaning up a patch set for hosts and devices so it is not
> quite ready for review yet (I got pulled off on to some other issues for
> a few days).

After thinking it through a little harder, it was clear that the problem I
saw was actually a problem with the driver (in this case, the cdrom driver
sr.c), not the SCSI core.  I should have realized this right away, since
exactly the same thing happens with USB device drivers.

The driver needs to keep track of whether the device is currently open for 
any process.  When a detach occurs, the driver must retain its internal 
data structures until the device has been released, failing all system 
calls in the meantime.  In 2.5.74, sr.c doesn't do this.  I was able to 
provoke a nice segfault by mounting a USB cdrom, unplugging the drive, and 
then unmounting it.  I haven't yet checked whether sd.c, st.c, and sg.c 
are subject to this same problem.

There's another similar problem common to all hot-unpluggable devices: a
race between open and detach.  The only way to resolve it is to use a
static driver-wide mutex.  Again, I don't think sr.c does this.

I'll work on a fix for sr.c this weekend and run it by you.

Alan Stern

Re: scsi_forget_host() and scsi_remove_device()

[Alan Stern]

On Thu, 3 Jul 2003, Mike Anderson wrote:

> I am still cleaning up a patch set for hosts and devices so it is not
> quite ready for review yet (I got pulled off on to some other issues for
> a few days).
> 
> For scsi devices here is a quick snap shot of what I trying to test.
> 	scsi_device_register:
> 		calls device_initialize on sdev_driverfs_dev
> 		calls class_device_initialize on sdev_classdev
> 		calls device_add on sdev_driverfs_dev
> 		calls class_device_add on sdev_classdev
> 		calls get_device on sdev_driverfs_dev
> 
> 	scsi_device_unregister:
> 		sets a state the scsi_device indicating "delete"
> 		calls class_device_unregister on sdev_classdev
> 
> 	scsi_device_cls_release:
> 		calls put_device on sdev_driverfs_dev
> 
> 	scsi_device_dev_release:
> 		calls device_del sdev_driverfs_dev (I have a hack in here to ensure
> 		device_add was previously called).	
> 		calls scsi_free_sdev
> 
> 	scsi_device_get:
> 		if device state says get ok
> 			calls get_device on sdev_driverfs_dev
> 	(I still have access_count on try_module_get in the function).
> 
> 	scsi_device_put:
> 		calls put_device on sdev_driverfs_dev
> 	(I still have access_count on try_module_put in the function).

Okay, it looks like maybe I'm running up against problems from the areas
that you haven't had time to fix yet.

What I'm seeing is that the host is gone and the device is marked offline, 
but the device structure is still present because sr.c hasn't yet done its 
final scsi_device_put(), since a user process still has the device opened.  

When the user process closes the file, sr.c tries to unlock the door by
calling scsi_set_medium_removal().  This filters through
ioctl_internal_command(), scsi_wait_req(), scsi_do_req(),
scsi_request_fn(), scsi_prep_fn(), scsi_get_command(), and
__scsi_get_command() -- all without noticing that the device is offline!
Finally a segmentation violation occurs when __scsi_get_command() tries to
access dev->host->cmd_pool->gfp_mask.  Not surprising since the host
structure is gone.

[By the way, even to get this far I had to change scsi_device_get() and 
scsi_device_put() to make them do get_device(&sdev->sdev_driverfs_dev) and 
put_device(&sdev->sdev_driverfs_dev).  Without that, the scsi_device's 
refcount would go to 0 and the structure would be deallocated, even though 
sr.c still had outstanding references via scsi_device_get().]

My feeling is that when the device is offline, it should still be okay 
for the higher layers (sr, sd, etc.) to submit SCSI requests.  The check 
for whether the device is still available should be centralized in the 
midlayer instead of spread all over the place.  Such requests should 
simply fail with -ENODEV or something similar.

Below are the changes I made to sr.h and sr.c.

Alan Stern


--- 2.5.74-drivers/scsi/sr.h.orig	Mon Jun 16 15:28:36 2003
+++ 2.5.74-drivers/scsi/sr.h	Fri Jul  4 14:11:29 2003
@@ -35,6 +35,8 @@
 	unsigned xa_flag:1;	/* CD has XA sectors ? */
 	unsigned readcd_known:1;	/* drive supports READ_CD (0xbe) */
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
+	unsigned gone:1;	/* device has been hot-unplugged */
+	unsigned int use_count;	/* number of current users */
 	struct cdrom_device_info cdi;
 	struct gendisk *disk;
 } Scsi_CD;
--- 2.5.74-drivers/scsi/sr.c.orig	Wed Jul  2 11:42:54 2003
+++ 2.5.74-drivers/scsi/sr.c	Fri Jul  4 14:14:23 2003
@@ -86,6 +86,9 @@
 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
 static spinlock_t sr_index_lock = SPIN_LOCK_UNLOCKED;
 
+/* prevent races between sr_block_open() and sr_remove() */
+static DECLARE_MUTEX (sr_open_remove_sem);
+
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
 
@@ -386,14 +389,42 @@
 
 static int sr_block_open(struct inode *inode, struct file *file)
 {
-	struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
-	return cdrom_open(&cd->cdi, inode, file);
+	struct scsi_cd *cd;
+	int rc;
+
+	down(&sr_open_remove_sem);
+	if (!inode->i_bdev->bd_disk->private_data)
+		rc = -ENODEV;
+	else {
+		cd = scsi_cd(inode->i_bdev->bd_disk);
+		if (cd->gone)
+			rc = -ENODEV;
+		else {
+			++cd->use_count;
+			up(&sr_open_remove_sem);
+			rc = cdrom_open(&cd->cdi, inode, file);
+			down(&sr_open_remove_sem);
+			if (rc) {
+				if (--cd->use_count == 0 && cd->gone)
+					kfree(cd);
+			}
+		}
+	}
+	up(&sr_open_remove_sem);
+	return rc;
 }
 
 static int sr_block_release(struct inode *inode, struct file *file)
 {
+	int rc;
 	struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
-	return cdrom_release(&cd->cdi, file);
+
+	rc = cdrom_release(&cd->cdi, file);
+	down(&sr_open_remove_sem);
+	if (--cd->use_count == 0 && cd->gone)
+		kfree(cd);
+	up(&sr_open_remove_sem);
+	return rc;
 }
 
 static int sr_block_ioctl(struct inode *inode, struct file *file, unsigned cmd,
@@ -775,7 +806,11 @@
 
 	put_disk(cd->disk);
 	unregister_cdrom(&cd->cdi);
-	kfree(cd);
+	down(&sr_open_remove_sem);
+	cd->gone = 1;
+	if (!cd->use_count)
+		kfree(cd);
+	up(&sr_open_remove_sem);
 
 	return 0;
 }

Re: scsi_forget_host() and scsi_remove_device()

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On Fri, Jul 04, 2003 at 03:36:55PM -0400, Alan Stern wrote:
> Finally a segmentation violation occurs when __scsi_get_command() tries to
> access dev->host->cmd_pool->gfp_mask.  Not surprising since the host
> structure is gone.

This could still be our (usb-storage) problem.

We're not supposed to release the host structures until after ->release is
called.  I think we're violating that right now.

Add a little bit of debugging (a stub ->release() function that printk's)
and see if the mid-layer really thinks that everything should have been
freed already.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

What, are you one of those Microsoft-bashing Linux freaks?
					-- Customer to Greg
User Friendly, 2/10/1999

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: scsi_forget_host() and scsi_remove_device()

[Alan Stern]

On Fri, 4 Jul 2003, Matthew Dharm wrote:

> On Fri, Jul 04, 2003 at 03:36:55PM -0400, Alan Stern wrote:
> > Finally a segmentation violation occurs when __scsi_get_command() tries to
> > access dev->host->cmd_pool->gfp_mask.  Not surprising since the host
> > structure is gone.
> 
> This could still be our (usb-storage) problem.
> 
> We're not supposed to release the host structures until after ->release is
> called.  I think we're violating that right now.

Although that used to be true, it's not true any longer.  The SCSI 
midlayer interface changed very recently, and ->release is no longer 
called for hot-pluggable hosts.  See 
Documentation/scsi/scsi_mid_low_api.txt in 2.5.74.

> Add a little bit of debugging (a stub ->release() function that printk's)
> and see if the mid-layer really thinks that everything should have been
> freed already.

Not necessary.  Furthermore, it has been true for a while that the host 
structure is not deallocated when the driver calls scsi_unregister() -- or 
now scsi_host_put() -- but rather when its reference count goes to 0.  
This is a good example of the sort of thing that Christoph Hellwig says 
the driver shouldn't even be aware of, it should be completely internal to 
the SCSI core.

In any case, this particular problem isn't a result of deallocating the 
host too early; it's a result of trying to access the host (and maybe 
other related characteristics) of a device that is offline.  Now, I'm not 
familiar with the plans for completing the integration of the SCSI 
subsystem with the driver model -- maybe the host _isn't_ supposed to be 
deallocated until all the device structures have been freed first.  In 
that case, the problem would be that the device is failing to maintain a 
reference to the host structure.  Regardless, it will be solved when the 
driver model integration is completed; I think it's definitely _not_ a 
failing of usb-storage.

Alan Stern

Re: scsi_forget_host() and scsi_remove_device()

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

On Sat, Jul 05, 2003 at 10:11:18AM -0400, Alan Stern wrote:
> On Fri, 4 Jul 2003, Matthew Dharm wrote:
> 
> > On Fri, Jul 04, 2003 at 03:36:55PM -0400, Alan Stern wrote:
> > > Finally a segmentation violation occurs when __scsi_get_command() tries to
> > > access dev->host->cmd_pool->gfp_mask.  Not surprising since the host
> > > structure is gone.
> > 
> > This could still be our (usb-storage) problem.
> > 
> > We're not supposed to release the host structures until after ->release is
> > called.  I think we're violating that right now.
> 
> Although that used to be true, it's not true any longer.  The SCSI 
> midlayer interface changed very recently, and ->release is no longer 
> called for hot-pluggable hosts.  See 
> Documentation/scsi/scsi_mid_low_api.txt in 2.5.74.

Oh.  Well, that's convenient then.  One less thing for me to worry about
for this weekend. :)

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

It's not that hard.  No matter what the problem is, tell the customer 
to reinstall Windows.
					-- Nurse
User Friendly, 3/22/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: Host drivers and conversion of SCSI to the driver model

[Christoph Hellwig]

On Thu, Jul 03, 2003 at 11:15:32AM -0400, Alan Stern wrote:
> > If you think a bit about this this is what's really needed - even with
> > your notification you'd still have module unload races.  With the above
> > fix you can just free your private data in your ->remove method
> > (or wherever you want) after you marked the struct device / class_device
> > gone and be sure it'll never be used again.
> 
> How will this interact with the module reference-counting just added to 
> sysfs by Greg K-H?  Maybe with that new feature, none of this will be 
> needed.

The module refounting helps with the attribute vs unload races but
nothing else.

Re: scsi_forget_host() and scsi_remove_device()

[Christoph Hellwig]

On Thu, Jul 03, 2003 at 05:02:22PM -0400, Alan Stern wrote:
> There's a real problem about the way scsi_forget_host() calls
> scsi_remove_device() for each device on the host's bus.  The problem is
> that scsi_remove_device() unregisters the device in sysfs, which unbinds
> the device's driver.  This happens immediately, without waiting for the
> reference count to be 0.  So if the device is open (mounted, for example)  
> when the host is unplugged, the filesystem will have a dangling reference
> to the unbound driver.  Of course this will most likely cause a segfault
> when the user attempts to unmount the device.

There's two issues in this area:

b) after scsi_remove_device the upper driver still holds a reference
   to the struct scsi_device but not the host, so references to
   sdev->host barf up.  This needs fixing by holding a reference
a)  struct device/scsi_device misses a proper state bits that sais
   "I'm gone now".  Mike is implementing this for scsi now but this
   really needs to be handled at the driver model level as other subsystems
   have the same problems.

I'm waiting for Mike to post his patches and will attack a) after that.
In fact it's quite easy and I could just do it now :)

Re: scsi_forget_host() and scsi_remove_device()

[Christoph Hellwig]

On Thu, Jul 03, 2003 at 03:19:40PM -0700, Mike Anderson wrote:
> Yes, this is an issue. I became more of issue one we started using the
> LDM driver probe / remove functions.
> 
> I am still cleaning up a patch set for hosts and devices so it is not
> quite ready for review yet (I got pulled off on to some other issues for
> a few days).
> 
> For scsi devices here is a quick snap shot of what I trying to test.
> 	scsi_device_register:
> 		calls device_initialize on sdev_driverfs_dev
> 		calls class_device_initialize on sdev_classdev
> 		calls device_add on sdev_driverfs_dev
> 		calls class_device_add on sdev_classdev
> 		calls get_device on sdev_driverfs_dev
> 
> 	scsi_device_unregister:
> 		sets a state the scsi_device indicating "delete"
> 		calls class_device_unregister on sdev_classdev
> 
> 	scsi_device_cls_release:
> 		calls put_device on sdev_driverfs_dev

Hmm, where calling the put_device in the class device release func?

To be honest I'm everything but happy with the class_device / device
double-refcounting but it looks we can work around core issues in
scsi :P

PATCH: (as54) Fix hot-unplugging for sr.c

[Alan Stern]

Mike:

This is the final version of my patch to fix hot-unplugging for the SCSI 
cdrom driver.  It prevents the private data structure from being 
deallocated until all references to it have been dropped, and it prevents 
races between sr_block_open() and sr_remove().

The patch does assume, however, that it is legal to submit requests (via 
scsi_ioctl, for example) for a device that is marked offline.

Similar changes may be needed for the sd, st, and sg drivers; I haven't 
looked at them.

Alan Stern


--- usb-2.5/drivers/scsi/sr.h.orig	Mon Jun 16 15:28:36 2003
+++ usb-2.5/drivers/scsi/sr.h	Fri Jul  4 14:11:29 2003
@@ -35,6 +35,8 @@
 	unsigned xa_flag:1;	/* CD has XA sectors ? */
 	unsigned readcd_known:1;	/* drive supports READ_CD (0xbe) */
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
+	unsigned gone:1;	/* device has been hot-unplugged */
+	unsigned int use_count;	/* number of current users */
 	struct cdrom_device_info cdi;
 	struct gendisk *disk;
 } Scsi_CD;
--- usb-2.5/drivers/scsi/sr.c.orig	Wed Jul  2 11:42:54 2003
+++ usb-2.5/drivers/scsi/sr.c	Sun Jul  6 15:19:40 2003
@@ -86,6 +86,9 @@
 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
 static spinlock_t sr_index_lock = SPIN_LOCK_UNLOCKED;
 
+/* prevent races between sr_block_open() and sr_remove() */
+static DECLARE_MUTEX (sr_open_remove_sem);
+
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
 
@@ -166,7 +169,7 @@
  
 static inline struct scsi_cd *scsi_cd(struct gendisk *disk)
 {
-	return container_of(disk->private_data, struct scsi_cd, driver);
+	return disk->private_data;
 }
 
 /*
@@ -387,13 +390,40 @@
 static int sr_block_open(struct inode *inode, struct file *file)
 {
 	struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
-	return cdrom_open(&cd->cdi, inode, file);
+	int rc;
+
+	down(&sr_open_remove_sem);
+	if (!cd || cd->gone)
+		rc = -ENODEV;
+	else {
+		++cd->use_count;
+		up(&sr_open_remove_sem);
+		rc = cdrom_open(&cd->cdi, inode, file);
+		down(&sr_open_remove_sem);
+		if (rc) {
+			if (--cd->use_count == 0 && cd->gone) {
+				cd->disk->private_data = NULL;
+				kfree(cd);
+			}
+		}
+	}
+	up(&sr_open_remove_sem);
+	return rc;
 }
 
 static int sr_block_release(struct inode *inode, struct file *file)
 {
 	struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
-	return cdrom_release(&cd->cdi, file);
+	int rc;
+
+	rc = cdrom_release(&cd->cdi, file);
+	down(&sr_open_remove_sem);
+	if (--cd->use_count == 0 && cd->gone) {
+		cd->disk->private_data = NULL;
+		kfree(cd);
+	}
+	up(&sr_open_remove_sem);
+	return rc;
 }
 
 static int sr_block_ioctl(struct inode *inode, struct file *file, unsigned cmd,
@@ -417,6 +447,9 @@
 static int sr_block_media_changed(struct gendisk *disk)
 {
 	struct scsi_cd *cd = scsi_cd(disk);
+
+	if (!cd)
+		return 1;
 	return cdrom_media_changed(&cd->cdi);
 }
 
@@ -511,7 +544,6 @@
 	cd->device = sdev;
 	cd->disk = disk;
 	cd->driver = &sr_template;
-	cd->disk = disk;
 	cd->capacity = 0x1fffff;
 	cd->needs_sector_size = 1;
 	cd->device->changed = 1;	/* force recheck CD type */
@@ -536,7 +568,7 @@
 	disk->driverfs_dev = &sdev->sdev_driverfs_dev;
 	register_cdrom(&cd->cdi);
 	set_capacity(disk, cd->capacity);
-	disk->private_data = &cd->driver;
+	disk->private_data = cd;
 	disk->queue = sdev->request_queue;
 
 	dev_set_drvdata(dev, cd);
@@ -766,6 +798,7 @@
 static int sr_remove(struct device *dev)
 {
 	struct scsi_cd *cd = dev_get_drvdata(dev);
+	struct gendisk *disk = cd->disk;
 
 	del_gendisk(cd->disk);
 
@@ -773,10 +806,17 @@
 	clear_bit(cd->disk->first_minor, sr_index_bits);
 	spin_unlock(&sr_index_lock);
 
-	put_disk(cd->disk);
 	unregister_cdrom(&cd->cdi);
-	kfree(cd);
 
+	down(&sr_open_remove_sem);
+	cd->gone = 1;
+	if (cd->use_count == 0) {
+		cd->disk->private_data = NULL;
+		kfree(cd);
+	}
+	up(&sr_open_remove_sem);
+
+	put_disk(disk);
 	return 0;
 }
 

Re: scsi_forget_host() and scsi_remove_device()

[Alan Stern]

On Sun, 6 Jul 2003, Christoph Hellwig wrote:

> a)  struct device/scsi_device misses a proper state bits that sais
>    "I'm gone now".  Mike is implementing this for scsi now but this
>    really needs to be handled at the driver model level as other subsystems
>    have the same problems.
> 
> I'm waiting for Mike to post his patches and will attack a) after that.
> In fact it's quite easy and I could just do it now :)

I'm not sure exactly what you have in mind, but here's an example patch
that does something along the lines you're talking about.  You mentioned
earlier that there needs to be a way to tell sysfs not to make any more
call-backs because the item in question has gone away.  This patch does
exactly that for attribute files.  It guarantees that after
sys_remove_file() -- or the driver model equivalent -- returns, there will
be no calls to the attribute's show() or store() methods.  This isn't 
currently true, because a process can hold an attribute file open even 
after it has been removed.

I implented this in a klugy kind of way, so it shouldn't be considered for 
inclusion in the kernel until someone has cleaned it up.

Is this the sort of thing you're looking for?

Alan Stern


===== bin.c 1.7 vs edited =====
--- 1.7/fs/sysfs/bin.c	Thu Jun 12 12:40:23 2003
+++ edited/fs/sysfs/bin.c	Mon Jul  7 11:34:00 2003
@@ -17,8 +17,15 @@
 {
 	struct bin_attribute * attr = dentry->d_fsdata;
 	struct kobject * kobj = dentry->d_parent->d_fsdata;
+	int rc;
 
-	return attr->read(kobj, buffer, off, count);
+	down(&dentry->d_inode->i_sem);
+	if (ATTR_GONE(dentry))
+		rc = -ENXIO;
+	else
+		rc = attr->read(kobj, buffer, off, count);
+	up(&dentry->d_inode->i_sem);
+	return rc;
 }
 
 static ssize_t
@@ -61,8 +68,15 @@
 {
 	struct bin_attribute *attr = dentry->d_fsdata;
 	struct kobject *kobj = dentry->d_parent->d_fsdata;
+	int rc;
 
-	return attr->write(kobj, buffer, offset, count);
+	down(&dentry->d_inode->i_sem);
+	if (ATTR_GONE(dentry))
+		rc = -ENXIO;
+	else
+		rc = attr->write(kobj, buffer, offset, count);
+	up(&dentry->d_inode->i_sem);
+	return rc;
 }
 
 static ssize_t write(struct file * file, const char __user * userbuf,
===== dir.c 1.2 vs edited =====
--- 1.2/fs/sysfs/dir.c	Mon Mar 17 08:45:12 2003
+++ edited/fs/sysfs/dir.c	Mon Jul  7 11:23:07 2003
@@ -98,6 +98,9 @@
 			 * Unlink and unhash.
 			 */
 			spin_unlock(&dcache_lock);
+			down(&d->d_inode->i_sem);
+			SET_ATTR_GONE(d);
+			up(&d->d_inode->i_sem);
 			simple_unlink(dentry->d_inode,d);
 			dput(d);
 			spin_lock(&dcache_lock);
===== file.c 1.7 vs edited =====
--- 1.7/fs/sysfs/file.c	Wed Jul  2 17:35:09 2003
+++ edited/fs/sysfs/file.c	Mon Jul  7 11:32:43 2003
@@ -77,8 +77,9 @@
  */
 static int fill_read_buffer(struct file * file, struct sysfs_buffer * buffer)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct dentry * dentry = file->f_dentry;
+	struct attribute * attr = dentry->d_fsdata;
+	struct kobject * kobj = dentry->d_parent->d_fsdata;
 	struct sysfs_ops * ops = buffer->ops;
 	int ret = 0;
 	ssize_t count;
@@ -88,7 +89,12 @@
 	if (!buffer->page)
 		return -ENOMEM;
 
-	count = ops->show(kobj,attr,buffer->page);
+	down(&dentry->d_inode->i_sem);
+	if (ATTR_GONE(dentry))
+		count = -ENXIO;
+	else
+		count = ops->show(kobj,attr,buffer->page);
+	up(&dentry->d_inode->i_sem);
 	if (count >= 0)
 		buffer->count = count;
 	else
@@ -198,11 +204,19 @@
 static int 
 flush_write_buffer(struct file * file, struct sysfs_buffer * buffer, size_t count)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct dentry * dentry = file->f_dentry;
+	struct attribute * attr = dentry->d_fsdata;
+	struct kobject * kobj = dentry->d_parent->d_fsdata;
 	struct sysfs_ops * ops = buffer->ops;
+	int rc;
 
-	return ops->store(kobj,attr,buffer->page,count);
+	down(&dentry->d_inode->i_sem);
+	if (ATTR_GONE(dentry))
+		rc = -ENXIO;
+	else
+		rc = ops->store(kobj,attr,buffer->page,count);
+	up(&dentry->d_inode->i_sem);
+	return rc;
 }
 
 
===== inode.c 1.18 vs edited =====
--- 1.18/fs/sysfs/inode.c	Mon Jun  9 13:37:23 2003
+++ edited/fs/sysfs/inode.c	Mon Jul  7 11:23:07 2003
@@ -11,6 +11,7 @@
 #include <linux/pagemap.h>
 #include <linux/namei.h>
 #include <linux/backing-dev.h>
+#include "sysfs.h"
 extern struct super_block * sysfs_sb;
 
 static struct address_space_operations sysfs_aops = {
@@ -97,6 +98,9 @@
 			pr_debug("sysfs: Removing %s (%d)\n", victim->d_name.name,
 				 atomic_read(&victim->d_count));
 
+			down(&victim->d_inode->i_sem);
+			SET_ATTR_GONE(victim);
+			up(&victim->d_inode->i_sem);
 			simple_unlink(dir->d_inode,victim);
 			d_delete(victim);
 		}
===== sysfs.h 1.1 vs edited =====
--- 1.1/fs/sysfs/sysfs.h	Wed Mar  5 00:02:34 2003
+++ edited/fs/sysfs/sysfs.h	Mon Jul  7 11:30:03 2003
@@ -4,7 +4,16 @@
 extern struct inode * sysfs_new_inode(mode_t mode);
 extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));
 
-extern struct dentry * sysfs_get_dentry(struct dentry *, char *);
+extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
 
 extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
 
+/*
+ * Misuse an attribute's inode's i_mode: set it to 0 to indicate that the
+ * attribute file has been removed.
+ */
+
+#define ATTR_GONE(dentry)	((dentry)->d_inode->i_mode == 0)
+
+#define SET_ATTR_GONE(dentry)	do {		\
+		(dentry)->d_inode->i_mode = 0;	} while (0)

Re: PATCH: (as54) Fix hot-unplugging for sr.c

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> Mike:
> 
> This is the final version of my patch to fix hot-unplugging for the SCSI 
> cdrom driver.  It prevents the private data structure from being 
> deallocated until all references to it have been dropped, and it prevents 
> races between sr_block_open() and sr_remove().
> 
> The patch does assume, however, that it is legal to submit requests (via 
> scsi_ioctl, for example) for a device that is marked offline.
> 
> Similar changes may be needed for the sd, st, and sg drivers; I haven't 
> looked at them.
> 
> Alan Stern

Alan,
	This looks like it solves the problem for sr and could be
	replicated for other upper level drivers. The patch I created
	works a little differently in that the remove function is not
	called until opens drop to zero (if the device is open).

	I started a new thread for the cleanup patches I have been
	working on  because I have four patches in the series (
	including a refresh of your /proc/scsi fixups).

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: PATCH: (as54) Fix hot-unplugging for sr.c

[Alan Stern]

On Tue, 8 Jul 2003, Mike Anderson wrote:

> Alan,
> 	This looks like it solves the problem for sr and could be
> 	replicated for other upper level drivers. The patch I created
> 	works a little differently in that the remove function is not
> 	called until opens drop to zero (if the device is open).

Interesting.  As I understand it, remove() is called from the SCSI core
whereas block_release() is called through the filesystem.  The two code
paths only intersect in sr.c.  So how does the core know when opens drops
to 0?

> 	I started a new thread for the cleanup patches I have been
> 	working on  because I have four patches in the series (
> 	including a refresh of your /proc/scsi fixups).

I'm looking forward to seeing the end results.  Incidentally, it turns out
that my patch still needs some minor fixups.  Locking is notoriously
difficult to get exactly right.  But it can wait until after this other
stuff is finished.

Speaking of /proc/scsi, have you given any thought to making the procinfo 
contents available through sysfs?  That way the /proc entry points can be 
deprecated and eventually removed, but the information will still be 
easily available.

Alan Stern

Re: PATCH: (as54) Fix hot-unplugging for sr.c

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> On Tue, 8 Jul 2003, Mike Anderson wrote:
> 
> > Alan,
> > 	This looks like it solves the problem for sr and could be
> > 	replicated for other upper level drivers. The patch I created
> > 	works a little differently in that the remove function is not
> > 	called until opens drop to zero (if the device is open).
> 
> Interesting.  As I understand it, remove() is called from the SCSI core
> whereas block_release() is called through the filesystem.  The two code
> paths only intersect in sr.c.  So how does the core know when opens drops
> to 0?

struct device_driver remove() is called as a result of device_unregister
or device_del being called which usually is called by the SCSI core.

I changed scsi_device_put to check if access_count drops to zero and the
device is marked to be deleted. If these two checks are true then call
device_del. access_count is also checked in the scsi_sysfs.c code under
a common sema.

> Speaking of /proc/scsi, have you given any thought to making the procinfo 
> contents available through sysfs?  That way the /proc entry points can be 
> deprecated and eventually removed, but the information will still be 
> easily available.

Yes we should look into this. A old issue was a policy for sysfs of
one attribute value per file. This would be a large overhead for some of
the LLDDs to convert to and it would probably be a mess in the
directory.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: PATCH: (as54) Fix hot-unplugging for sr.c

[Alan Stern]

On Wed, 9 Jul 2003, Mike Anderson wrote:

> struct device_driver remove() is called as a result of device_unregister
> or device_del being called which usually is called by the SCSI core.
> 
> I changed scsi_device_put to check if access_count drops to zero and the
> device is marked to be deleted. If these two checks are true then call
> device_del. access_count is also checked in the scsi_sysfs.c code under
> a common sema.

I'll need to see all the changes together to fully grasp it.  There are 
lots of little things not described explicitly anywhere but only implicit 
in the code.  In particular, the detailed nature of the interlocks 
relating the struct scsi_device and the struct scsi_cd is unclear (and 
seems to be changing rapidly).  Not to mention the struct gendisk and the 
struct cdrom_device_info.

> > Speaking of /proc/scsi, have you given any thought to making the procinfo 
> > contents available through sysfs?  That way the /proc entry points can be 
> > deprecated and eventually removed, but the information will still be 
> > easily available.
> 
> Yes we should look into this. A old issue was a policy for sysfs of
> one attribute value per file. This would be a large overhead for some of
> the LLDDs to convert to and it would probably be a mess in the
> directory.

As far as I'm concerned, that policy need not be a stumbling block.  I can 
envision two reasons for wanting to have more than one value per file:

	1. The file might represent a snapshot of real-time data being
maintained by the driver.  If the values were stored in different 
attribute files there would be no way to retrieve a consistent snapshot.

	2. The file might be intended to convey summarized information
of interest to a human but not meant to be parsed by any sort of automatic 
tool.  A summary is a lot easier to use when it's all in one place and not 
scattered among lots of little files.

Of course, the information in /proc/scsi/scsi is already in sysfs, obeying 
the one-value-per-file dictate.  But the /proc/scsi/host/host# files were 
never meant to be structured at all, so far as I know -- they would fall 
under category 2 above.

Alan Stern

PATCH: (as33e) Fix removal of /proc/scsi/hostdir on hot-unplug

[Alan Stern]

Mike:

I sent this patch to you some time ago, and now that 2.6.0 is coming maybe
it's time to submit it again.  This fixes the problem that
/proc/scsi/hostdir isn't removed when the host adapter is hot-unplugged.
It accomplishes this by creating the directory and the files within it at
separate times, and it should work with drivers using either the legacy or
the hot-plugging model.

The patch applies to 2.6.0-test2.

Alan Stern


# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1656  -> 1.1657 
#	drivers/scsi/scsi_priv.h	1.6     -> 1.7    
#	drivers/scsi/hosts.c	1.30    -> 1.31   
#	drivers/scsi/scsi_proc.c	1.15    -> 1.16   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/07/31	stern@ida.rowland.org	1.1657
# Make sure /proc/scsi/hostdir is removed upon hot-unplug.
# Separate the creation of /proc/scsi/hostdir and /proc/scsi/hostdir/device.
# --------------------------------------------
#
diff -Nru a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
--- a/drivers/scsi/hosts.c	Thu Jul 31 15:28:30 2003
+++ b/drivers/scsi/hosts.c	Thu Jul 31 15:28:30 2003
@@ -108,7 +108,7 @@
 		shost->eh_notify = NULL;
 	}
 
-	shost->hostt->present--;
+	scsi_proc_hostdir_rm(shost->hostt);
 	scsi_destroy_command_freelist(shost);
 	kfree(shost);
 }
@@ -209,7 +209,7 @@
 	kernel_thread((int (*)(void *))scsi_error_handler, shost, 0);
 	wait_for_completion(&complete);
 	shost->eh_notify = NULL;
-	shost->hostt->present++;
+	scsi_proc_hostdir_add(shost->hostt);
 	return shost;
  fail:
 	kfree(shost);
diff -Nru a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
--- a/drivers/scsi/scsi_priv.h	Thu Jul 31 15:28:30 2003
+++ b/drivers/scsi/scsi_priv.h	Thu Jul 31 15:28:30 2003
@@ -41,6 +41,8 @@
 #define SCSI_SENSE_VALID(scmd) \
 	(((scmd)->sense_buffer[0] & 0x70) == 0x70)
 
+struct scsi_host_template;
+
 /*
  * scsi_target: representation of a scsi target, for now, this is only
  * used for single_lun devices. If no one has active IO to the target,
@@ -90,11 +92,15 @@
 
 /* scsi_proc.c */
 #ifdef CONFIG_PROC_FS
+extern void scsi_proc_hostdir_add(struct scsi_host_template *);
+extern void scsi_proc_hostdir_rm(struct scsi_host_template *);
 extern void scsi_proc_host_add(struct Scsi_Host *);
 extern void scsi_proc_host_rm(struct Scsi_Host *);
 extern int scsi_init_procfs(void);
 extern void scsi_exit_procfs(void);
 #else
+# define scsi_proc_hostdir_add(sht)	do { } while (0)
+# define scsi_proc_hostdir_rm(sht)	do { } while (0)
 # define scsi_proc_host_add(shost)	do { } while (0)
 # define scsi_proc_host_rm(shost)	do { } while (0)
 # define scsi_init_procfs()		(0)
diff -Nru a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
--- a/drivers/scsi/scsi_proc.c	Thu Jul 31 15:28:30 2003
+++ b/drivers/scsi/scsi_proc.c	Thu Jul 31 15:28:30 2003
@@ -41,6 +41,9 @@
 struct proc_dir_entry *proc_scsi;
 EXPORT_SYMBOL(proc_scsi);
 
+/* Protect sht->present and sht->proc_dir */
+static DECLARE_MUTEX(global_host_template_sem);
+
 
 static int proc_scsi_read(char *buffer, char **start, off_t offset,
 			  int length, int *eof, void *data)
@@ -77,24 +80,39 @@
 	return ret;
 }
 
-void scsi_proc_host_add(struct Scsi_Host *shost)
+void scsi_proc_hostdir_add(struct scsi_host_template *sht)
 {
-	struct scsi_host_template *sht = shost->hostt;
-	struct proc_dir_entry *p;
-	char name[10];
-
-	if (!sht->proc_info)
-		return;
-
-	if (!sht->proc_dir) {
+	down(&global_host_template_sem);
+	if (!sht->present++) {
 		sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
-        	if (!sht->proc_dir) {
+		if (!sht->proc_dir) {
 			printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
 			       __FUNCTION__, sht->proc_name);
 			return;
 		}
 		sht->proc_dir->owner = sht->module;
 	}
+	up(&global_host_template_sem);
+}
+
+void scsi_proc_hostdir_rm(struct scsi_host_template *sht)
+{
+	down(&global_host_template_sem);
+	if (!--sht->present && sht->proc_dir) {
+		remove_proc_entry(sht->proc_name, proc_scsi);
+		sht->proc_dir = NULL;
+	}
+	up(&global_host_template_sem);
+}
+
+void scsi_proc_host_add(struct Scsi_Host *shost)
+{
+	struct scsi_host_template *sht = shost->hostt;
+	struct proc_dir_entry *p;
+	char name[10];
+
+	if (!sht->proc_dir)
+		return;
 
 	sprintf(name,"%d", shost->host_no);
 	p = create_proc_read_entry(name, S_IFREG | S_IRUGO | S_IWUSR,
@@ -107,7 +125,7 @@
 	} 
 
 	p->write_proc = proc_scsi_write_proc;
-	p->owner = shost->hostt->module;
+	p->owner = sht->module;
 }
 
 void scsi_proc_host_rm(struct Scsi_Host *shost)
@@ -115,12 +133,11 @@
 	struct scsi_host_template *sht = shost->hostt;
 	char name[10];
 
-	if (sht->proc_info) {
-		sprintf(name,"%d", shost->host_no);
-		remove_proc_entry(name, sht->proc_dir);
-		if (!sht->present)
-			remove_proc_entry(sht->proc_name, proc_scsi);
-	}
+	if (!sht->proc_dir)
+		return;
+
+	sprintf(name,"%d", shost->host_no);
+	remove_proc_entry(name, sht->proc_dir);
 }
 
 static int proc_print_scsidevice(struct device *dev, void *data)

Re: PATCH: (as33e) Fix removal of /proc/scsi/hostdir on hot-unplug

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> Mike:
> 
> I sent this patch to you some time ago, and now that 2.6.0 is coming maybe
> it's time to submit it again.

I have resent this patch out a couple of times also plus the addition of
reducing users of the present bit. The version of your patch I have is
slightly differnt on a few lines. I will resend that version I have back
out to the list as I have been testing the whole series. It does not
matter to me which one gets use, but we need to use one of them so that
we stop getting errors.

-andmike

PATCH: (as84) Fix my earlier scsi procdir patch

[Alan Stern]

Mike:

I'm pleased to see that the procdir patch has made it into
2.6.0-test3-bk2.  But looking at it again, I realized it has a mistake:  
The semaphore isn't released when an error occurs.  This patch fixes it.

Alan Stern


===== scsi_proc.c 1.16 vs edited =====
--- 1.16/drivers/scsi/scsi_proc.c	Thu Aug 14 18:37:32 2003
+++ edited/drivers/scsi/scsi_proc.c	Fri Aug 15 15:40:28 2003
@@ -84,12 +84,11 @@
 	down(&global_host_template_sem);
 	if (!sht->present++) {
 		sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
-        	if (!sht->proc_dir) {
+        	if (!sht->proc_dir)
 			printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
 			       __FUNCTION__, sht->proc_name);
-			return;
-		}
-		sht->proc_dir->owner = sht->module;
+		else
+			sht->proc_dir->owner = sht->module;
 	}
 	up(&global_host_template_sem);
 }

PATCH: (as84) Small fixup for SCSI proc code

[Alan Stern]

Mike:

The updated SCSI proc directory code that I sent you a few months ago has
a small mistake -- the error path fails to release a semaphore.  This
patch fixes it; please apply.

Alan Stern


===== scsi_proc.c 1.16 vs edited =====
--- 1.16/drivers/scsi/scsi_proc.c	Thu Aug 14 18:37:32 2003
+++ edited/drivers/scsi/scsi_proc.c	Fri Aug 15 15:40:28 2003
@@ -85,12 +85,11 @@
 	down(&global_host_template_sem);
 	if (!sht->present++) {
 		sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
-        	if (!sht->proc_dir) {
+        	if (!sht->proc_dir)
 			printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
 			       __FUNCTION__, sht->proc_name);
-			return;
-		}
-		sht->proc_dir->owner = sht->module;
+		else
+			sht->proc_dir->owner = sht->module;
 	}
 	up(&global_host_template_sem);
 }

Race in removal of host class device attribute file

[Alan Stern]

I've created a class device attribute file for the hosts supported by the
usb-storage driver, using the shost_attrs member of the host template.  
It seems to contain an intractable race condition when unregistering the
host.  There's no way for the driver to know when the scsi host's class
device is released, so there's no way to know when it's safe to deallocate
the host's driver-specific data structure.  Waiting until
scsi_remove_host() returns isn't good enough, because a user process might
keep the attribute file open and thus prevent release of the class device
indefinitely.

What's the right way to solve this problem?  It seems to me that it must 
involve creating a kobject in the driver-specific host data structure, on 
which the class device's release() routine could do a kobject_put().  That 
way the driver would be able to tell when it should free the data 
structure.  But code to handle this would have to be added to the scsi 
core.

Alan Stern

Re: Race in removal of host class device attribute file

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> I've created a class device attribute file for the hosts supported by the
> usb-storage driver, using the shost_attrs member of the host template.  
> It seems to contain an intractable race condition when unregistering the
> host.  There's no way for the driver to know when the scsi host's class
> device is released, so there's no way to know when it's safe to deallocate
> the host's driver-specific data structure.  Waiting until
> scsi_remove_host() returns isn't good enough, because a user process might
> keep the attribute file open and thus prevent release of the class device
> indefinitely.
> 
> What's the right way to solve this problem?  It seems to me that it must 
> involve creating a kobject in the driver-specific host data structure, on 
> which the class device's release() routine could do a kobject_put().  That 
> way the driver would be able to tell when it should free the data 
> structure.  But code to handle this would have to be added to the scsi 
> core.
> 
> Alan Stern
> 

The free of this data has to happen on some release function. How are
you allocating the data / class_device_attribute structure? If the
allocation came from the scsi_host_alloc extra bytes the data will not
be free'd until scsi_host_dev_release().

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Race in removal of host class device attribute file

[Alan Stern]

On Thu, 16 Oct 2003, Mike Anderson wrote:

> Alan Stern [stern@rowland.harvard.edu] wrote:
> > I've created a class device attribute file for the hosts supported by the
> > usb-storage driver, using the shost_attrs member of the host template.  
> > It seems to contain an intractable race condition when unregistering the
> > host.  There's no way for the driver to know when the scsi host's class
> > device is released, so there's no way to know when it's safe to deallocate
> > the host's driver-specific data structure.  Waiting until
> > scsi_remove_host() returns isn't good enough, because a user process might
> > keep the attribute file open and thus prevent release of the class device
> > indefinitely.
> > 
> > What's the right way to solve this problem?  It seems to me that it must 
> > involve creating a kobject in the driver-specific host data structure, on 
> > which the class device's release() routine could do a kobject_put().  That 
> > way the driver would be able to tell when it should free the data 
> > structure.  But code to handle this would have to be added to the scsi 
> > core.
> > 
> > Alan Stern
> > 
> 
> The free of this data has to happen on some release function. How are
> you allocating the data / class_device_attribute structure? If the
> allocation came from the scsi_host_alloc extra bytes the data will not
> be free'd until scsi_host_dev_release().

The data structure is allocated at the time the USB connection is 
established, and it must be done _before_ registering the scsi host.  
Likewise, it is freed when the USB cable is unplugged, _after_ removing 
the scsi host.  Hence it cannot be stored in the extra bytes.

However, I figured out an answer.  This is the sort of problem faced by
practically all drivers that use the sysfs filesystem: opening an
attribute file races with disconnecting the device.  The solution is to
have a driver-wide semaphore that protects the hostdata[] bytes in the
host structure.  The attribute file's read routine must hold the semaphore
while accessing the driver-specific host data structure.  The driver must
hold the semaphore when it erases the pointer to the data structure (which
is stored in the hostdata[] area) at the time a disconnect occurs.

That way, when a user process attempts to read the attribute file after
the data structure has been freed, it's guaranteed that the pointer to the
data structure will be NULL.  The attribute read routine can then avoid
trying to access deallocated memory.  Without that semaphore, it would be
possible for the driver to free the data structure _while_ the read
routine was using it.

Alan Stern

Re: Race in removal of host class device attribute file

[Christoph Hellwig]

On Thu, Oct 16, 2003 at 05:09:04PM -0400, Alan Stern wrote:
> I've created a class device attribute file for the hosts supported by the
> usb-storage driver, using the shost_attrs member of the host template.  
> It seems to contain an intractable race condition when unregistering the
> host.  There's no way for the driver to know when the scsi host's class
> device is released, so there's no way to know when it's safe to deallocate
> the host's driver-specific data structure.  Waiting until
> scsi_remove_host() returns isn't good enough, because a user process might
> keep the attribute file open and thus prevent release of the class device
> indefinitely.

What's the problem with that?  Yes, that means a user process can lock
the memory into core, but we do this in many places.  General rule
of the thumb is that a driver may only release data after scsi_remove_host()
returns for per-host data or in ->slave_destroy() for per-device data.

Suggestion for aiding debugging of host removal

[Alan Stern]

Mike:

I've got a question about host removal.  Once scsi_remove_host() has
returned, the host driver's module is free to unload from memory (assuming
the module's reference count is 0, which it normally is).  Hence it is a
mistake to access the host template in any way after that time.

But it looks like scsi_host_dev_release() can be called after
scsi_remove_host() has returned, and it uses shost->hostt.  There may be 
other uses as well.

Would it help flush out such illegal accesses if at some appropriate point 
shost->hostt was set to NULL, maybe near the end of scsi_remove_host()?

Alan Stern

Re: Suggestion for aiding debugging of host removal

[Christoph Hellwig]

On Wed, Dec 10, 2003 at 10:02:22AM -0500, Alan Stern wrote:
> Mike:
> 
> I've got a question about host removal.  Once scsi_remove_host() has
> returned, the host driver's module is free to unload from memory (assuming
> the module's reference count is 0, which it normally is).  Hence it is a
> mistake to access the host template in any way after that time.
> 
> But it looks like scsi_host_dev_release() can be called after
> scsi_remove_host() has returned, and it uses shost->hostt.  There may be 
> other uses as well.
> 
> Would it help flush out such illegal accesses if at some appropriate point 
> shost->hostt was set to NULL, maybe near the end of scsi_remove_host()?

In fact that's a bug in the current scsi_host lifetime handling - before
the driver can leave it's upper layer ->remove function we need to wait
to the host refcount to become zero, similar to what free_netdev does.

I'll see whether I can come up with a fix.

DMA Timeout with Promise S150TX4 and 2.6.0-test11-bk8

[Paul]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

Hi All,

I am currently testing a box with a Promise Serial ATA RAID controller 4
port and 2.6.0-test11-bk8 (latest from kernel.org)
I can boot OK, access the drives correctly, read/write OK. When I do a
command such as rm on a 3-4GB file and then whilst it's doing that command
issue an ls -la on the same console the original rm process goes into "D"
state and just sits there. The original console never responds back. I can
login to a new session OK, just the one with the rm freezes up. I can
kill -9 the bash pid but not the rm pid, its in state D. A reboot halts just
after "unmounting filesystems" and it doesn't complete the reboot. I suspect
this is because it can't unmount the disk due to a locked operation of some
description. Once this problem occurs I can no longer read/write to the
disks on the mentioned controller. Any attempts, ie: ls just freeze and it
goes into state D

I get in dmesg (when the problem occurs)

ata2: DMA timeout
ata1: DMA timeout

Below are my dmesg output and other debugging in case this helps. I can
complile/change/test at will so if you have anything you need tested let me
know, the box isnt in production yet.

Linux backup01 2.6.0-test11-bk8 #2 Thu Dec 11 13:00:41 EST 2003 i686 unknown

Dmesg/lspci/kernel config are attached as text files

[-- Attachment #2: box-dmesg.txt --]
[-- Type: text/plain, Size: 7945 bytes --]

Linux version 2.6.0-test11-bk8 (root@backup01) (gcc version 3.2.2) #2 Thu Dec 11 13:00:41 EST 2003
BIOS-provided physical RAM map:
 BIOS-e820: 0000000000000000 - 0000000000098c00 (usable)
 BIOS-e820: 0000000000098c00 - 00000000000a0000 (reserved)
 BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
 BIOS-e820: 0000000000100000 - 000000000effc000 (usable)
 BIOS-e820: 000000000effc000 - 000000000efff000 (ACPI data)
 BIOS-e820: 000000000efff000 - 000000000f000000 (ACPI NVS)
 BIOS-e820: 00000000fff80000 - 0000000100000000 (reserved)
239MB LOWMEM available.
On node 0 totalpages: 61436
  DMA zone: 4096 pages, LIFO batch:1
  Normal zone: 57340 pages, LIFO batch:13
  HighMem zone: 0 pages, LIFO batch:1
DMI 2.3 present.
Building zonelist for node : 0
Kernel command line: auto BOOT_IMAGE=Linux-2.6.0 ro root=301
Initializing CPU#0
PID hash table entries: 1024 (order 10: 8192 bytes)
Detected 1002.237 MHz processor.
Console: colour VGA+ 80x25
Memory: 239792k/245744k available (1818k kernel code, 5228k reserved, 711k data, 116k init, 0k highmem)
Calibrating delay loop... 1986.56 BogoMIPS
Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
CPU:     After generic identify, caps: 0387f9ff 00000000 00000000 00000000
CPU:     After vendor identify, caps: 0387f9ff 00000000 00000000 00000000
CPU: L1 I cache: 16K, L1 D cache: 16K
CPU: L2 cache: 256K
CPU serial number disabled.
CPU:     After all inits, caps: 0383f9ff 00000000 00000000 00000040
Intel machine check architecture supported.
Intel machine check reporting enabled on CPU#0.
CPU: Intel Pentium III (Coppermine) stepping 0a
Enabling fast FPU save and restore... done.
Enabling unmasked SIMD FPU exception support... done.
Checking 'hlt' instruction... OK.
POSIX conformance testing by UNIFIX
NET: Registered protocol family 16
PCI: PCI BIOS revision 2.10 entry at 0xf0e90, last bus=1
PCI: Using configuration type 1
mtrr: v2.0 (20020519)
Linux Plug and Play Support v0.97 (c) Adam Belay
SCSI subsystem initialized
PCI: Probing PCI hardware
PCI: Probing PCI hardware (bus 00)
PCI: Using IRQ router SIS [1039/0008] at 0000:00:01.0
PCI: IRQ 0 for device 0000:00:0f.0 doesn't match PIRQ mask - try pci=usepirqmask
SBF: Simple Boot Flag extension found and enabled.
SBF: Setting boot flags 0x80
Machine check exception polling timer started.
udf: registering filesystem
pty: 256 Unix98 ptys configured
request_module: failed /sbin/modprobe -- parport_lowlevel. error = -16
lp: driver loaded but no devices found
Linux agpgart interface v0.100 (c) Dave Jones
[drm:drm_init] *ERROR* Cannot initialize the agpgart module.
Serial: 8250/16550 driver $Revision: 1.90 $ 8 ports, IRQ sharing disabled
ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A
parport0: PC-style at 0x378 (0x778) [PCSPP(,...)]
parport0: irq 7 detected
lp0: using parport0 (polling).
Using anticipatory io scheduler
Floppy drive(s): fd0 is 1.44M
FDC 0 is a post-1991 82077
8139cp: 10/100 PCI Ethernet driver v1.1 (Aug 30, 2003)
8139cp: pci dev 0000:00:0f.0 (id 10ec:8139 rev 10) is not an 8139C+ compatible chip
8139cp: Try the "8139too" driver instead.
8139too Fast Ethernet driver 0.9.26
PCI: Enabling device 0000:00:0f.0 (0004 -> 0007)
PCI: IRQ 0 for device 0000:00:0f.0 doesn't match PIRQ mask - try pci=usepirqmask
PCI: Assigned IRQ 9 for device 0000:00:0f.0
eth0: RealTek RTL8139 at 0x8800, 00:40:f4:2b:91:7f, IRQ 9
eth0:  Identified 8139 chip type 'RTL-8139C'
Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
SIS5513: IDE controller at PCI slot 0000:00:00.1
SIS5513: chipset revision 208
SIS5513: not 100% native mode: will probe irqs later
SIS5513: SiS630 ATA 100 (1st gen) controller
    ide0: BM-DMA at 0xd800-0xd807, BIOS settings: hda:DMA, hdb:DMA
    ide1: BM-DMA at 0xd808-0xd80f, BIOS settings: hdc:DMA, hdd:DMA
hda: ST3160023A, ATA DISK drive
hdb: WDC WD1200JB-00DUA0, ATA DISK drive
ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
hdc: WDC WD1200JB-00CRA1, ATA DISK drive
hdd: WDC WD1200JB-00DUA0, ATA DISK drive
ide1 at 0x170-0x177,0x376 on irq 15
hda: max request size: 1024KiB
hda: 312581808 sectors (160041 MB) w/8192KiB Cache, CHS=19457/255/63, UDMA(100)
 hda: hda1 hda2
hdb: max request size: 1024KiB
hdb: 234441648 sectors (120034 MB) w/8192KiB Cache, CHS=16383/255/63, UDMA(100)
 hdb: hdb1
hdc: max request size: 128KiB
hdc: 234441648 sectors (120034 MB) w/8192KiB Cache, CHS=65535/16/63, UDMA(33)
 hdc: hdc1
hdd: max request size: 1024KiB
hdd: 234441648 sectors (120034 MB) w/8192KiB Cache, CHS=16383/255/63, UDMA(33)
 hdd: hdd1
libata version 0.81 loaded.
sata_promise version 0.86
PCI: Found IRQ 5 for device 0000:00:0e.0
PCI: Sharing IRQ 5 with 0000:00:01.2
PCI: Sharing IRQ 5 with 0000:00:01.3
ata1: SATA max UDMA/133 cmd 0xCF8C0200 ctl 0xCF8C0238 bmdma 0x0 irq 5
ata2: SATA max UDMA/133 cmd 0xCF8C0280 ctl 0xCF8C02B8 bmdma 0x0 irq 5
ata3: SATA max UDMA/133 cmd 0xCF8C0300 ctl 0xCF8C0338 bmdma 0x0 irq 5
ata4: SATA max UDMA/133 cmd 0xCF8C0380 ctl 0xCF8C03B8 bmdma 0x0 irq 5
ata1: dev 0 cfg 49:2f00 82:346b 83:7f21 84:4003 85:3469 86:3c01 87:4003 88:203f
ata1: dev 0 ATA, max UDMA/100, 488397168 sectors (lba48)
ata1: dev 0 configured for UDMA/100
scsi0 : sata_promise
ata2: dev 0 cfg 49:2f00 82:346b 83:7f21 84:4003 85:3469 86:3c01 87:4003 88:203f
ata2: dev 0 ATA, max UDMA/100, 488397168 sectors (lba48)
ata2: dev 0 configured for UDMA/100
scsi1 : sata_promise
ATA: abnormal status 0x7F on port 0xCF8C031C
ata3: thread exiting
scsi2 : sata_promise
ATA: abnormal status 0x7F on port 0xCF8C039C
ata4: thread exiting
scsi3 : sata_promise
  Vendor: ATA       Model: WDC WD2500JD-00F  Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
  Vendor: ATA       Model: WDC WD2500JD-00F  Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sda: 488397168 512-byte hdwr sectors (250059 MB)
SCSI device sda: drive cache: write through
 sda: sda1
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
SCSI device sdb: 488397168 512-byte hdwr sectors (250059 MB)
SCSI device sdb: drive cache: write through
 sdb: sdb1
Attached scsi disk sdb at scsi1, channel 0, id 0, lun 0
Attached scsi generic sg0 at scsi0, channel 0, id 0, lun 0,  type 0
Attached scsi generic sg1 at scsi1, channel 0, id 0, lun 0,  type 0
mice: PS/2 mouse device common for all mice
input: PS/2 Logitech Mouse on isa0060/serio1
serio: i8042 AUX port at 0x60,0x64 irq 12
input: AT Translated Set 2 keyboard on isa0060/serio0
serio: i8042 KBD port at 0x60,0x64 irq 1
NET: Registered protocol family 2
IP: routing cache hash table of 2048 buckets, 16Kbytes
TCP: Hash tables configured (established 16384 bind 32768)
NET: Registered protocol family 1
NET: Registered protocol family 17
kjournald starting.  Commit interval 5 seconds
EXT3-fs: mounted filesystem with ordered data mode.
VFS: Mounted root (ext3 filesystem) readonly.
Freeing unused kernel memory: 116k freed
Adding 457844k swap on /dev/hda2.  Priority:-1 extents:1
EXT3 FS on hda1, internal journal
kjournald starting.  Commit interval 5 seconds
EXT3 FS on hdd1, internal journal
EXT3-fs: mounted filesystem with ordered data mode.
kjournald starting.  Commit interval 5 seconds
EXT3 FS on sda1, internal journal
EXT3-fs: mounted filesystem with ordered data mode.
kjournald starting.  Commit interval 5 seconds
EXT3 FS on sdb1, internal journal
EXT3-fs: mounted filesystem with ordered data mode.
eth0: link up, 100Mbps, full-duplex, lpa 0x45E1
process `syslogd' is using obsolete setsockopt SO_BSDCOMPAT
process `snmpd' is using obsolete setsockopt SO_BSDCOMPAT
ata2: DMA timeout
ata1: DMA timeout

[-- Attachment #3: box-lspci.txt --]
[-- Type: text/plain, Size: 6003 bytes --]

00:00.0 Host bridge: Silicon Integrated Systems [SiS] 630 Host (rev 30)
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR-
        Latency: 32
        Region 0: Memory at ee000000 (32-bit, non-prefetchable) [size=16M]
        Capabilities: [c0] AGP version 2.0
                Status: RQ=32 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- HTrans- 64bit- FW- AGP3- Rate=x1,x2,x4
                Command: RQ=1 ArqSz=0 Cal=0 SBA- AGP- GART64- 64bit- FW- Rate=<none>

00:00.1 IDE interface: Silicon Integrated Systems [SiS] 5513 [IDE] (rev d0) (prog-if 80 [Master])
        Subsystem: Asustek Computer, Inc.: Unknown device 80e1
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 16
        Region 4: I/O ports at d800 [size=16]

00:01.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513
        Control: I/O+ Mem+ BusMaster+ SpecCycle+ MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 0

00:01.2 USB Controller: Silicon Integrated Systems [SiS] SiS7001 USB Controller (rev 07) (prog-if 10 [OHCI])
        Subsystem: Unknown device 0039:7001
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 32 (20000ns max), cache line size 08
        Interrupt: pin D routed to IRQ 5
        Region 0: Memory at ed800000 (32-bit, non-prefetchable) [size=4K]

00:01.3 USB Controller: Silicon Integrated Systems [SiS] SiS7001 USB Controller (rev 07) (prog-if 10 [OHCI])
        Subsystem: Unknown device 0039:7000
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 32 (20000ns max), cache line size 08
        Interrupt: pin D routed to IRQ 5
        Region 0: Memory at ed000000 (32-bit, non-prefetchable) [size=4K]

00:02.0 PCI bridge: Silicon Integrated Systems [SiS] SiS 530 Virtual PCI-to-PCI bridge (AGP) (prog-if 00 [Normal decode])
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 0
        Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
        I/O behind bridge: 0000a000-0000afff
        Memory behind bridge: ec000000-ec7fffff
        Prefetchable memory behind bridge: f0000000-febfffff
        BridgeCtl: Parity- SERR- NoISA- VGA+ MAbort- >Reset- FastB2B-

00:0e.0 RAID bus controller: Promise Technology, Inc.: Unknown device 6622 (rev 01)
        Subsystem: Promise Technology, Inc.: Unknown device 6622
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66Mhz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 96 (1500ns min, 4500ns max), cache line size 08
        Interrupt: pin A routed to IRQ 5
        Region 0: I/O ports at 9800 [size=256]
        Region 1: I/O ports at 9400 [size=256]
        Region 2: I/O ports at 9000 [size=256]
        Region 3: Memory at eb800000 (32-bit, non-prefetchable) [size=1M]
        Region 4: Memory at eb000000 (32-bit, non-prefetchable) [size=32K]
        Expansion ROM at <unassigned> [disabled] [size=64K]
        Capabilities: [60] Power Management version 1
                Flags: PMEClk- DSI+ D1+ D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-

00:0f.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 10)
        Subsystem: Realtek Semiconductor Co., Ltd. RT8139
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 32 (8000ns min, 16000ns max)
        Interrupt: pin A routed to IRQ 9
        Region 0: I/O ports at 8800 [size=256]
        Region 1: Memory at ea800000 (32-bit, non-prefetchable) [size=256]
        Capabilities: [50] Power Management version 2
                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0-,D1+,D2+,D3hot+,D3cold+)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-

01:00.0 VGA compatible controller: Silicon Integrated Systems [SiS] SiS630 GUI Accelerator+3D (rev 21) (prog-if 00 [VGA])
        Subsystem: Asustek Computer, Inc.: Unknown device 80e1
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66Mhz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Interrupt: pin A routed to IRQ 11
        BIST result: 00
        Region 0: Memory at f0000000 (32-bit, prefetchable) [size=128M]
        Region 1: Memory at ec000000 (32-bit, non-prefetchable) [size=128K]
        Region 2: I/O ports at a800 [size=128]
        Capabilities: [40] Power Management version 1
                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [50] AGP version 2.0
                Status: RQ=16 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- HTrans- 64bit- FW- AGP3- Rate=x1,x2,x4
                Command: RQ=1 ArqSz=0 Cal=0 SBA- AGP- GART64- 64bit- FW- Rate=<none>

[-- Attachment #4: box-config.txt --]
[-- Type: text/plain, Size: 18733 bytes --]

#
# Automatically generated make config: don't edit
#
CONFIG_X86=y
CONFIG_MMU=y
CONFIG_UID16=y
CONFIG_GENERIC_ISA_DMA=y

#
# Code maturity level options
#
CONFIG_EXPERIMENTAL=y
CONFIG_CLEAN_COMPILE=y
CONFIG_STANDALONE=y
CONFIG_BROKEN_ON_SMP=y

#
# General setup
#
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
# CONFIG_BSD_PROCESS_ACCT is not set
CONFIG_SYSCTL=y
CONFIG_LOG_BUF_SHIFT=14
# CONFIG_IKCONFIG is not set
# CONFIG_EMBEDDED is not set
CONFIG_KALLSYMS=y
CONFIG_FUTEX=y
CONFIG_EPOLL=y
CONFIG_IOSCHED_NOOP=y
CONFIG_IOSCHED_AS=y
CONFIG_IOSCHED_DEADLINE=y

#
# Loadable module support
#
CONFIG_MODULES=y
# CONFIG_MODULE_UNLOAD is not set
CONFIG_OBSOLETE_MODPARM=y
# CONFIG_MODVERSIONS is not set
CONFIG_KMOD=y

#
# Processor type and features
#
CONFIG_X86_PC=y
# CONFIG_X86_VOYAGER is not set
# CONFIG_X86_NUMAQ is not set
# CONFIG_X86_SUMMIT is not set
# CONFIG_X86_BIGSMP is not set
# CONFIG_X86_VISWS is not set
# CONFIG_X86_GENERICARCH is not set
# CONFIG_X86_ES7000 is not set
# CONFIG_M386 is not set
# CONFIG_M486 is not set
# CONFIG_M586 is not set
# CONFIG_M586TSC is not set
# CONFIG_M586MMX is not set
# CONFIG_M686 is not set
# CONFIG_MPENTIUMII is not set
CONFIG_MPENTIUMIII=y
# CONFIG_MPENTIUM4 is not set
# CONFIG_MK6 is not set
# CONFIG_MK7 is not set
# CONFIG_MK8 is not set
# CONFIG_MELAN is not set
# CONFIG_MCRUSOE is not set
# CONFIG_MWINCHIPC6 is not set
# CONFIG_MWINCHIP2 is not set
# CONFIG_MWINCHIP3D is not set
# CONFIG_MCYRIXIII is not set
# CONFIG_MVIAC3_2 is not set
CONFIG_X86_GENERIC=y
CONFIG_X86_CMPXCHG=y
CONFIG_X86_XADD=y
CONFIG_X86_L1_CACHE_SHIFT=7
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_X86_WP_WORKS_OK=y
CONFIG_X86_INVLPG=y
CONFIG_X86_BSWAP=y
CONFIG_X86_POPAD_OK=y
CONFIG_X86_GOOD_APIC=y
CONFIG_X86_INTEL_USERCOPY=y
CONFIG_X86_USE_PPRO_CHECKSUM=y
# CONFIG_HPET_TIMER is not set
# CONFIG_HPET_EMULATE_RTC is not set
# CONFIG_SMP is not set
# CONFIG_PREEMPT is not set
# CONFIG_X86_UP_APIC is not set
CONFIG_X86_TSC=y
CONFIG_X86_MCE=y
CONFIG_X86_MCE_NONFATAL=y
# CONFIG_TOSHIBA is not set
# CONFIG_I8K is not set
# CONFIG_MICROCODE is not set
# CONFIG_X86_MSR is not set
# CONFIG_X86_CPUID is not set
# CONFIG_EDD is not set
CONFIG_NOHIGHMEM=y
# CONFIG_HIGHMEM4G is not set
# CONFIG_HIGHMEM64G is not set
# CONFIG_MATH_EMULATION is not set
CONFIG_MTRR=y

#
# Power management options (ACPI, APM)
#
# CONFIG_PM is not set

#
# ACPI (Advanced Configuration and Power Interface) Support
#
# CONFIG_ACPI is not set

#
# CPU Frequency scaling
#
# CONFIG_CPU_FREQ is not set

#
# Bus options (PCI, PCMCIA, EISA, MCA, ISA)
#
CONFIG_PCI=y
# CONFIG_PCI_GOBIOS is not set
# CONFIG_PCI_GODIRECT is not set
CONFIG_PCI_GOANY=y
CONFIG_PCI_BIOS=y
CONFIG_PCI_DIRECT=y
CONFIG_PCI_LEGACY_PROC=y
CONFIG_PCI_NAMES=y
CONFIG_ISA=y
# CONFIG_EISA is not set
# CONFIG_MCA is not set
# CONFIG_SCx200 is not set
CONFIG_HOTPLUG=y

#
# PCMCIA/CardBus support
#
# CONFIG_PCMCIA is not set
CONFIG_PCMCIA_PROBE=y

#
# PCI Hotplug Support
#
# CONFIG_HOTPLUG_PCI is not set

#
# Executable file formats
#
CONFIG_BINFMT_ELF=y
CONFIG_BINFMT_AOUT=y
CONFIG_BINFMT_MISC=y

#
# Device Drivers
#

#
# Generic Driver Options
#
# CONFIG_FW_LOADER is not set

#
# Memory Technology Devices (MTD)
#
# CONFIG_MTD is not set

#
# Parallel port support
#
CONFIG_PARPORT=y
CONFIG_PARPORT_PC=y
CONFIG_PARPORT_PC_CML1=y
# CONFIG_PARPORT_SERIAL is not set
# CONFIG_PARPORT_PC_FIFO is not set
# CONFIG_PARPORT_PC_SUPERIO is not set
# CONFIG_PARPORT_OTHER is not set
# CONFIG_PARPORT_1284 is not set

#
# Plug and Play support
#
CONFIG_PNP=y
# CONFIG_PNP_DEBUG is not set

#
# Protocols
#
# CONFIG_ISAPNP is not set
# CONFIG_PNPBIOS is not set

#
# Block devices
#
CONFIG_BLK_DEV_FD=y
# CONFIG_BLK_DEV_XD is not set
# CONFIG_PARIDE is not set
# CONFIG_BLK_CPQ_DA is not set
# CONFIG_BLK_CPQ_CISS_DA is not set
# CONFIG_BLK_DEV_DAC960 is not set
# CONFIG_BLK_DEV_UMEM is not set
# CONFIG_BLK_DEV_LOOP is not set
# CONFIG_BLK_DEV_NBD is not set
# CONFIG_BLK_DEV_RAM is not set
# CONFIG_BLK_DEV_INITRD is not set
CONFIG_LBD=y

#
# ATA/ATAPI/MFM/RLL support
#
CONFIG_IDE=y
CONFIG_BLK_DEV_IDE=y

#
# Please see Documentation/ide.txt for help/info on IDE drives
#
# CONFIG_BLK_DEV_HD_IDE is not set
CONFIG_BLK_DEV_IDEDISK=y
CONFIG_IDEDISK_MULTI_MODE=y
# CONFIG_IDEDISK_STROKE is not set
CONFIG_BLK_DEV_IDECD=y
# CONFIG_BLK_DEV_IDETAPE is not set
# CONFIG_BLK_DEV_IDEFLOPPY is not set
# CONFIG_BLK_DEV_IDESCSI is not set
# CONFIG_IDE_TASK_IOCTL is not set
CONFIG_IDE_TASKFILE_IO=y

#
# IDE chipset support/bugfixes
#
CONFIG_BLK_DEV_CMD640=y
# CONFIG_BLK_DEV_CMD640_ENHANCED is not set
# CONFIG_BLK_DEV_IDEPNP is not set
CONFIG_BLK_DEV_IDEPCI=y
CONFIG_IDEPCI_SHARE_IRQ=y
# CONFIG_BLK_DEV_OFFBOARD is not set
CONFIG_BLK_DEV_GENERIC=y
# CONFIG_BLK_DEV_OPTI621 is not set
CONFIG_BLK_DEV_RZ1000=y
CONFIG_BLK_DEV_IDEDMA_PCI=y
# CONFIG_BLK_DEV_IDEDMA_FORCED is not set
CONFIG_IDEDMA_PCI_AUTO=y
# CONFIG_IDEDMA_ONLYDISK is not set
# CONFIG_IDEDMA_PCI_WIP is not set
CONFIG_BLK_DEV_ADMA=y
# CONFIG_BLK_DEV_AEC62XX is not set
# CONFIG_BLK_DEV_ALI15X3 is not set
# CONFIG_BLK_DEV_AMD74XX is not set
# CONFIG_BLK_DEV_CMD64X is not set
# CONFIG_BLK_DEV_TRIFLEX is not set
# CONFIG_BLK_DEV_CY82C693 is not set
# CONFIG_BLK_DEV_CS5520 is not set
# CONFIG_BLK_DEV_CS5530 is not set
# CONFIG_BLK_DEV_HPT34X is not set
# CONFIG_BLK_DEV_HPT366 is not set
# CONFIG_BLK_DEV_SC1200 is not set
CONFIG_BLK_DEV_PIIX=y
# CONFIG_BLK_DEV_NS87415 is not set
# CONFIG_BLK_DEV_PDC202XX_OLD is not set
# CONFIG_BLK_DEV_PDC202XX_NEW is not set
# CONFIG_BLK_DEV_SVWKS is not set
# CONFIG_BLK_DEV_SIIMAGE is not set
CONFIG_BLK_DEV_SIS5513=y
# CONFIG_BLK_DEV_SLC90E66 is not set
# CONFIG_BLK_DEV_TRM290 is not set
# CONFIG_BLK_DEV_VIA82CXXX is not set
# CONFIG_IDE_CHIPSETS is not set
CONFIG_BLK_DEV_IDEDMA=y
# CONFIG_IDEDMA_IVB is not set
CONFIG_IDEDMA_AUTO=y
# CONFIG_DMA_NONPCI is not set
# CONFIG_BLK_DEV_HD is not set

#
# SCSI device support
#
CONFIG_SCSI=y
CONFIG_SCSI_PROC_FS=y

#
# SCSI support type (disk, tape, CD-ROM)
#
CONFIG_BLK_DEV_SD=y
# CONFIG_CHR_DEV_ST is not set
# CONFIG_CHR_DEV_OSST is not set
# CONFIG_BLK_DEV_SR is not set
CONFIG_CHR_DEV_SG=y

#
# Some SCSI devices (e.g. CD jukebox) support multiple LUNs
#
# CONFIG_SCSI_MULTI_LUN is not set
CONFIG_SCSI_REPORT_LUNS=y
# CONFIG_SCSI_CONSTANTS is not set
# CONFIG_SCSI_LOGGING is not set

#
# SCSI low-level drivers
#
# CONFIG_BLK_DEV_3W_XXXX_RAID is not set
# CONFIG_SCSI_7000FASST is not set
# CONFIG_SCSI_ACARD is not set
# CONFIG_SCSI_AHA152X is not set
# CONFIG_SCSI_AHA1542 is not set
# CONFIG_SCSI_AACRAID is not set
# CONFIG_SCSI_AIC7XXX is not set
# CONFIG_SCSI_AIC7XXX_OLD is not set
# CONFIG_SCSI_AIC79XX is not set
# CONFIG_SCSI_ADVANSYS is not set
# CONFIG_SCSI_IN2000 is not set
# CONFIG_SCSI_MEGARAID is not set
CONFIG_SCSI_SATA=y
CONFIG_SCSI_SATA_SVW=y
CONFIG_SCSI_ATA_PIIX=y
CONFIG_SCSI_SATA_PROMISE=y
CONFIG_SCSI_SATA_VIA=y
# CONFIG_SCSI_BUSLOGIC is not set
# CONFIG_SCSI_CPQFCTS is not set
# CONFIG_SCSI_DMX3191D is not set
# CONFIG_SCSI_DTC3280 is not set
# CONFIG_SCSI_EATA is not set
# CONFIG_SCSI_EATA_PIO is not set
# CONFIG_SCSI_FUTURE_DOMAIN is not set
# CONFIG_SCSI_GDTH is not set
# CONFIG_SCSI_GENERIC_NCR5380 is not set
# CONFIG_SCSI_GENERIC_NCR5380_MMIO is not set
# CONFIG_SCSI_IPS is not set
# CONFIG_SCSI_INIA100 is not set
# CONFIG_SCSI_PPA is not set
# CONFIG_SCSI_IMM is not set
# CONFIG_SCSI_NCR53C406A is not set
# CONFIG_SCSI_SYM53C8XX_2 is not set
# CONFIG_SCSI_PAS16 is not set
# CONFIG_SCSI_PSI240I is not set
# CONFIG_SCSI_QLOGIC_FAS is not set
# CONFIG_SCSI_QLOGIC_ISP is not set
# CONFIG_SCSI_QLOGIC_FC is not set
# CONFIG_SCSI_QLOGIC_1280 is not set
# CONFIG_SCSI_SYM53C416 is not set
# CONFIG_SCSI_DC395x is not set
# CONFIG_SCSI_T128 is not set
# CONFIG_SCSI_U14_34F is not set
# CONFIG_SCSI_ULTRASTOR is not set
# CONFIG_SCSI_NSP32 is not set
# CONFIG_SCSI_DEBUG is not set

#
# Old CD-ROM drivers (not SCSI, not IDE)
#
# CONFIG_CD_NO_IDESCSI is not set

#
# Multi-device support (RAID and LVM)
#
# CONFIG_MD is not set

#
# Fusion MPT device support
#
# CONFIG_FUSION is not set

#
# IEEE 1394 (FireWire) support (EXPERIMENTAL)
#
CONFIG_IEEE1394=y

#
# Subsystem Options
#
# CONFIG_IEEE1394_VERBOSEDEBUG is not set
# CONFIG_IEEE1394_OUI_DB is not set

#
# Device Drivers
#

#
# Texas Instruments PCILynx requires I2C bit-banging
#
CONFIG_IEEE1394_OHCI1394=y

#
# Protocol Drivers
#
# CONFIG_IEEE1394_VIDEO1394 is not set
# CONFIG_IEEE1394_SBP2 is not set
# CONFIG_IEEE1394_ETH1394 is not set
# CONFIG_IEEE1394_DV1394 is not set
# CONFIG_IEEE1394_RAWIO is not set
# CONFIG_IEEE1394_CMP is not set

#
# I2O device support
#
# CONFIG_I2O is not set

#
# Networking support
#
CONFIG_NET=y

#
# Networking options
#
CONFIG_PACKET=y
# CONFIG_PACKET_MMAP is not set
# CONFIG_NETLINK_DEV is not set
CONFIG_UNIX=y
# CONFIG_NET_KEY is not set
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
# CONFIG_IP_ADVANCED_ROUTER is not set
# CONFIG_IP_PNP is not set
# CONFIG_NET_IPIP is not set
# CONFIG_NET_IPGRE is not set
# CONFIG_IP_MROUTE is not set
# CONFIG_ARPD is not set
# CONFIG_INET_ECN is not set
# CONFIG_SYN_COOKIES is not set
# CONFIG_INET_AH is not set
# CONFIG_INET_ESP is not set
# CONFIG_INET_IPCOMP is not set
# CONFIG_IPV6 is not set
# CONFIG_DECNET is not set
# CONFIG_BRIDGE is not set
# CONFIG_NETFILTER is not set

#
# SCTP Configuration (EXPERIMENTAL)
#
CONFIG_IPV6_SCTP__=y
# CONFIG_IP_SCTP is not set
# CONFIG_ATM is not set
# CONFIG_VLAN_8021Q is not set
# CONFIG_LLC2 is not set
# CONFIG_IPX is not set
# CONFIG_ATALK is not set
# CONFIG_X25 is not set
# CONFIG_LAPB is not set
# CONFIG_NET_DIVERT is not set
# CONFIG_ECONET is not set
# CONFIG_WAN_ROUTER is not set
# CONFIG_NET_FASTROUTE is not set
# CONFIG_NET_HW_FLOWCONTROL is not set

#
# QoS and/or fair queueing
#
# CONFIG_NET_SCHED is not set

#
# Network testing
#
# CONFIG_NET_PKTGEN is not set
CONFIG_NETDEVICES=y

#
# ARCnet devices
#
# CONFIG_ARCNET is not set
CONFIG_DUMMY=m
# CONFIG_BONDING is not set
# CONFIG_EQUALIZER is not set
# CONFIG_TUN is not set
# CONFIG_NET_SB1000 is not set

#
# Ethernet (10 or 100Mbit)
#
CONFIG_NET_ETHERNET=y
CONFIG_MII=y
# CONFIG_HAPPYMEAL is not set
# CONFIG_SUNGEM is not set
# CONFIG_NET_VENDOR_3COM is not set
# CONFIG_LANCE is not set
# CONFIG_NET_VENDOR_SMC is not set
# CONFIG_NET_VENDOR_RACAL is not set

#
# Tulip family network device support
#
# CONFIG_NET_TULIP is not set
# CONFIG_AT1700 is not set
# CONFIG_DEPCA is not set
# CONFIG_HP100 is not set
# CONFIG_NET_ISA is not set
CONFIG_NET_PCI=y
# CONFIG_PCNET32 is not set
# CONFIG_AMD8111_ETH is not set
# CONFIG_ADAPTEC_STARFIRE is not set
# CONFIG_AC3200 is not set
# CONFIG_APRICOT is not set
# CONFIG_B44 is not set
# CONFIG_CS89x0 is not set
# CONFIG_DGRS is not set
# CONFIG_EEPRO100 is not set
# CONFIG_E100 is not set
# CONFIG_FEALNX is not set
# CONFIG_NATSEMI is not set
# CONFIG_NE2K_PCI is not set
CONFIG_8139CP=y
CONFIG_8139TOO=y
CONFIG_8139TOO_PIO=y
CONFIG_8139TOO_TUNE_TWISTER=y
CONFIG_8139TOO_8129=y
CONFIG_8139_OLD_RX_RESET=y
CONFIG_SIS900=y
# CONFIG_EPIC100 is not set
# CONFIG_SUNDANCE is not set
# CONFIG_TLAN is not set
# CONFIG_VIA_RHINE is not set
# CONFIG_NET_POCKET is not set

#
# Ethernet (1000 Mbit)
#
# CONFIG_ACENIC is not set
# CONFIG_DL2K is not set
# CONFIG_E1000 is not set
# CONFIG_NS83820 is not set
# CONFIG_HAMACHI is not set
# CONFIG_YELLOWFIN is not set
# CONFIG_R8169 is not set
# CONFIG_SIS190 is not set
# CONFIG_SK98LIN is not set
# CONFIG_TIGON3 is not set

#
# Ethernet (10000 Mbit)
#
# CONFIG_IXGB is not set
# CONFIG_FDDI is not set
# CONFIG_HIPPI is not set
# CONFIG_PLIP is not set
# CONFIG_PPP is not set
# CONFIG_SLIP is not set

#
# Wireless LAN (non-hamradio)
#
# CONFIG_NET_RADIO is not set

#
# Token Ring devices
#
# CONFIG_TR is not set
# CONFIG_NET_FC is not set
# CONFIG_RCPCI is not set
# CONFIG_SHAPER is not set

#
# Wan interfaces
#
# CONFIG_WAN is not set

#
# Amateur Radio support
#
# CONFIG_HAMRADIO is not set

#
# IrDA (infrared) support
#
# CONFIG_IRDA is not set

#
# Bluetooth support
#
# CONFIG_BT is not set

#
# ISDN subsystem
#
# CONFIG_ISDN_BOOL is not set

#
# Telephony Support
#
# CONFIG_PHONE is not set

#
# Input device support
#
CONFIG_INPUT=y

#
# Userland interfaces
#
CONFIG_INPUT_MOUSEDEV=y
CONFIG_INPUT_MOUSEDEV_PSAUX=y
CONFIG_INPUT_MOUSEDEV_SCREEN_X=1024
CONFIG_INPUT_MOUSEDEV_SCREEN_Y=768
# CONFIG_INPUT_JOYDEV is not set
# CONFIG_INPUT_TSDEV is not set
# CONFIG_INPUT_EVDEV is not set
# CONFIG_INPUT_EVBUG is not set

#
# Input I/O drivers
#
# CONFIG_GAMEPORT is not set
CONFIG_SOUND_GAMEPORT=y
CONFIG_SERIO=y
CONFIG_SERIO_I8042=y
# CONFIG_SERIO_SERPORT is not set
# CONFIG_SERIO_CT82C710 is not set
# CONFIG_SERIO_PARKBD is not set
# CONFIG_SERIO_PCIPS2 is not set

#
# Input Device Drivers
#
CONFIG_INPUT_KEYBOARD=y
CONFIG_KEYBOARD_ATKBD=y
# CONFIG_KEYBOARD_SUNKBD is not set
# CONFIG_KEYBOARD_XTKBD is not set
# CONFIG_KEYBOARD_NEWTON is not set
CONFIG_INPUT_MOUSE=y
CONFIG_MOUSE_PS2=y
# CONFIG_MOUSE_PS2_SYNAPTICS is not set
# CONFIG_MOUSE_SERIAL is not set
# CONFIG_MOUSE_INPORT is not set
# CONFIG_MOUSE_LOGIBM is not set
# CONFIG_MOUSE_PC110PAD is not set
# CONFIG_INPUT_JOYSTICK is not set
# CONFIG_INPUT_TOUCHSCREEN is not set
# CONFIG_INPUT_MISC is not set

#
# Character devices
#
CONFIG_VT=y
CONFIG_VT_CONSOLE=y
CONFIG_HW_CONSOLE=y
# CONFIG_SERIAL_NONSTANDARD is not set

#
# Serial drivers
#
CONFIG_SERIAL_8250=y
# CONFIG_SERIAL_8250_CONSOLE is not set
CONFIG_SERIAL_8250_NR_UARTS=4
# CONFIG_SERIAL_8250_EXTENDED is not set

#
# Non-8250 serial port support
#
CONFIG_SERIAL_CORE=y
CONFIG_UNIX98_PTYS=y
CONFIG_UNIX98_PTY_COUNT=256
CONFIG_PRINTER=y
# CONFIG_LP_CONSOLE is not set
# CONFIG_PPDEV is not set
# CONFIG_TIPAR is not set

#
# I2C support
#
# CONFIG_I2C is not set

#
# I2C Algorithms
#

#
# I2C Hardware Bus support
#

#
# I2C Hardware Sensors Chip support
#
# CONFIG_I2C_SENSOR is not set

#
# Mice
#
# CONFIG_BUSMOUSE is not set
# CONFIG_QIC02_TAPE is not set

#
# IPMI
#
# CONFIG_IPMI_HANDLER is not set

#
# Watchdog Cards
#
# CONFIG_WATCHDOG is not set
# CONFIG_HW_RANDOM is not set
# CONFIG_NVRAM is not set
# CONFIG_RTC is not set
# CONFIG_GEN_RTC is not set
# CONFIG_DTLK is not set
# CONFIG_R3964 is not set
# CONFIG_APPLICOM is not set
# CONFIG_SONYPI is not set

#
# Ftape, the floppy tape device driver
#
# CONFIG_FTAPE is not set
CONFIG_AGP=y
# CONFIG_AGP_ALI is not set
# CONFIG_AGP_ATI is not set
# CONFIG_AGP_AMD is not set
# CONFIG_AGP_AMD64 is not set
CONFIG_AGP_INTEL=y
# CONFIG_AGP_NVIDIA is not set
# CONFIG_AGP_SIS is not set
# CONFIG_AGP_SWORKS is not set
# CONFIG_AGP_VIA is not set
CONFIG_DRM=y
# CONFIG_DRM_TDFX is not set
# CONFIG_DRM_GAMMA is not set
# CONFIG_DRM_R128 is not set
# CONFIG_DRM_RADEON is not set
# CONFIG_DRM_I810 is not set
CONFIG_DRM_I830=y
# CONFIG_DRM_MGA is not set
# CONFIG_DRM_SIS is not set
# CONFIG_MWAVE is not set
# CONFIG_RAW_DRIVER is not set
# CONFIG_HANGCHECK_TIMER is not set

#
# Multimedia devices
#
# CONFIG_VIDEO_DEV is not set

#
# Digital Video Broadcasting Devices
#
# CONFIG_DVB is not set

#
# Graphics support
#
# CONFIG_FB is not set
# CONFIG_VIDEO_SELECT is not set

#
# Console display driver support
#
CONFIG_VGA_CONSOLE=y
# CONFIG_MDA_CONSOLE is not set
CONFIG_DUMMY_CONSOLE=y

#
# Sound
#
# CONFIG_SOUND is not set

#
# USB support
#
# CONFIG_USB is not set
# CONFIG_USB_GADGET is not set

#
# File systems
#
CONFIG_EXT2_FS=y
# CONFIG_EXT2_FS_XATTR is not set
CONFIG_EXT3_FS=y
CONFIG_EXT3_FS_XATTR=y
# CONFIG_EXT3_FS_POSIX_ACL is not set
# CONFIG_EXT3_FS_SECURITY is not set
CONFIG_JBD=y
# CONFIG_JBD_DEBUG is not set
CONFIG_FS_MBCACHE=y
# CONFIG_REISERFS_FS is not set
# CONFIG_JFS_FS is not set
# CONFIG_XFS_FS is not set
# CONFIG_MINIX_FS is not set
# CONFIG_ROMFS_FS is not set
# CONFIG_QUOTA is not set
# CONFIG_AUTOFS_FS is not set
CONFIG_AUTOFS4_FS=y

#
# CD-ROM/DVD Filesystems
#
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
# CONFIG_ZISOFS is not set
CONFIG_UDF_FS=y

#
# DOS/FAT/NT Filesystems
#
CONFIG_FAT_FS=y
CONFIG_MSDOS_FS=y
CONFIG_VFAT_FS=y
# CONFIG_NTFS_FS is not set

#
# Pseudo filesystems
#
CONFIG_PROC_FS=y
CONFIG_PROC_KCORE=y
# CONFIG_DEVFS_FS is not set
CONFIG_DEVPTS_FS=y
# CONFIG_DEVPTS_FS_XATTR is not set
CONFIG_TMPFS=y
# CONFIG_HUGETLBFS is not set
# CONFIG_HUGETLB_PAGE is not set
CONFIG_RAMFS=y

#
# Miscellaneous filesystems
#
# CONFIG_ADFS_FS is not set
# CONFIG_AFFS_FS is not set
# CONFIG_HFS_FS is not set
# CONFIG_BEFS_FS is not set
# CONFIG_BFS_FS is not set
# CONFIG_EFS_FS is not set
# CONFIG_CRAMFS is not set
# CONFIG_VXFS_FS is not set
# CONFIG_HPFS_FS is not set
# CONFIG_QNX4FS_FS is not set
# CONFIG_SYSV_FS is not set
# CONFIG_UFS_FS is not set

#
# Network File Systems
#
# CONFIG_NFS_FS is not set
# CONFIG_NFSD is not set
# CONFIG_EXPORTFS is not set
CONFIG_SMB_FS=y
CONFIG_SMB_NLS_DEFAULT=y
CONFIG_SMB_NLS_REMOTE="cp437"
# CONFIG_CIFS is not set
# CONFIG_NCP_FS is not set
# CONFIG_CODA_FS is not set
# CONFIG_INTERMEZZO_FS is not set
# CONFIG_AFS_FS is not set

#
# Partition Types
#
# CONFIG_PARTITION_ADVANCED is not set
CONFIG_MSDOS_PARTITION=y
CONFIG_SMB_NLS=y
CONFIG_NLS=y

#
# Native Language Support
#
CONFIG_NLS_DEFAULT="iso8859-1"
CONFIG_NLS_CODEPAGE_437=y
# CONFIG_NLS_CODEPAGE_737 is not set
# CONFIG_NLS_CODEPAGE_775 is not set
# CONFIG_NLS_CODEPAGE_850 is not set
# CONFIG_NLS_CODEPAGE_852 is not set
# CONFIG_NLS_CODEPAGE_855 is not set
# CONFIG_NLS_CODEPAGE_857 is not set
# CONFIG_NLS_CODEPAGE_860 is not set
# CONFIG_NLS_CODEPAGE_861 is not set
# CONFIG_NLS_CODEPAGE_862 is not set
# CONFIG_NLS_CODEPAGE_863 is not set
# CONFIG_NLS_CODEPAGE_864 is not set
# CONFIG_NLS_CODEPAGE_865 is not set
# CONFIG_NLS_CODEPAGE_866 is not set
# CONFIG_NLS_CODEPAGE_869 is not set
# CONFIG_NLS_CODEPAGE_936 is not set
# CONFIG_NLS_CODEPAGE_950 is not set
# CONFIG_NLS_CODEPAGE_932 is not set
# CONFIG_NLS_CODEPAGE_949 is not set
# CONFIG_NLS_CODEPAGE_874 is not set
# CONFIG_NLS_ISO8859_8 is not set
# CONFIG_NLS_CODEPAGE_1250 is not set
# CONFIG_NLS_CODEPAGE_1251 is not set
CONFIG_NLS_ISO8859_1=y
# CONFIG_NLS_ISO8859_2 is not set
# CONFIG_NLS_ISO8859_3 is not set
# CONFIG_NLS_ISO8859_4 is not set
# CONFIG_NLS_ISO8859_5 is not set
# CONFIG_NLS_ISO8859_6 is not set
# CONFIG_NLS_ISO8859_7 is not set
# CONFIG_NLS_ISO8859_9 is not set
# CONFIG_NLS_ISO8859_13 is not set
# CONFIG_NLS_ISO8859_14 is not set
# CONFIG_NLS_ISO8859_15 is not set
# CONFIG_NLS_KOI8_R is not set
# CONFIG_NLS_KOI8_U is not set
# CONFIG_NLS_UTF8 is not set

#
# Profiling support
#
# CONFIG_PROFILING is not set

#
# Kernel hacking
#
# CONFIG_DEBUG_KERNEL is not set
CONFIG_DEBUG_SPINLOCK_SLEEP=y
CONFIG_FRAME_POINTER=y

#
# Security options
#
# CONFIG_SECURITY is not set

#
# Cryptographic options
#
# CONFIG_CRYPTO is not set

#
# Library routines
#
CONFIG_CRC32=y
CONFIG_X86_BIOS_REBOOT=y
CONFIG_PC=y

Re: Suggestion for aiding debugging of host removal

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> On Wed, Dec 10, 2003 at 10:02:22AM -0500, Alan Stern wrote:
> > Mike:
> > 
> > I've got a question about host removal.  Once scsi_remove_host() has
> > returned, the host driver's module is free to unload from memory (assuming
> > the module's reference count is 0, which it normally is).  Hence it is a
> > mistake to access the host template in any way after that time.

Normally it would not be zero on the return from scsi_remove_host. It
would not go to zero until scsi_host_put is called. If the scsi_host_put
is the last ref then prior to the returning of the scsi_host_put
scsi_host_dev_release would have been called. If it is not then there
must of been a open on a device outstanding (which would keep a rmmod
from being called, but would allow hotplug and unexpectged disconnects).
When the device ref count drops this will cause the module count to drop
and allow the module to be removed. We may want to reorder our
scsi_device_put calls to module_put and put_device, but I do not know if
that would prevent any races.

> > 
> > But it looks like scsi_host_dev_release() can be called after
> > scsi_remove_host() has returned, and it uses shost->hostt.  There may be 
> > other uses as well.
> > 
> > Would it help flush out such illegal accesses if at some appropriate point 
> > shost->hostt was set to NULL, maybe near the end of scsi_remove_host()?

If we wanted to set this to something to catch illegal access it might
be good to set hostt to a fake hostt that did a WARN_ON so that it would
not take the system down.

> 
> In fact that's a bug in the current scsi_host lifetime handling - before
> the driver can leave it's upper layer ->remove function we need to wait
> to the host refcount to become zero, similar to what free_netdev does.
> 
> I'll see whether I can come up with a fix.

Unless I missing something I do not see any interlock / wait in free_netdev
(in looking at test11).

Currently the host refcount will not go to zero until the scsi_host_put
is made from the LLDD post the return of the scsi_remove_host call.
While it is valid to sleep in the remove call and we could reorder the
scsi_host_put, I thought the previous goal was to not have an unbounded
time in scsi_remove_host. I thought we where trying to allow the host to
cleanup fast and if user space wanted to take time we would allow that
to happen, but just not send any commands to the LLDD.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Suggestion for aiding debugging of host removal

[Christoph Hellwig]

On Wed, Dec 10, 2003 at 11:48:28PM -0800, Mike Anderson wrote:
> > > I've got a question about host removal.  Once scsi_remove_host() has
> > > returned, the host driver's module is free to unload from memory (assuming
> > > the module's reference count is 0, which it normally is).  Hence it is a
> > > mistake to access the host template in any way after that time.
> 
> Normally it would not be zero on the return from scsi_remove_host. It
> would not go to zero until scsi_host_put is called. If the scsi_host_put
> is the last ref then prior to the returning of the scsi_host_put
> scsi_host_dev_release would have been called. If it is not then there
> must of been a open on a device outstanding (which would keep a rmmod
> from being called, but would allow hotplug and unexpectged disconnects).

An opened sysfs file won't stop the unload.  Only if we still have
a scsi_device still referencing the host.

> > I'll see whether I can come up with a fix.
> 
> Unless I missing something I do not see any interlock / wait in free_netdev
> (in looking at test11).

Sorry, it's actually unregister_nedevice (throuch netdev_run_todo and
netdev_wait_allrefs).

Re: Suggestion for aiding debugging of host removal

[Alan Stern]

On Wed, 10 Dec 2003, Mike Anderson wrote:

> > > Would it help flush out such illegal accesses if at some appropriate point 
> > > shost->hostt was set to NULL, maybe near the end of scsi_remove_host()?
> 
> If we wanted to set this to something to catch illegal access it might
> be good to set hostt to a fake hostt that did a WARN_ON so that it would
> not take the system down.
> 
> > 
> > In fact that's a bug in the current scsi_host lifetime handling - before
> > the driver can leave it's upper layer ->remove function we need to wait
> > to the host refcount to become zero, similar to what free_netdev does.
> > 
> > I'll see whether I can come up with a fix.
> 
> Unless I missing something I do not see any interlock / wait in free_netdev
> (in looking at test11).
> 
> Currently the host refcount will not go to zero until the scsi_host_put
> is made from the LLDD post the return of the scsi_remove_host call.
> While it is valid to sleep in the remove call and we could reorder the
> scsi_host_put, I thought the previous goal was to not have an unbounded
> time in scsi_remove_host. I thought we where trying to allow the host to
> cleanup fast and if user space wanted to take time we would allow that
> to happen, but just not send any commands to the LLDD.

Remember that it's not enough to avoid sending any commands to the LLDD.  
You must avoid accessing the host template at all, since it may no longer 
be present in memory.  Putting in a fake entry would be a good way to 
insure that never happens.

Alan Stern