Prev: AMD L3 cache index disable fixes for .35
Next: KVM: SVM: Handle MCE intercepts always on host level
From: Alan Stern on 23 Apr 2010 10:30
On Fri, 23 Apr 2010 fabien.chouteau(a)gmail.com wrote:

> From: Fabien Chouteau <fabien.chouteau(a)barco.com>
>
> This patch adds handling of the "Start/Stop Unit" SCSI request to simulate media
> ejection. A sysfs entry to get the "eject" state of a LUN is also added.

Sorry I didn't notice this earlier...

> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -163,6 +163,13 @@
> * ro setting are not allowed when the medium is loaded or if CD-ROM
> * emulation is being used.
> *
> + * When a LUN receive an "eject" SCSI request (Start/Stop Unit),
> + * if the LUN is removable, the backing file is released to simulate
> + * ejection.
> + * The "eject" state of a LUN is available in the "ejected" file of the
> + * LUN's sysfs directory (see above). The "eject" state is only updated
> + * by SCSI request, not by user ejection.

What's the reason for that? With a real removable device, like a CD
player, it doesn't make any difference whether the medium was ejected
because of a SCSI command or because I pressed the "eject" button.

I just don't see any point in keeping track of the two actions
separately, since they end up having the same final result.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chouteau Fabien on 23 Apr 2010 11:10
On Fri, Apr 23, 2010 at 4:23 PM, Alan Stern <stern(a)rowland.harvard.edu> wrote:
>
> On Fri, 23 Apr 2010 fabien.chouteau(a)gmail.com wrote:
>
> > From: Fabien Chouteau <fabien.chouteau(a)barco.com>
> >
> > This patch adds handling of the "Start/Stop Unit" SCSI request to simulate media
> > ejection. A sysfs entry to get the "eject" state of a LUN is also added.
>
> Sorry I didn't notice this earlier...
>
> > --- a/drivers/usb/gadget/f_mass_storage.c
> > +++ b/drivers/usb/gadget/f_mass_storage.c
> > @@ -163,6 +163,13 @@
> > � * ro setting are not allowed when the medium is loaded or if CD-ROM
> > � * emulation is being used.
> > � *
> > + * When a LUN receive an "eject" SCSI request (Start/Stop Unit),
> > + * if the LUN is removable, the backing file is released to simulate
> > + * ejection.
> > + * The "eject" state of a LUN is available in the "ejected" file of the
> > + * LUN's sysfs directory (see above). The "eject" state is only updated
> > + * by SCSI request, not by user ejection.
>
> What's the reason for that? �With a real removable device, like a CD
> player, it doesn't make any difference whether the medium was ejected
> because of a SCSI command or because I pressed the "eject" button.
>
> I just don't see any point in keeping track of the two actions
> separately, since they end up having the same final result.
>

By user ejection, I mean send an empty line in the "file" sysfs entry.
The Start/Stop request is an action from the USB host side, user
ejection is from the USB device side, for me it's two different
events.
Maybe my comment is not clear about this point.

I use a FAT disk image as LUN file, users can put some files in the
"fake" disk and then eject it. When I get the ejected signal, I mount
the disk image on loop device and perform operations on the user's
files.
So I want to know when users eject the disk and only when users do.

I still can use the LUN ejection from device side to disable the mass
storage device, and in this case I don't want to mount the disk and
search for user's files.

--
Fabien Chouteau
EPITA GISTR 2010
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 23 Apr 2010 11:40
On Fri, 23 Apr 2010, Chouteau Fabien wrote:

> > > + * When a LUN receive an "eject" SCSI request (Start/Stop Unit),
> > > + * if the LUN is removable, the backing file is released to simulate
> > > + * ejection.
> > > + * The "eject" state of a LUN is available in the "ejected" file of the
> > > + * LUN's sysfs directory (see above). The "eject" state is only updated
> > > + * by SCSI request, not by user ejection.
> >
> > What's the reason for that? �With a real removable device, like a CD
> > player, it doesn't make any difference whether the medium was ejected
> > because of a SCSI command or because I pressed the "eject" button.
> >
> > I just don't see any point in keeping track of the two actions
> > separately, since they end up having the same final result.
> >
>
> By user ejection, I mean send an empty line in the "file" sysfs entry.
> The Start/Stop request is an action from the USB host side, user
> ejection is from the USB device side, for me it's two different
> events.
> Maybe my comment is not clear about this point.

No; it's clear enough and I understood what you meant. It's true that
they are two different events, but they have the same end result.

