* [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
@ 2008-03-03 20:16 Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803031508490.7094-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-03 20:16 UTC (permalink / raw
To: Greg KH, James Bottomley; +Cc: USB list, SCSI development list
This patch (as1050) adds a new field to struct scsi_device, to keep a
count of the number of block-device open file references. This count
will be used by usb-storage to determine whether USB-PERSIST should be
forced on during a suspend.
Signed-off-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
This patch is being submitted through the USB tree rather than the SCSI
tree because it is part of a series of USB patches.
Index: usb-2.6/drivers/scsi/sd.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sd.c
+++ usb-2.6/drivers/scsi/sd.c
@@ -611,6 +611,7 @@ static int sd_open(struct inode *inode,
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
}
+ atomic_inc(&sdev->blockdev_open_cnt);
return 0;
error_out:
@@ -637,6 +638,8 @@ static int sd_release(struct inode *inod
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_release\n"));
+ atomic_dec(&sdev->blockdev_open_cnt);
+
if (!--sdkp->openers && sdev->removable) {
if (scsi_block_when_processing_errors(sdev))
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
Index: usb-2.6/drivers/scsi/sr.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sr.c
+++ usb-2.6/drivers/scsi/sr.c
@@ -564,6 +564,7 @@ static int sr_open(struct cdrom_device_i
if (!scsi_block_when_processing_errors(sdev))
goto error_out;
+ atomic_inc(&sdev->blockdev_open_cnt);
return 0;
error_out:
@@ -573,10 +574,12 @@ error_out:
static void sr_release(struct cdrom_device_info *cdi)
{
struct scsi_cd *cd = cdi->handle;
+ struct scsi_device *sdev = cd->device;
+
+ atomic_dec(&sdev->blockdev_open_cnt);
if (cd->device->sector_size > 2048)
sr_set_blocklength(cd, 2048);
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803031508490.7094-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-03 22:01 ` James Bottomley
2008-03-03 23:04 ` Alan Stern
2008-03-05 20:13 ` patch scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch added to gregkh-2.6 tree gregkh-l3A5Bk7waGM
1 sibling, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-03 22:01 UTC (permalink / raw
To: Alan Stern; +Cc: Greg KH, USB list, SCSI development list
On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> This patch (as1050) adds a new field to struct scsi_device, to keep a
> count of the number of block-device open file references. This count
> will be used by usb-storage to determine whether USB-PERSIST should be
> forced on during a suspend.
I don't think this does what's advertised if you mean it to keep a count
as the atomics seem to imply. (->open is only called on first open and
->release on last close. openers was a historical 2.4 field that used
to count opens but now just flags whether sd is open or not.
Secondly, if you really want an openers count (which I remember SCSI
rendering a bad idea ages ago), that's held in struct block_device as
openers, isn't it? So there's no need to duplicate this in SCSI.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-03 22:01 ` James Bottomley
@ 2008-03-03 23:04 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803031758310.8280-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-04 0:37 ` Stefan Richter
0 siblings, 2 replies; 26+ messages in thread
From: Alan Stern @ 2008-03-03 23:04 UTC (permalink / raw
To: James Bottomley; +Cc: Greg KH, USB list, SCSI development list
On Mon, 3 Mar 2008, James Bottomley wrote:
> On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> > This patch (as1050) adds a new field to struct scsi_device, to keep a
> > count of the number of block-device open file references. This count
> > will be used by usb-storage to determine whether USB-PERSIST should be
> > forced on during a suspend.
>
> I don't think this does what's advertised if you mean it to keep a count
> as the atomics seem to imply. (->open is only called on first open and
> ->release on last close. openers was a historical 2.4 field that used
> to count opens but now just flags whether sd is open or not.
Um. The real purpose of that field is to let usb-storage know whether
or not the device is open at all. I used a count because that seemed
like the only way to do it; it doesn't matter if the count value
doesn't exactly match the actual number of open file references.
I didn't want to use the openers field in struct scsi_disk because this
should apply to CDs and any other block devices as well as to disks.
> Secondly, if you really want an openers count (which I remember SCSI
> rendering a bad idea ages ago), that's held in struct block_device as
> openers, isn't it? So there's no need to duplicate this in SCSI.
But there's no way to get to the block_device structure if all you're
given is the scsi_device, right? You would have to know something
special about it, such as whether it is a disk, a CD, or something
else. This is intended to be generic.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803031758310.8280-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-03 23:13 ` James Bottomley
[not found] ` <1204585995.3043.85.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-03 23:13 UTC (permalink / raw
To: Alan Stern; +Cc: Greg KH, USB list, SCSI development list
On Mon, 2008-03-03 at 18:04 -0500, Alan Stern wrote:
> On Mon, 3 Mar 2008, James Bottomley wrote:
>
> > On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> > > This patch (as1050) adds a new field to struct scsi_device, to keep a
> > > count of the number of block-device open file references. This count
> > > will be used by usb-storage to determine whether USB-PERSIST should be
> > > forced on during a suspend.
> >
> > I don't think this does what's advertised if you mean it to keep a count
> > as the atomics seem to imply. (->open is only called on first open and
> > ->release on last close. openers was a historical 2.4 field that used
> > to count opens but now just flags whether sd is open or not.
>
> Um. The real purpose of that field is to let usb-storage know whether
> or not the device is open at all. I used a count because that seemed
> like the only way to do it; it doesn't matter if the count value
> doesn't exactly match the actual number of open file references.
OK, so your change log is "whether the device is open" not "a count of
openers".
> I didn't want to use the openers field in struct scsi_disk because this
> should apply to CDs and any other block devices as well as to disks.
>
> > Secondly, if you really want an openers count (which I remember SCSI
> > rendering a bad idea ages ago), that's held in struct block_device as
> > openers, isn't it? So there's no need to duplicate this in SCSI.
>
> But there's no way to get to the block_device structure if all you're
> given is the scsi_device, right? You would have to know something
> special about it, such as whether it is a disk, a CD, or something
> else. This is intended to be generic.
OK, but then the obvious question is why this isn't at the block device
layer? It doesn't seem right that we'd have a shadow (and not a very
effective one) of bdev->openers just so you can access it from a
scsi_device. We had a similar problem with the AHCI AN patches which
were solved by altering the layered accesses.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-03 23:04 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803031758310.8280-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-04 0:37 ` Stefan Richter
2008-03-04 17:00 ` Alan Stern
1 sibling, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2008-03-04 0:37 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
>> On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
>>> This count
>>> will be used by usb-storage to determine whether USB-PERSIST should be
>>> forced on during a suspend.
Why should devices without openers be treated differently from devices
with openers?
--
Stefan Richter
-=====-==--- --== --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <1204585995.3043.85.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2008-03-04 16:16 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803041101200.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-04 16:52 ` James Bottomley
0 siblings, 2 replies; 26+ messages in thread
From: Alan Stern @ 2008-03-04 16:16 UTC (permalink / raw
To: James Bottomley; +Cc: Greg KH, USB list, SCSI development list
On Mon, 3 Mar 2008, James Bottomley wrote:
> On Mon, 2008-03-03 at 18:04 -0500, Alan Stern wrote:
> > On Mon, 3 Mar 2008, James Bottomley wrote:
> >
> > > On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> > > > This patch (as1050) adds a new field to struct scsi_device, to keep a
> > > > count of the number of block-device open file references. This count
> > > > will be used by usb-storage to determine whether USB-PERSIST should be
> > > > forced on during a suspend.
> > >
> > > I don't think this does what's advertised if you mean it to keep a count
> > > as the atomics seem to imply. (->open is only called on first open and
> > > ->release on last close. openers was a historical 2.4 field that used
> > > to count opens but now just flags whether sd is open or not.
> >
> > Um. The real purpose of that field is to let usb-storage know whether
> > or not the device is open at all. I used a count because that seemed
> > like the only way to do it; it doesn't matter if the count value
> > doesn't exactly match the actual number of open file references.
>
> OK, so your change log is "whether the device is open" not "a count of
> openers".
Yes, sort of. That is, the new field is used just for its "whether the
device is open" meaning, but the only way to keep track of that is to
count the calls to the open and release methods.
BTW, is it really accurate to say that this isn't a count of the
openers? I guess it depends on what you mean by "opener". IIUC, every
system call to open(2) will go through the driver's open method and
thus increment the count, whereas calls to dup(2) or dup2(2) will not.
It that correct, or is there a stronger distinction at work?
> > > Secondly, if you really want an openers count (which I remember SCSI
> > > rendering a bad idea ages ago), that's held in struct block_device as
> > > openers, isn't it? So there's no need to duplicate this in SCSI.
> >
> > But there's no way to get to the block_device structure if all you're
> > given is the scsi_device, right? You would have to know something
> > special about it, such as whether it is a disk, a CD, or something
> > else. This is intended to be generic.
>
> OK, but then the obvious question is why this isn't at the block device
> layer? It doesn't seem right that we'd have a shadow (and not a very
> effective one) of bdev->openers just so you can access it from a
> scsi_device. We had a similar problem with the AHCI AN patches which
> were solved by altering the layered accesses.
Well then, how would you solve the problem? Given a pointer to the
scsi_host, we need to know whether there are any block-device files
open for any of the SCSI devices beneath it. Ideally we would like to
know strictly if there are any exclusive opens, but merely knowing
about any kind of open device file would be good enough.
If you can suggest an alternative approach, I'll be glad to change the
patch.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803041101200.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-04 16:50 ` Mike Anderson
0 siblings, 0 replies; 26+ messages in thread
From: Mike Anderson @ 2008-03-04 16:50 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> On Mon, 3 Mar 2008, James Bottomley wrote:
>
> > On Mon, 2008-03-03 at 18:04 -0500, Alan Stern wrote:
> > > On Mon, 3 Mar 2008, James Bottomley wrote:
> > >
> > > > On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> > > > > This patch (as1050) adds a new field to struct scsi_device, to keep a
> > > > > count of the number of block-device open file references. This count
> > > > > will be used by usb-storage to determine whether USB-PERSIST should be
> > > > > forced on during a suspend.
> > > >
> > > > I don't think this does what's advertised if you mean it to keep a count
> > > > as the atomics seem to imply. (->open is only called on first open and
> > > > ->release on last close. openers was a historical 2.4 field that used
> > > > to count opens but now just flags whether sd is open or not.
> > >
> > > Um. The real purpose of that field is to let usb-storage know whether
> > > or not the device is open at all. I used a count because that seemed
> > > like the only way to do it; it doesn't matter if the count value
> > > doesn't exactly match the actual number of open file references.
> >
> > OK, so your change log is "whether the device is open" not "a count of
> > openers".
>
> Yes, sort of. That is, the new field is used just for its "whether the
> device is open" meaning, but the only way to keep track of that is to
> count the calls to the open and release methods.
>
> BTW, is it really accurate to say that this isn't a count of the
> openers? I guess it depends on what you mean by "opener". IIUC, every
> system call to open(2) will go through the driver's open method and
> thus increment the count, whereas calls to dup(2) or dup2(2) will not.
> It that correct, or is there a stronger distinction at work?
>
In do_open there is a different path taken if bd_openers is non zero.
> > > > Secondly, if you really want an openers count (which I remember SCSI
> > > > rendering a bad idea ages ago), that's held in struct block_device as
> > > > openers, isn't it? So there's no need to duplicate this in SCSI.
> > >
> > > But there's no way to get to the block_device structure if all you're
> > > given is the scsi_device, right? You would have to know something
> > > special about it, such as whether it is a disk, a CD, or something
> > > else. This is intended to be generic.
> >
> > OK, but then the obvious question is why this isn't at the block device
> > layer? It doesn't seem right that we'd have a shadow (and not a very
> > effective one) of bdev->openers just so you can access it from a
> > scsi_device. We had a similar problem with the AHCI AN patches which
> > were solved by altering the layered accesses.
>
> Well then, how would you solve the problem? Given a pointer to the
> scsi_host, we need to know whether there are any block-device files
> open for any of the SCSI devices beneath it. Ideally we would like to
> know strictly if there are any exclusive opens, but merely knowing
> about any kind of open device file would be good enough.
>
> If you can suggest an alternative approach, I'll be glad to change the
> patch.
The field that you are adding is very close in behavior to
(sdev->sdev_gendev.kobj.kref.refcount). Though sd_open increments the sdev
kref through a call chain started by a call to scsi_disk_get the sdev kref
would cover more than block opens which may effect the usefulness in your
case.
-andmike
--
Michael Anderson
andmike-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-04 16:16 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803041101200.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-04 16:52 ` James Bottomley
[not found] ` <1204649571.3091.36.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
1 sibling, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-04 16:52 UTC (permalink / raw
To: Alan Stern; +Cc: Greg KH, USB list, SCSI development list
On Tue, 2008-03-04 at 11:16 -0500, Alan Stern wrote:
> On Mon, 3 Mar 2008, James Bottomley wrote:
>
> > On Mon, 2008-03-03 at 18:04 -0500, Alan Stern wrote:
> > > On Mon, 3 Mar 2008, James Bottomley wrote:
> > >
> > > > On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> > > > > This patch (as1050) adds a new field to struct scsi_device, to keep a
> > > > > count of the number of block-device open file references. This count
> > > > > will be used by usb-storage to determine whether USB-PERSIST should be
> > > > > forced on during a suspend.
> > > >
> > > > I don't think this does what's advertised if you mean it to keep a count
> > > > as the atomics seem to imply. (->open is only called on first open and
> > > > ->release on last close. openers was a historical 2.4 field that used
> > > > to count opens but now just flags whether sd is open or not.
> > >
> > > Um. The real purpose of that field is to let usb-storage know whether
> > > or not the device is open at all. I used a count because that seemed
> > > like the only way to do it; it doesn't matter if the count value
> > > doesn't exactly match the actual number of open file references.
> >
> > OK, so your change log is "whether the device is open" not "a count of
> > openers".
>
> Yes, sort of. That is, the new field is used just for its "whether the
> device is open" meaning, but the only way to keep track of that is to
> count the calls to the open and release methods.
>
> BTW, is it really accurate to say that this isn't a count of the
> openers? I guess it depends on what you mean by "opener". IIUC, every
> system call to open(2) will go through the driver's open method and
> thus increment the count, whereas calls to dup(2) or dup2(2) will not.
> It that correct, or is there a stronger distinction at work?
I thought I already explained that it didn't in the previous email, in
the bit quoted above.
---
> > > > > I don't think this does what's advertised if you mean it to keep a count
> > > > > as the atomics seem to imply. (->open is only called on first open and
> > > > > ->release on last close. openers was a historical 2.4 field that used
> > > > > to count opens but now just flags whether sd is open or not.
---
If you want to see for yourself, it's fs/block_dev.c:do_open().
> > > > Secondly, if you really want an openers count (which I remember SCSI
> > > > rendering a bad idea ages ago), that's held in struct block_device as
> > > > openers, isn't it? So there's no need to duplicate this in SCSI.
> > >
> > > But there's no way to get to the block_device structure if all you're
> > > given is the scsi_device, right? You would have to know something
> > > special about it, such as whether it is a disk, a CD, or something
> > > else. This is intended to be generic.
> >
> > OK, but then the obvious question is why this isn't at the block device
> > layer? It doesn't seem right that we'd have a shadow (and not a very
> > effective one) of bdev->openers just so you can access it from a
> > scsi_device. We had a similar problem with the AHCI AN patches which
> > were solved by altering the layered accesses.
>
> Well then, how would you solve the problem? Given a pointer to the
> scsi_host, we need to know whether there are any block-device files
> open for any of the SCSI devices beneath it. Ideally we would like to
> know strictly if there are any exclusive opens, but merely knowing
> about any kind of open device file would be good enough.
OK, but this is the problem. Shadowing information held at a higher
layer because you can't get at that layer sounds like some type of
layering problem in the code.
The exclusive open information is also held at the block device
structure in the bd_holder/bd_holders fields.
> If you can suggest an alternative approach, I'll be glad to change the
> patch.
Well, since this is the 7th patch of an eight patch series, I don't
really have the context for that. However, I assume it's all on the
linux-usb list? If so, I'll add it to my list of things to look at.
James
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-04 0:37 ` Stefan Richter
@ 2008-03-04 17:00 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803041151510.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-04 17:00 UTC (permalink / raw
To: Stefan Richter; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
On Tue, 4 Mar 2008, Stefan Richter wrote:
> >> On Mon, 2008-03-03 at 15:16 -0500, Alan Stern wrote:
> >>> This count
> >>> will be used by usb-storage to determine whether USB-PERSIST should be
> >>> forced on during a suspend.
>
> Why should devices without openers be treated differently from devices
> with openers?
It's not the openers themselves that matter; it's whether or not the
device contains a mounted filesystem. I'm using the openers as a proxy
for that. If you can suggest a better approach, I'd like to hear it.
Devices containing a mounted filesystem need special treatment to avoid
being disconnected when power is lost during a system suspend or
hibernation. In principle this special treatment could be extended to
all USB devices, whether mounted or not. However the USB spec says not
to do it.
Really it's a tradeoff. If power goes out during a suspend and a
device with a mounted filesystem is disconnected as a result, there's a
very high risk of data loss. Conversely, if the user replaces the
medium while the system is asleep and the kernel doesn't realize it,
then upon waking the data on the new medium will be corrupted.
After suffering from exactly this sort of data loss, Linus decreed that
the kernel should automatically avoid disconnecting devices with
mounted filesystems, even though it contradicts the spec. So I need a
way to know whether anything is mounted on a device below a SCSI host.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803041151510.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-04 17:26 ` Stefan Richter
2008-03-04 17:56 ` Stefan Richter
1 sibling, 0 replies; 26+ messages in thread
From: Stefan Richter @ 2008-03-04 17:26 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern wrote:
> On Tue, 4 Mar 2008, Stefan Richter wrote:
>> Why should devices without openers be treated differently from devices
>> with openers?
...
> In principle this special treatment could be extended to
> all USB devices, whether mounted or not. However the USB spec says not
> to do it.
Ah, so that's how it is. Thanks.
--
Stefan Richter
-=====-==--- --== --=--
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <1204649571.3091.36.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2008-03-04 17:43 ` Alan Stern
2008-03-05 20:55 ` James Bottomley
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-04 17:43 UTC (permalink / raw
To: James Bottomley; +Cc: Greg KH, USB list, SCSI development list
On Tue, 4 Mar 2008, James Bottomley wrote:
> If you want to see for yourself, it's fs/block_dev.c:do_open().
Thanks for the pointer. I see the subroutine is not exactly loaded
with comments...
> > If you can suggest an alternative approach, I'll be glad to change the
> > patch.
>
> Well, since this is the 7th patch of an eight patch series, I don't
> really have the context for that. However, I assume it's all on the
> linux-usb list? If so, I'll add it to my list of things to look at.
The patches before it probably won't give you any additional useful
context. The following patch might; that's the one affecting
usb-storage.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803041151510.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-04 17:26 ` Stefan Richter
@ 2008-03-04 17:56 ` Stefan Richter
2008-03-04 20:04 ` Alan Stern
2008-03-05 18:04 ` Alan Stern
1 sibling, 2 replies; 26+ messages in thread
From: Stefan Richter @ 2008-03-04 17:56 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern wrote:
> So I need a
> way to know whether anything is mounted on a device below a SCSI host.
Actually I have been thinking of something related for a while but
didn't send a patch yet. Consider this:
[include/scsi/scsi_host.h]
struct scsi_host_template {
...
+ int (* slave_sdev_get)(struct scsi_device *);
+ void (* slave_sdev_put)(struct scsi_device *);
...
}
[drivers/scsi/scsi.c]
int scsi_device_get(struct scsi_device *sdev)
{
+ int err = 0;
+
if (sdev->sdev_state == SDEV_DEL)
return -ENXIO;
if (!get_device(&sdev->sdev_gendev))
return -ENXIO;
/* We can fail this if we're doing SCSI operations
* from module exit (like cache flush) */
try_module_get(sdev->host->hostt->module);
- return 0;
+ if (sdev->host->hostt->slave_sdev_get)
+ err = sdev->host->hostt->slave_sdev_get(sdev);
+ if (err) {
+ ...some ifdef'd module_put stuff...;
+ put_device(&sdev->sdev_gendev);
+ }
+ return err;
}
and equivalent additions to scsi_device_put().
Example usage:
[drivers/ieee1394/sbp2.c]
+int sbp2_slave_sdev_get(struct scsi_device *sdev)
+{
+ struct sbp2_lu *lu = (struct sbp2_lu *)sdev->host->hostdata[0];
+
+ if (!try_module_get(lu->hi->host->driver->owner))
+ return -ENOMEM;
+ return 0;
+}
and an equivalent sbp2_slave_sdev_put(). These examples would prevent
unloading ohci1394 while somebody has mounted a filesystem on a device
driven by sbp2, or otherwise opened or referenced that device. (At the
moment sbp2 unintelligently gets the host->driver->owner as long as it
is logged in into a device. This is somewhat necessary because there
are unrelated drivers in the ieee1394 stack which when unloaded with
modprobe would cause ohci1394 to be unloaded too.)
You could use slave_sdev_get()/ slave_sdev_put() callbacks in
usb-storage to do an own bookkeeping of references there.
(Since I haven't looked at your other current work, all of the above may
be utterly besides the point...)
--
Stefan Richter
-=====-==--- --== --=--
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-04 17:56 ` Stefan Richter
@ 2008-03-04 20:04 ` Alan Stern
2008-03-05 18:04 ` Alan Stern
1 sibling, 0 replies; 26+ messages in thread
From: Alan Stern @ 2008-03-04 20:04 UTC (permalink / raw
To: Stefan Richter; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
On Tue, 4 Mar 2008, Stefan Richter wrote:
> Alan Stern wrote:
> > So I need a
> > way to know whether anything is mounted on a device below a SCSI host.
>
> Actually I have been thinking of something related for a while but
> didn't send a patch yet. Consider this:
>
> [include/scsi/scsi_host.h]
>
> struct scsi_host_template {
> ...
> + int (* slave_sdev_get)(struct scsi_device *);
> + void (* slave_sdev_put)(struct scsi_device *);
> ...
> }
>
> [drivers/scsi/scsi.c]
>
> int scsi_device_get(struct scsi_device *sdev)
> {
> + int err = 0;
> +
> if (sdev->sdev_state == SDEV_DEL)
> return -ENXIO;
> if (!get_device(&sdev->sdev_gendev))
> return -ENXIO;
> /* We can fail this if we're doing SCSI operations
> * from module exit (like cache flush) */
> try_module_get(sdev->host->hostt->module);
>
> - return 0;
> + if (sdev->host->hostt->slave_sdev_get)
> + err = sdev->host->hostt->slave_sdev_get(sdev);
> + if (err) {
> + ...some ifdef'd module_put stuff...;
> + put_device(&sdev->sdev_gendev);
> + }
> + return err;
> }
>
> and equivalent additions to scsi_device_put().
That would be okay. It does a little more than I need, but the
difference doesn't matter.
> Example usage:
>
> [drivers/ieee1394/sbp2.c]
>
> +int sbp2_slave_sdev_get(struct scsi_device *sdev)
> +{
> + struct sbp2_lu *lu = (struct sbp2_lu *)sdev->host->hostdata[0];
> +
> + if (!try_module_get(lu->hi->host->driver->owner))
> + return -ENOMEM;
> + return 0;
> +}
>
> and an equivalent sbp2_slave_sdev_put(). These examples would prevent
> unloading ohci1394 while somebody has mounted a filesystem on a device
> driven by sbp2, or otherwise opened or referenced that device. (At the
> moment sbp2 unintelligently gets the host->driver->owner as long as it
> is logged in into a device. This is somewhat necessary because there
> are unrelated drivers in the ieee1394 stack which when unloaded with
> modprobe would cause ohci1394 to be unloaded too.)
>
> You could use slave_sdev_get()/ slave_sdev_put() callbacks in
> usb-storage to do an own bookkeeping of references there.
>
> (Since I haven't looked at your other current work, all of the above may
> be utterly besides the point...)
It's a good idea. I'd be willing to use it, if you submit it.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-04 17:56 ` Stefan Richter
2008-03-04 20:04 ` Alan Stern
@ 2008-03-05 18:04 ` Alan Stern
2008-03-05 19:28 ` Stefan Richter
1 sibling, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-05 18:04 UTC (permalink / raw
To: Stefan Richter; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
On Tue, 4 Mar 2008, Stefan Richter wrote:
> Alan Stern wrote:
> > So I need a
> > way to know whether anything is mounted on a device below a SCSI host.
>
> Actually I have been thinking of something related for a while but
> didn't send a patch yet. Consider this:
>
> [include/scsi/scsi_host.h]
>
> struct scsi_host_template {
> ...
> + int (* slave_sdev_get)(struct scsi_device *);
> + void (* slave_sdev_put)(struct scsi_device *);
> ...
> }
>
> [drivers/scsi/scsi.c]
>
> int scsi_device_get(struct scsi_device *sdev)
> {
> + int err = 0;
> +
> if (sdev->sdev_state == SDEV_DEL)
> return -ENXIO;
> if (!get_device(&sdev->sdev_gendev))
> return -ENXIO;
> /* We can fail this if we're doing SCSI operations
> * from module exit (like cache flush) */
> try_module_get(sdev->host->hostt->module);
>
> - return 0;
> + if (sdev->host->hostt->slave_sdev_get)
> + err = sdev->host->hostt->slave_sdev_get(sdev);
> + if (err) {
> + ...some ifdef'd module_put stuff...;
> + put_device(&sdev->sdev_gendev);
> + }
> + return err;
> }
>
> and equivalent additions to scsi_device_put().
On second thought, I'm not so sure this would work the way I want.
scsi_device_get() ends up being called for all sorts of extra things
unrelated to device opens. For instance, scsi_probe_and_add_lun()
calls it whenever a new LUN is discovered.
Would it be acceptable to add a similar pair of callbacks to the host
template, invoked by the upper-level driver's open and release
routines?
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 18:04 ` Alan Stern
@ 2008-03-05 19:28 ` Stefan Richter
2008-03-05 20:34 ` Alan Stern
0 siblings, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2008-03-05 19:28 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern wrote:
> On Tue, 4 Mar 2008, Stefan Richter wrote:
>> [drivers/scsi/scsi.c]
>>
>> int scsi_device_get(struct scsi_device *sdev)
>> {
...
>> + if (sdev->host->hostt->slave_sdev_get)
>> + err = sdev->host->hostt->slave_sdev_get(sdev);
...
>> and equivalent additions to scsi_device_put().
>
> On second thought, I'm not so sure this would work the way I want.
> scsi_device_get() ends up being called for all sorts of extra things
> unrelated to device opens. For instance, scsi_probe_and_add_lun()
> calls it whenever a new LUN is discovered.
True.
> Would it be acceptable to add a similar pair of callbacks to the host
> template, invoked by the upper-level driver's open and release
> routines?
This would actually be nice for that sbp2 thing which I mentioned as well.
--
Stefan Richter
-=====-==--- --== --=-=
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 26+ messages in thread
* patch scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch added to gregkh-2.6 tree
[not found] ` <Pine.LNX.4.44L0.0803031508490.7094-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-03 22:01 ` James Bottomley
@ 2008-03-05 20:13 ` gregkh-l3A5Bk7waGM
[not found] ` <1204748015695-4DuetyUvsWfYtjvyW6yDsg@public.gmane.org>
1 sibling, 1 reply; 26+ messages in thread
From: gregkh-l3A5Bk7waGM @ 2008-03-05 20:13 UTC (permalink / raw
To: stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
James.Bottomley-lYEaACU144FWk0Htik3J/w,
greg-U8xfFu+wG4EAvxtiuMwx3w, gregkh-l3A5Bk7waGM,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
This is a note to let you know that I've just added the patch titled
Subject: SCSI: add a field to scsi_device to count open file references
to my gregkh-2.6 tree. Its filename is
scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch
This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
>From stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org Wed Mar 5 12:05:47 2008
From: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Date: Mon, 3 Mar 2008 15:16:16 -0500 (EST)
Subject: SCSI: add a field to scsi_device to count open file references
To: Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>, James Bottomley <James.Bottomley-lYEaACU144FWk0Htik3J/w@public.gmane.org>
Cc: USB list <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, SCSI development list <linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Message-ID: <Pine.LNX.4.44L0.0803031508490.7094-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
This patch (as1050) adds a new field to struct scsi_device, to keep a
count of the number of block-device open file references. This count
will be used by usb-storage to determine whether USB-PERSIST should be
forced on during a suspend.
Signed-off-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Cc: James Bottomley <James.Bottomley-lYEaACU144FWk0Htik3J/w@public.gmane.org>
Signed-off-by: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
---
drivers/scsi/sd.c | 3 +++
drivers/scsi/sr.c | 5 ++++-
include/scsi/scsi_device.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -610,6 +610,7 @@ static int sd_open(struct inode *inode,
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
}
+ atomic_inc(&sdev->blockdev_open_cnt);
return 0;
error_out:
@@ -636,6 +637,8 @@ static int sd_release(struct inode *inod
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_release\n"));
+ atomic_dec(&sdev->blockdev_open_cnt);
+
if (!--sdkp->openers && sdev->removable) {
if (scsi_block_when_processing_errors(sdev))
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -564,6 +564,7 @@ static int sr_open(struct cdrom_device_i
if (!scsi_block_when_processing_errors(sdev))
goto error_out;
+ atomic_inc(&sdev->blockdev_open_cnt);
return 0;
error_out:
@@ -573,10 +574,12 @@ error_out:
static void sr_release(struct cdrom_device_info *cdi)
{
struct scsi_cd *cd = cdi->handle;
+ struct scsi_device *sdev = cd->device;
+
+ atomic_dec(&sdev->blockdev_open_cnt);
if (cd->device->sector_size > 2048)
sr_set_blocklength(cd, 2048);
-
}
static int sr_probe(struct device *dev)
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -153,6 +153,7 @@ struct scsi_device {
atomic_t iorequest_cnt;
atomic_t iodone_cnt;
atomic_t ioerr_cnt;
+ atomic_t blockdev_open_cnt;
int timeout;
Patches currently in gregkh-2.6 which might be from stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org are
usb/usb-convert-usb.h-struct-usb_device-to-kernel-doc.patch
usb/usb-make-usb_storage_onetouch-available-with-pm.patch
usb/usb-usb-ohci-sm501-driver-use-the-conventional-convention-for-suspend-and-resume.patch
usb/usb-reorganize-code-in-hub.c.patch
usb/usb-ehci-carry-out-port-handover-during-each-root-hub-resume.patch
usb/scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch
usb/usb-allow-drivers-to-force-usb-persist.patch
usb/usb-remove-config_usb_persist-setting.patch
usb/usb-check-serial-number-string-after-device-reset.patch
usb/usb-make-usb-persist-work-after-every-system-sleep.patch
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 19:28 ` Stefan Richter
@ 2008-03-05 20:34 ` Alan Stern
2008-03-05 21:14 ` Stefan Richter
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-05 20:34 UTC (permalink / raw
To: Stefan Richter; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
On Wed, 5 Mar 2008, Stefan Richter wrote:
> > Would it be acceptable to add a similar pair of callbacks to the host
> > template, invoked by the upper-level driver's open and release
> > routines?
>
> This would actually be nice for that sbp2 thing which I mentioned as well.
James and Stefan, what do you think of this patch?
(Unfortunately it won't quite apply as is, because it touches a region
of documentation that was changed by the SCSI dynamic power management
patch.)
Alan Stern
--------------------------------------------------------------------
This patch (as1050b) adds two new methods to the SCSI host template
structure. These methods are called to notify lower-level drivers
when a block device below one of their hosts has been opened or
released.
This information is useful for host drivers that want to know whether
a device on one of its buses contains a mounted filesystem. The
open/release information isn't quite the same as mount/unmount, but it
will suffice.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
---
Index: usb-2.6/Documentation/scsi/scsi_mid_low_api.txt
===================================================================
--- usb-2.6.orig/Documentation/scsi/scsi_mid_low_api.txt
+++ usb-2.6/Documentation/scsi/scsi_mid_low_api.txt
@@ -785,6 +785,8 @@ Summary:
autoresume - perform dynamic (runtime) host resume
autosuspend - perform dynamic (runtime) host suspend
bios_param - fetch head, sector, cylinder info for a disk
+ blockdev_open - notify the host about a block-device open
+ blockdev_release - notify the host about a block-device release
detect - detects HBAs this driver wants to control
eh_timed_out - notify the host that a command timer expired
eh_abort_handler - abort given command
@@ -882,6 +884,48 @@ Details:
/**
+ * blockdev_open - notify the host about a block-device open
+ * @sdev: the SCSI device being opened
+ * @disk: the gendisk corresponding to the block device
+ *
+ * Block device opens occur primarily when a filesystem on the
+ * device is mounted (although they crop up occasionally at other
+ * times, such as when a partition table is edited). An LLD
+ * can define this function and blockdev_release below to keep
+ * track of whether or not a device below one of its hosts has
+ * been mounted.
+ *
+ * Locks: none
+ *
+ * Calling context: process
+ *
+ * Optionally defined in: LLD
+ **/
+ void blockdev_open(struct scsi_device *sdev, struct gendisk *disk)
+
+
+/**
+ * blockdev_release - notify the host about a block-device release
+ * @sdev: the SCSI device being released
+ * @disk: the gendisk corresponding to the block device
+ *
+ * Block device releases occur primarily when a filesystem on the
+ * device is unmounted (although they crop up occasionally at other
+ * times, such as when a partition table is edited). An LLD
+ * can define this function and blockdev_open above to keep
+ * track of whether or not a device below one of its hosts has
+ * been mounted.
+ *
+ * Locks: none
+ *
+ * Calling context: process
+ *
+ * Optionally defined in: LLD
+ **/
+ void blockdev_release(struct scsi_device *sdev, struct gendisk *disk)
+
+
+/**
* detect - detects HBAs this driver wants to control
* @shtp: host template for this driver.
*
Index: usb-2.6/include/scsi/scsi_host.h
===================================================================
--- usb-2.6.orig/include/scsi/scsi_host.h
+++ usb-2.6/include/scsi/scsi_host.h
@@ -306,6 +306,27 @@ struct scsi_host_template {
void (* scan_start)(struct Scsi_Host *);
/*
+ * If the host driver wants to know about block-device open
+ * calls, it can fill in this function. Note that these calls
+ * are made only for the first opener, not for opens that occur
+ * when the device is already open.
+ *
+ * Status: OPTIONAL
+ */
+ void (* blockdev_open)(struct scsi_device *, struct gendisk *);
+
+ /*
+ * If the host driver wants to know about block-device release
+ * calls, it can fill in this function. Note that these calls
+ * are made only for the last closer, not for closes that leave
+ * the device still open (i.e., these exactly balance the
+ * block-device open calls).
+ *
+ * Status: OPTIONAL
+ */
+ void (* blockdev_release)(struct scsi_device *, struct gendisk *);
+
+ /*
* Fill in this function to allow the queue depth of this host
* to be changeable (on a per device basis). Returns either
* the current queue depth setting (may be different from what
Index: usb-2.6/drivers/scsi/sd.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sd.c
+++ usb-2.6/drivers/scsi/sd.c
@@ -606,6 +606,9 @@ static int sd_open(struct inode *inode,
if (!scsi_device_online(sdev))
goto error_out;
+ if (sdev->host->hostt->blockdev_open)
+ (sdev->host->hostt->blockdev_open)(sdev, disk);
+
if (!sdkp->openers++ && sdev->removable) {
if (scsi_block_when_processing_errors(sdev))
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
@@ -642,6 +645,9 @@ static int sd_release(struct inode *inod
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
}
+ if (sdev->host->hostt->blockdev_release)
+ (sdev->host->hostt->blockdev_release)(sdev, disk);
+
/*
* XXX and what if there are packets in flight and this close()
* XXX is followed by a "rmmod sd_mod"?
Index: usb-2.6/drivers/scsi/sr.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sr.c
+++ usb-2.6/drivers/scsi/sr.c
@@ -473,13 +473,17 @@ static int sr_block_open(struct inode *i
{
struct gendisk *disk = inode->i_bdev->bd_disk;
struct scsi_cd *cd;
+ struct scsi_device *sdev;
int ret = 0;
if(!(cd = scsi_cd_get(disk)))
return -ENXIO;
+ sdev = cd->device;
if((ret = cdrom_open(&cd->cdi, inode, file)) != 0)
scsi_cd_put(cd);
+ else if (sdev->host->hostt->blockdev_open)
+ (sdev->host->hostt->blockdev_open)(sdev, disk);
return ret;
}
@@ -487,11 +491,16 @@ static int sr_block_open(struct inode *i
static int sr_block_release(struct inode *inode, struct file *file)
{
int ret;
- struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
+ struct gendisk *disk = inode->i_bdev->bd_disk;
+ struct scsi_cd *cd = scsi_cd(disk);
+ struct scsi_device *sdev = cd->device;
+
ret = cdrom_release(&cd->cdi, file);
if(ret)
return ret;
-
+
+ if (sdev->host->hostt->blockdev_release)
+ (sdev->host->hostt->blockdev_release)(sdev, disk);
scsi_cd_put(cd);
return 0;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: patch scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch added to gregkh-2.6 tree
[not found] ` <1204748015695-4DuetyUvsWfYtjvyW6yDsg@public.gmane.org>
@ 2008-03-05 20:36 ` James Bottomley
[not found] ` <1204749410.3047.57.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-05 20:36 UTC (permalink / raw
To: gregkh-l3A5Bk7waGM
Cc: stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
greg-U8xfFu+wG4EAvxtiuMwx3w, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
On Wed, 2008-03-05 at 12:13 -0800, gregkh-l3A5Bk7waGM@public.gmane.org wrote:
> This is a note to let you know that I've just added the patch titled
>
> Subject: SCSI: add a field to scsi_device to count open file references
>
> to my gregkh-2.6 tree. Its filename is
>
> scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch
>
> This tree can be found at
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
Could you un add it, please ... as I've explained several times,
shadowing bdev->bd_openers isn't the right thing to do in SCSI.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: patch scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch added to gregkh-2.6 tree
[not found] ` <1204749410.3047.57.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2008-03-05 20:41 ` Greg KH
0 siblings, 0 replies; 26+ messages in thread
From: Greg KH @ 2008-03-05 20:41 UTC (permalink / raw
To: James Bottomley
Cc: stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
greg-U8xfFu+wG4EAvxtiuMwx3w, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
On Wed, Mar 05, 2008 at 02:36:50PM -0600, James Bottomley wrote:
> On Wed, 2008-03-05 at 12:13 -0800, gregkh-l3A5Bk7waGM@public.gmane.org wrote:
> > This is a note to let you know that I've just added the patch titled
> >
> > Subject: SCSI: add a field to scsi_device to count open file references
> >
> > to my gregkh-2.6 tree. Its filename is
> >
> > scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch
> >
> > This tree can be found at
> > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
>
> Could you un add it, please ... as I've explained several times,
> shadowing bdev->bd_openers isn't the right thing to do in SCSI.
Ok, I wanted to at least test this stuff out. As the follow-on patch
doesn't compile yet, I'll drop this one as well and let Alan send
updates after this gets all hashed out.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-04 17:43 ` Alan Stern
@ 2008-03-05 20:55 ` James Bottomley
2008-03-05 21:24 ` Alan Stern
0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-05 20:55 UTC (permalink / raw
To: Alan Stern; +Cc: Greg KH, USB list, SCSI development list
On Tue, 2008-03-04 at 12:43 -0500, Alan Stern wrote:
> On Tue, 4 Mar 2008, James Bottomley wrote:
>
> > If you want to see for yourself, it's fs/block_dev.c:do_open().
>
> Thanks for the pointer. I see the subroutine is not exactly loaded
> with comments...
>
> > > If you can suggest an alternative approach, I'll be glad to change the
> > > patch.
> >
> > Well, since this is the 7th patch of an eight patch series, I don't
> > really have the context for that. However, I assume it's all on the
> > linux-usb list? If so, I'll add it to my list of things to look at.
>
> The patches before it probably won't give you any additional useful
> context. The following patch might; that's the one affecting
> usb-storage.
OK, I looked through the patch series. It really looks like the
in-kernel thing is the wrong approach.
Why not export this want_persist flag via sysfs and then have hal and
udev update it when they see a filesystem on an underlying usb device
mounted (and zero it out on unmount)? That way you always have the
exactly correct state and there's no need to go fishing for information
in layers that are difficult to get to.
James
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 20:34 ` Alan Stern
@ 2008-03-05 21:14 ` Stefan Richter
[not found] ` <47CF0D2E.7000607-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2008-03-05 21:14 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern wrote:
> This patch (as1050b) adds two new methods to the SCSI host template
> structure. These methods are called to notify lower-level drivers
> when a block device below one of their hosts has been opened or
> released.
I like it and would make use of it in sbp2.
sg, st, ch could be callers as well, but I suppose you don't really need
them. And for me, sd and sr already cover 99.9% of the use cases.
--
Stefan Richter
-=====-==--- --== --=-=
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 20:55 ` James Bottomley
@ 2008-03-05 21:24 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803051608140.4161-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-05 21:24 UTC (permalink / raw
To: James Bottomley; +Cc: Linus Torvalds, Greg KH, USB list, SCSI development list
On Wed, 5 Mar 2008, James Bottomley wrote:
> OK, I looked through the patch series. It really looks like the
> in-kernel thing is the wrong approach.
>
> Why not export this want_persist flag via sysfs and then have hal and
> udev update it when they see a filesystem on an underlying usb device
> mounted (and zero it out on unmount)? That way you always have the
> exactly correct state and there's no need to go fishing for information
> in layers that are difficult to get to.
The flag (or rather, a similar one named "persist_enabled") already is
exported via sysfs.
Having hal+udev set it automatically as you describe never occurred
to me. The reason for doing all of this within the kernel is that
Linus said (off-list) that the kernel should handle this automatically.
Whether he considered userspace tools taking over the responsibility, I
can't say. (But he might object to having to wait for the tools to add
the extra functionality.)
Now, it has to be mentioned that mounted filesystems on a USB device
account for something like 90% of the objections against USB-PERSIST.
If we go ahead and enable it automatically in that case, there is
really very little reason not to enable it for every USB device,
mounted or not.
In fact, the only other important reason for not turning it on by
default (or always!) is because the USB spec says not to. By adding
this check for mounted filesystems, we are trying to adhere as closely
as possible to the spec -- but only in cases where we don't care since
it doesn't matter!
Whether to enable USB-PERSIST for mounted devices is a policy decision.
Whether that policy should be set by the kernel or by userspace is
outside my jursidiction. If you think this approach is wrong-headed
and you can convince Linus to go along, I'll be willing to drop the
last three patches in this series.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <47CF0D2E.7000607-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>
@ 2008-03-05 21:27 ` Alan Stern
2008-03-05 21:47 ` Stefan Richter
0 siblings, 1 reply; 26+ messages in thread
From: Alan Stern @ 2008-03-05 21:27 UTC (permalink / raw
To: Stefan Richter; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
On Wed, 5 Mar 2008, Stefan Richter wrote:
> Alan Stern wrote:
> > This patch (as1050b) adds two new methods to the SCSI host template
> > structure. These methods are called to notify lower-level drivers
> > when a block device below one of their hosts has been opened or
> > released.
>
> I like it and would make use of it in sbp2.
>
> sg, st, ch could be callers as well, but I suppose you don't really need
> them. And for me, sd and sr already cover 99.9% of the use cases.
Those drivers don't export a block-device interface, do they? So you
can't mount a filesystem on a device they control.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
[not found] ` <Pine.LNX.4.44L0.0803051608140.4161-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2008-03-05 21:30 ` James Bottomley
2008-03-05 21:52 ` Alan Stern
0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2008-03-05 21:30 UTC (permalink / raw
To: Alan Stern; +Cc: Linus Torvalds, Greg KH, USB list, SCSI development list
On Wed, 2008-03-05 at 16:24 -0500, Alan Stern wrote:
> On Wed, 5 Mar 2008, James Bottomley wrote:
>
> > OK, I looked through the patch series. It really looks like the
> > in-kernel thing is the wrong approach.
> >
> > Why not export this want_persist flag via sysfs and then have hal and
> > udev update it when they see a filesystem on an underlying usb device
> > mounted (and zero it out on unmount)? That way you always have the
> > exactly correct state and there's no need to go fishing for information
> > in layers that are difficult to get to.
>
> The flag (or rather, a similar one named "persist_enabled") already is
> exported via sysfs.
>
> Having hal+udev set it automatically as you describe never occurred
> to me. The reason for doing all of this within the kernel is that
> Linus said (off-list) that the kernel should handle this automatically.
> Whether he considered userspace tools taking over the responsibility, I
> can't say. (But he might object to having to wait for the tools to add
> the extra functionality.)
>
> Now, it has to be mentioned that mounted filesystems on a USB device
> account for something like 90% of the objections against USB-PERSIST.
> If we go ahead and enable it automatically in that case, there is
> really very little reason not to enable it for every USB device,
> mounted or not.
>
> In fact, the only other important reason for not turning it on by
> default (or always!) is because the USB spec says not to. By adding
> this check for mounted filesystems, we are trying to adhere as closely
> as possible to the spec -- but only in cases where we don't care since
> it doesn't matter!
>
> Whether to enable USB-PERSIST for mounted devices is a policy decision.
> Whether that policy should be set by the kernel or by userspace is
> outside my jursidiction. If you think this approach is wrong-headed
> and you can convince Linus to go along, I'll be willing to drop the
> last three patches in this series.
So make it default to one now and also put the processes in train to
update hal and udev. That way everything should work immediately and we
come back to spec compliance later.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 21:27 ` Alan Stern
@ 2008-03-05 21:47 ` Stefan Richter
0 siblings, 0 replies; 26+ messages in thread
From: Stefan Richter @ 2008-03-05 21:47 UTC (permalink / raw
To: Alan Stern; +Cc: James Bottomley, Greg KH, USB list, SCSI development list
Alan Stern wrote:
> On Wed, 5 Mar 2008, Stefan Richter wrote:
>> sg, st, ch could be callers as well, but I suppose you don't really need
>> them. And for me, sd and sr already cover 99.9% of the use cases.
>
> Those drivers don't export a block-device interface, do they? So you
> can't mount a filesystem on a device they control.
That's right, but users can "use" those devices. ;-)
Anyway, go ahead with sd and sr only; I don't really care about having
those calls in the other drivers.
--
Stefan Richter
-=====-==--- --== --=-=
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] SCSI: add a field to scsi_device to count open file references
2008-03-05 21:30 ` James Bottomley
@ 2008-03-05 21:52 ` Alan Stern
0 siblings, 0 replies; 26+ messages in thread
From: Alan Stern @ 2008-03-05 21:52 UTC (permalink / raw
To: James Bottomley
Cc: Linus Torvalds, Greg KH, David Brownell, USB list,
SCSI development list
On Wed, 5 Mar 2008, James Bottomley wrote:
> > In fact, the only other important reason for not turning it on by
> > default (or always!) is because the USB spec says not to. By adding
> > this check for mounted filesystems, we are trying to adhere as closely
> > as possible to the spec -- but only in cases where we don't care since
> > it doesn't matter!
> >
> > Whether to enable USB-PERSIST for mounted devices is a policy decision.
> > Whether that policy should be set by the kernel or by userspace is
> > outside my jursidiction. If you think this approach is wrong-headed
> > and you can convince Linus to go along, I'll be willing to drop the
> > last three patches in this series.
>
> So make it default to one now and also put the processes in train to
> update hal and udev. That way everything should work immediately and we
> come back to spec compliance later.
Under the circumstances that sounds like a reasonable course of action.
Does anybody have a serious objection?
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2008-03-05 21:52 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-03 20:16 [PATCH 7/8] SCSI: add a field to scsi_device to count open file references Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803031508490.7094-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-03 22:01 ` James Bottomley
2008-03-03 23:04 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803031758310.8280-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-03 23:13 ` James Bottomley
[not found] ` <1204585995.3043.85.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-03-04 16:16 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803041101200.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-04 16:50 ` Mike Anderson
2008-03-04 16:52 ` James Bottomley
[not found] ` <1204649571.3091.36.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-03-04 17:43 ` Alan Stern
2008-03-05 20:55 ` James Bottomley
2008-03-05 21:24 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803051608140.4161-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-05 21:30 ` James Bottomley
2008-03-05 21:52 ` Alan Stern
2008-03-04 0:37 ` Stefan Richter
2008-03-04 17:00 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.0803041151510.4039-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2008-03-04 17:26 ` Stefan Richter
2008-03-04 17:56 ` Stefan Richter
2008-03-04 20:04 ` Alan Stern
2008-03-05 18:04 ` Alan Stern
2008-03-05 19:28 ` Stefan Richter
2008-03-05 20:34 ` Alan Stern
2008-03-05 21:14 ` Stefan Richter
[not found] ` <47CF0D2E.7000607-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB@public.gmane.org>
2008-03-05 21:27 ` Alan Stern
2008-03-05 21:47 ` Stefan Richter
2008-03-05 20:13 ` patch scsi-add-a-field-to-scsi_device-to-count-open-file-references.patch added to gregkh-2.6 tree gregkh-l3A5Bk7waGM
[not found] ` <1204748015695-4DuetyUvsWfYtjvyW6yDsg@public.gmane.org>
2008-03-05 20:36 ` James Bottomley
[not found] ` <1204749410.3047.57.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-03-05 20:41 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.