> I use a FAT disk image as LUN file, users can put some files in the
> "fake" disk and then eject it. When I get the ejected signal, I mount
> the disk image on loop device and perform operations on the user's
> files.
> So I want to know when users eject the disk and only when users do.
>
> I still can use the LUN ejection from device side to disable the mass
> storage device, and in this case I don't want to mount the disk and
> search for user's files.

Why not? Isn't it possible that the user put some files there before
the device-side eject happened?

What happens if the user and the device both try to eject the medium at
approximately the same time? Which event occurs first will be purely
random chance. That means there's a 50% probability you will end up
doing the wrong thing.

No, I think you need to do the same thing whenever an eject occurs, or
else find a better criterion for deciding what to do.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michał Nazarewicz on 21 Apr 2010 05:20
On Tue, 20 Apr 2010 10:34:09 +0200, <fabien.chouteau(a)gmail.com> wrote:
> Handle eject request + sysfs entries to detect ejected LUN and suspended gadget.

> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index f4911c0..af13a2f 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -2412,6 +2449,26 @@ static void fsg_disable(struct usb_function *f)
> raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
> }
>+static void fsg_suspend(struct usb_function *f)
> +{
> + struct fsg_dev *fsg = fsg_from_func(f);
> + struct fsg_lun *curlun = fsg->common->luns;
> + int i;
> +
> + for (i = 0; i < fsg->common->nluns; i++, curlun++)
> + curlun->suspended = 1;
> +}
> +
> +static void fsg_resume(struct usb_function *f)
> +{
> + struct fsg_dev *fsg = fsg_from_func(f);
> + struct fsg_lun *curlun = fsg->common->luns;
> + int i;
> +
> + for (i = 0; i < fsg->common->nluns; i++, curlun++)
> + curlun->suspended = 0;
> +}
> +

Clearly, suspend seems like a state of the mass storage function
as a whole rather then attribute of each logical unit so I think
it'll be better to make it global for the mass storage function
rather then per-LUN.

Even more so, it's a state of the whole gadget so in my opinion the
whole code should be moved the the gadget rather then kept in
a function!

Going even further, I would propose an sysfs entry to be added to
the composite framework as a single generic interface rather then
hacking it in each gadget/function that might need to export this
information to user space.

This provided that there is no easy way of gaining that information
in user space through same other sysfs entry.

Other then that, the patch seems fine.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał "mina86" Nazarewicz (o o)
ooo +---[mina86(a)mina86.com]---[mina86(a)jabber.org]---ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michał Nazarewicz on 21 Apr 2010 09:10
> 2010/4/21 Michał Nazarewicz <m.nazarewicz(a)samsung.com>
>> Clearly, suspend seems like a state of the mass storage function
>> as a whole rather then attribute of each logical unit so I think
>> it'll be better to make it global for the mass storage function
>> rather then per-LUN.
>>
>> Even more so, it's a state of the whole gadget so in my opinion the
>> whole code should be moved the the gadget rather then kept in
>> a function!
>>
>> Going even further, I would propose an sysfs entry to be added to
>> the composite framework as a single generic interface rather then
>> hacking it in each gadget/function that might need to export this
>> information to user space.
>>
>> This provided that there is no easy way of gaining that information
>> in user space through same other sysfs entry.

On Wed, 21 Apr 2010 14:35:57 +0200, Chouteau Fabien <fabien.chouteau(a)gmail.com> wrote:
> You're right, the suspended state should be handled in the composite
> framework, I'm going resend the patch with this modification.

That'll be great.

> I have a question about the mass/file storage gadget, why is there a
> g_mass_storage gadget and a g_file_storage gadget? Code and feature seems
> redundant.

My Mass Storage Function is relatively young and thus not so mature as
File Storage Gadget. As a matter of fact, if one were to choose between
FSG and MSG then FSG is probably a better choice. MSG was provided to
test the MSF and show how to write composite gadgets using it. The
strength of MSF is of course that it is a composite function hence can
be mixed with other functions. Maybe in the future, when MSF (and MSG)
proves stability, FSG will be removed from the kernel but for now it
has been decided to let it be there. You may refer to my discussion with
Alan when I was posting the code.

> btw, the eject code of this patch comes from file_storage.c

It may be a good idea to point that in a comment.

Or maybe even extract the common do_*() functions to storage_common.c.
I always felt like the do_*() functions should be in the
storage_common.c but unfortunately there are many subtle differences,
which make those functions differ in little details between MSF and
FSG.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał "mina86" Nazarewicz (o o)
ooo +---[mina86(a)mina86.com]---[mina86(a)jabber.org]---ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: AMD L3 cache index disable fixes for .35
Next: KVM: SVM: Handle MCE intercepts always on host level