[RFC][PATCH] fs: configfs: programmatically create config groups

[RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

In some parts of the kernel (e.g. planned configfs integration into usb
gadget) there is a need to programmatically create config groups
(directories) but it would be preferable to disallow creating them by
the user. This is more or less what default_groups used to be for.
But e.g. in the mass storage gadget, after storing the number of
luns (logical units) into some configfs attribute, the corresponding lun#
directories should be created, their number is not known up front so
default_groups are no good for this.

Example:

$ echo 3 > /cfg/..../mass_storage/luns

causes

/cfg/....../mass_storage/lun0
/cfg/....../mass_storage/lun1
/cfg/....../mass_storage/lun2

to be created. Yet

$ mkdir /cfg/..../mass_storage/<any name>

should not be allowed.

With create_group exported it is very easily achieved: make_group and make_item
are set to NULL in mass_storage's config_group, yet the kernel can
create_groups at will.

I kindly ask for comments. In particular, I would like to discuss
if this is the right approach. A counterpart to remove config groups
is also required. It is not implemented in this patch, though. What are
your opinions?

Andrzej Pietrasiewicz (1):
  fs: configfs: allow other kernel parts to programmatically create
    config groups

 fs/configfs/dir.c        |    5 +++--
 include/linux/configfs.h |    2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

[RFC][PATCH] fs: configfs: allow other kernel parts to programmatically create config groups

[Andrzej Pietrasiewicz]

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 fs/configfs/dir.c        |    5 +++--
 include/linux/configfs.h |    2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 7414ae2..42608dc 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -656,7 +656,7 @@ static void detach_groups(struct config_group *group)
  * We could, perhaps, tweak our parent's ->mkdir for a minute and
  * try using vfs_mkdir.  Just a thought.
  */
-static int create_default_group(struct config_group *parent_group,
+int create_group(struct config_group *parent_group,
 				struct config_group *group)
 {
 	int ret;
@@ -690,6 +690,7 @@ static int create_default_group(struct config_group *parent_group,
 
 	return ret;
 }
+EXPORT_SYMBOL(create_group);
 
 static int populate_groups(struct config_group *group)
 {
@@ -701,7 +702,7 @@ static int populate_groups(struct config_group *group)
 		for (i = 0; group->default_groups[i]; i++) {
 			new_group = group->default_groups[i];
 
-			ret = create_default_group(group, new_group);
+			ret = create_group(group, new_group);
 			if (ret) {
 				detach_groups(group);
 				break;
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 34025df..8bf295f 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -252,6 +252,8 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
 int configfs_register_subsystem(struct configfs_subsystem *subsys);
 void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
 
+int create_group(struct config_group *parent_group, struct config_group *group);
+
 /* These functions can sleep and can alloc with GFP_KERNEL */
 /* WARNING: These cannot be called underneath configfs callbacks!! */
 int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
-- 
1.7.0.4

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

On Mon, Nov 26 2012, Andrzej Pietrasiewicz <andrzej.p@samsung.com> wrote:
> In some parts of the kernel (e.g. planned configfs integration into usb
> gadget) there is a need to programmatically create config groups
> (directories) but it would be preferable to disallow creating them by
> the user. This is more or less what default_groups used to be for.
> But e.g. in the mass storage gadget, after storing the number of
> luns (logical units) into some configfs attribute, the corresponding lun#
> directories should be created, their number is not known up front so
> default_groups are no good for this.
>
> Example:
>
> $ echo 3 > /cfg/..../mass_storage/luns
>
> causes
>
> /cfg/....../mass_storage/lun0
> /cfg/....../mass_storage/lun1
> /cfg/....../mass_storage/lun2
>
> to be created. Yet
>
> $ mkdir /cfg/..../mass_storage/<any name>
>
> should not be allowed.
>
> With create_group exported it is very easily achieved: make_group and make_item
> are set to NULL in mass_storage's config_group, yet the kernel can
> create_groups at will.

I think the above description should be part of the commit message. :)

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/26/2012 09:35 AM, Andrzej Pietrasiewicz wrote:
> In some parts of the kernel (e.g. planned configfs integration into usb
> gadget) there is a need to programmatically create config groups
> (directories) but it would be preferable to disallow creating them by
> the user. This is more or less what default_groups used to be for.
> But e.g. in the mass storage gadget, after storing the number of
> luns (logical units) into some configfs attribute, the corresponding lun#
> directories should be created, their number is not known up front so
> default_groups are no good for this.
>
> Example:
>
> $ echo 3>  /cfg/..../mass_storage/luns
>
> causes
>
> /cfg/....../mass_storage/lun0
> /cfg/....../mass_storage/lun1
> /cfg/....../mass_storage/lun2

I though we did not want the luns file but instead use

mkdir /cfg/....../mass_storage/lun0
mkdir /cfg/....../mass_storage/lun1

directly.

> to be created. Yet
>
> $ mkdir /cfg/..../mass_storage/<any name>
>
> should not be allowed.
>
> With create_group exported it is very easily achieved: make_group and make_item
> are set to NULL in mass_storage's config_group, yet the kernel can
> create_groups at will.
>
> I kindly ask for comments. In particular, I would like to discuss
> if this is the right approach. A counterpart to remove config groups
> is also required. It is not implemented in this patch, though. What are
> your opinions?

Could you please at the tcm gadget? This is a mass storage gadget using
target as backend and target is using configfs. Here is a snippet how
you setup it:

|mkdir -p $FABRIC/naa.6001405c3214b06a/tpgt_1
|mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_0
|mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_1

So you setup two luns without this patch. Would that work for you?

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

> On 11/26/2012 09:35 AM, Andrzej Pietrasiewicz wrote:
>> In some parts of the kernel (e.g. planned configfs integration into usb
>> gadget) there is a need to programmatically create config groups
>> (directories) but it would be preferable to disallow creating them by
>> the user. This is more or less what default_groups used to be for.
>> But e.g. in the mass storage gadget, after storing the number of
>> luns (logical units) into some configfs attribute, the corresponding lun#
>> directories should be created, their number is not known up front so
>> default_groups are no good for this.
>>
>> Example:
>>
>> $ echo 3>  /cfg/..../mass_storage/luns
>>
>> causes
>>
>> /cfg/....../mass_storage/lun0
>> /cfg/....../mass_storage/lun1
>> /cfg/....../mass_storage/lun2

On Mon, Nov 26 2012, Sebastian Andrzej Siewior wrote:
> I though we did not want the luns file but instead use
>
> mkdir /cfg/....../mass_storage/lun0
> mkdir /cfg/....../mass_storage/lun1
>
> directly.

I think that's suboptimal, and we can do better.  There are too many
corner cases (ie. what if user does “mkdir lun7” without the previous
luns?), it adds complexity to the kernel (ie. parsing of the name), and
the thing is overly complicated for user (“echo 5 > nluns” is much nicer
than having to create 5 directories).

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/26/2012 05:56 PM, Michal Nazarewicz wrote:
>> On 11/26/2012 09:35 AM, Andrzej Pietrasiewicz wrote:
>>> In some parts of the kernel (e.g. planned configfs integration into usb
>>> gadget) there is a need to programmatically create config groups
>>> (directories) but it would be preferable to disallow creating them by
>>> the user. This is more or less what default_groups used to be for.
>>> But e.g. in the mass storage gadget, after storing the number of
>>> luns (logical units) into some configfs attribute, the corresponding lun#
>>> directories should be created, their number is not known up front so
>>> default_groups are no good for this.
>>>
>>> Example:
>>>
>>> $ echo 3>   /cfg/..../mass_storage/luns
>>>
>>> causes
>>>
>>> /cfg/....../mass_storage/lun0
>>> /cfg/....../mass_storage/lun1
>>> /cfg/....../mass_storage/lun2
>
> On Mon, Nov 26 2012, Sebastian Andrzej Siewior wrote:
>> I though we did not want the luns file but instead use
>>
>> mkdir /cfg/....../mass_storage/lun0
>> mkdir /cfg/....../mass_storage/lun1
>>
>> directly.
>
> I think that's suboptimal, and we can do better.  There are too many
> corner cases (ie. what if user does “mkdir lun7” without the previous
> luns?), it adds complexity to the kernel (ie. parsing of the name), and
> the thing is overly complicated for user (“echo 5>  nluns” is much nicer
> than having to create 5 directories).

Wouldn't say that. It may adds complexity on another level. The target
subsystem has the same problem with adding luns and there seems nothing
wrong with having lun3 and 4 and leaving 0 and 1 unsued.
With the tcm gadget I get:

|scsi 0:0:0:2: Direct-Access     LIO-ORG  RAMDISK-MCP      4.0  PQ: 0 
ANSI: 5
|scsi 0:0:0:3: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 
ANSI: 5

You notice :2 and :3 instead :0 and :1. While should be there something
wrong with this?

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

On Mon, Nov 26 2012, Sebastian Andrzej Siewior wrote:
> Wouldn't say that. It may adds complexity on another level. The target
> subsystem has the same problem with adding luns and there seems nothing
> wrong with having lun3 and 4 and leaving 0 and 1 unsued.

That's not what Wikipedia claims though (from
<http://en.wikipedia.org/wiki/Logical_Unit_Number>):

	LUN 0: There is one LUN which is required to exist in every
	target: zero. The logical unit with LUN zero is special in that
	it must implement a few specific commands, most notably Report
	LUNs, which is how an initiator can find out all the other LUNs
	in the target. But LUN zero need not provide any other services,
	such as a storage volume.

That's why I proposed solution where one needs to have continuous
numbering of LUNs.  I'm not an expert on SCSI though.

> With the tcm gadget I get:
>
> |scsi 0:0:0:2: Direct-Access     LIO-ORG  RAMDISK-MCP      4.0  PQ: 0 
> ANSI: 5
> |scsi 0:0:0:3: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 
> ANSI: 5
>
> You notice :2 and :3 instead :0 and :1. While should be there something
> wrong with this?

It may be that it works on Linux but fails on some other systems (or
even older Linux kernels).  Like I've said, I'm not SCSI expert, so my
knowledge of it is (embarrassingly) minimal.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

On Monday, November 26, 2012 5:30 PM Sebastian Andrzej Siewior wrote:
> On 11/26/2012 09:35 AM, Andrzej Pietrasiewicz wrote:
> > In some parts of the kernel (e.g. planned configfs integration into
> > usb
> > gadget) there is a need to programmatically create config groups
> > (directories) but it would be preferable to disallow creating them by
> > the user. This is more or less what default_groups used to be for.
> > But e.g. in the mass storage gadget, after storing the number of luns
> > (logical units) into some configfs attribute, the corresponding lun#
> > directories should be created, their number is not known up front so
> > default_groups are no good for this.
> >

<snip>

> 
> |mkdir -p $FABRIC/naa.6001405c3214b06a/tpgt_1
> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_0
> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_1
> 
> So you setup two luns without this patch. Would that work for you?
> 

I think we _still_ need a way to programmatically create/remove configfs
directories. Without it, this: "After name is written it will request
the module and special configuration related files pop up."
(http://www.spinics.net/lists/linux-usb/msg75118.html)

is only possible for a static, predefined number of configuration
subdirectories. Can you guarantee there will be only such a need?
Are you sure lun# directories will not be created programmatically?
https://lkml.org/lkml/2012/11/26/584

And there are problems to be addressed right now, not possibly in
the future: take the intrefaceXX (read-only) directory, which
contains information about altsetting, interface class,
interface number, endpoints etc. It can be created only after
the gadget has actually been bound. But before the gadget is
bound it is being configured and the configfs directories
must already be there, so any default_groups are already created.
So the interfaceXX directory cannot be implemented as a default
group, but must be created programmatically.

Also, there is an idea to unbind the gadget with just doing
rmdir /cfg/...../gadgetX:
http://www.spinics.net/lists/linux-usb/msg74893.html

This implies doing a recursive rmdir first on its
subdirectories - a programmatic rmdir.

Taken all this into account, I would like to have a way
to programmatically create and remove configfs directories.
Right now creating them is like scratching the left ear
with the hand under the right knee. And it leads to
comments like: "Looking at this: full_name_hash(),
d_alloc(), d_add(), d_drop(), dput(). Do you write a
filesystem?"
http://www.spinics.net/lists/linux-usb/msg74841.html

Andrzej

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

On Tue, Nov 27 2012, Andrzej Pietrasiewicz <andrzej.p@samsung.com> wrote:
> I think we _still_ need a way to programmatically create/remove configfs
> directories. Without it, this: "After name is written it will request
> the module and special configuration related files pop up."
> (http://www.spinics.net/lists/linux-usb/msg75118.html)

I agree, and this is orthogonal to exact API used by configfs gadget.

I may be missing something about configfs, but the way I see it, lack of
such functions is a mistake in the configfs API.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/26/2012 06:54 PM, Michal Nazarewicz wrote:
> On Mon, Nov 26 2012, Sebastian Andrzej Siewior wrote:
>> Wouldn't say that. It may adds complexity on another level. The target
>> subsystem has the same problem with adding luns and there seems nothing
>> wrong with having lun3 and 4 and leaving 0 and 1 unsued.
>
> That's not what Wikipedia claims though (from
> <http://en.wikipedia.org/wiki/Logical_Unit_Number>):
>
> 	LUN 0: There is one LUN which is required to exist in every
> 	target: zero. The logical unit with LUN zero is special in that
> 	it must implement a few specific commands, most notably Report
> 	LUNs, which is how an initiator can find out all the other LUNs
> 	in the target. But LUN zero need not provide any other services,
> 	such as a storage volume.
>
> That's why I proposed solution where one needs to have continuous
> numbering of LUNs.  I'm not an expert on SCSI though.

Let me quote "4.6.4 Minimum LUN addressing requirements" of SAM4:

| All SCSI target devices shall support LUN 0 (i.e., 00000000
| 00000000h) or the REPORT LUNS well-known logical unit. For SCSI
| target devices that support the hierarchical addressing model the LUN
| 0 or the REPORT LUNS well-known logical unit shall be the logical
| unit that an application client addresses to determine
| information about the SCSI target device and the logical units
| contained within the SCSI target device.

Nab, I think not having LUN0 configured as long as REPORT LUNS says
which luns are available is fine. Target seems to work on linux without
it and SAM4 does no claim otherwise unless I miss interpret it. Any
opinion on this from your side?

>
>> With the tcm gadget I get:
>>
>> |scsi 0:0:0:2: Direct-Access     LIO-ORG  RAMDISK-MCP      4.0  PQ: 0
>> ANSI: 5
>> |scsi 0:0:0:3: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0
>> ANSI: 5
>>
>> You notice :2 and :3 instead :0 and :1. While should be there something
>> wrong with this?
>
> It may be that it works on Linux but fails on some other systems (or
> even older Linux kernels).  Like I've said, I'm not SCSI expert, so my
> knowledge of it is (embarrassingly) minimal.

Sure but still. You limit the user to create lunX folders where X can
be 0..255 for instance. If the user chooses not create lun0, why force
him?

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/27/2012 09:57 AM, Andrzej Pietrasiewicz wrote:
>> |mkdir -p $FABRIC/naa.6001405c3214b06a/tpgt_1
>> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_0
>> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_1
>>
>> So you setup two luns without this patch. Would that work for you?
>>
> I think we _still_ need a way to programmatically create/remove configfs
> directories. Without it, this: "After name is written it will request
> the module and special configuration related files pop up."
> (http://www.spinics.net/lists/linux-usb/msg75118.html)
>
> is only possible for a static, predefined number of configuration
> subdirectories. Can you guarantee there will be only such a need?

No I can't but until now I don't such a need.

> Are you sure lun# directories will not be created programmatically?
> https://lkml.org/lkml/2012/11/26/584

they are not by target and they are not complaining. We need it if we
use the num_luns file which I don't think is useful.

> And there are problems to be addressed right now, not possibly in
> the future: take the intrefaceXX (read-only) directory, which
> contains information about altsetting, interface class,
> interface number, endpoints etc. It can be created only after
> the gadget has actually been bound. But before the gadget is
> bound it is being configured and the configfs directories
> must already be there, so any default_groups are already created.

Here I understand it. This is to some point a limitation of the gadget
framework. We do know the number of interface that will be available
before we bind. We simply don't know the endpoint number. There are two 
exceptions to what I just wrote:
- g_zero drops the ISO endpoints if the UDC has no UDC support for it.
   This should not happen on-the-fly.
- UAC2 may want to make the number interfaces (and therefore configure
   able) and function (play / record) configurable.

> So the interfaceXX directory cannot be implemented as a default
> group, but must be created programmatically.
>
> Also, there is an idea to unbind the gadget with just doing
> rmdir /cfg/...../gadgetX:
> http://www.spinics.net/lists/linux-usb/msg74893.html

Since we link the gadget to the function, we should unlink it.

> This implies doing a recursive rmdir first on its
> subdirectories - a programmatic rmdir.

Hmm. On target I have to rmdir manually

|unlink naa.6001405c3214b06a/tpgt_1/lun/lun_2/virtual_scsi_port
|unlink naa.6001405c3214b06a/tpgt_1/lun/lun_3/virtual_scsi_port
|rmdir naa.6001405c3214b06a/tpgt_1/lun/lun_2
|rmdir naa.6001405c3214b06a/tpgt_1/lun/lun_3
|rmdir naa.6001405c3214b06a/tpgt_1/
|rmdir naa.6001405c3214b06a/
|cd ..
|rmdir usb_gadget

to make it all go away. Couldn't we have a tool to manage all this?
target has such a thing, you have just select a few things via a CLI
tool and the tool creates the directories for you _and_ removes them
later on.
I don't want to push python on anyone but the removal magic is simply
straight forward: unlink the disk ports, rmdir luns, tpgt,…

> Taken all this into account, I would like to have a way
> to programmatically create and remove configfs directories.
> Right now creating them is like scratching the left ear
> with the hand under the right knee. And it leads to
> comments like: "Looking at this: full_name_hash(),
> d_alloc(), d_add(), d_drop(), dput(). Do you write a
> filesystem?"
> http://www.spinics.net/lists/linux-usb/msg74841.html

That was wrong. Pushing it into configs is better but I am not sure we
need it. I understand the need for things that pop later like
interfaceXX but couldn't the user manually create them if he needs them?

>
> Andrzej

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

On Tue, Nov 27 2012, Sebastian Andrzej Siewior wrote:
> I don't want to push python on anyone but the removal magic is simply
> straight forward: unlink the disk ports, rmdir luns, tpgt,…

How should a generic tool know what kind of actions are needed for given
function to be removed?  If you ask me, there should be a way to unbind
gadget and unload all modules without any specific knowledge of the
functions.  If there is no such mechanism, then it's a bad user
interface.

> I understand the need for things that pop later like interfaceXX but
> couldn't the user manually create them if he needs them?

I think the question is of information flow direction.  If user gives
some information to the kernel, she should be the one creating any
necessary directories.  But if the information comes from kernel to the
user, the kernel should create the structure.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Nicholas A. Bellinger]

Hi Sebastian & Co,

On Tue, 2012-11-27 at 16:20 +0100, Sebastian Andrzej Siewior wrote:
> On 11/26/2012 06:54 PM, Michal Nazarewicz wrote:
> > On Mon, Nov 26 2012, Sebastian Andrzej Siewior wrote:
> >> Wouldn't say that. It may adds complexity on another level. The target
> >> subsystem has the same problem with adding luns and there seems nothing
> >> wrong with having lun3 and 4 and leaving 0 and 1 unsued.
> >
> > That's not what Wikipedia claims though (from
> > <http://en.wikipedia.org/wiki/Logical_Unit_Number>):
> >
> > 	LUN 0: There is one LUN which is required to exist in every
> > 	target: zero. The logical unit with LUN zero is special in that
> > 	it must implement a few specific commands, most notably Report
> > 	LUNs, which is how an initiator can find out all the other LUNs
> > 	in the target. But LUN zero need not provide any other services,
> > 	such as a storage volume.
> >
> > That's why I proposed solution where one needs to have continuous
> > numbering of LUNs.  I'm not an expert on SCSI though.
> 
> Let me quote "4.6.4 Minimum LUN addressing requirements" of SAM4:
> 
> | All SCSI target devices shall support LUN 0 (i.e., 00000000
> | 00000000h) or the REPORT LUNS well-known logical unit. For SCSI
> | target devices that support the hierarchical addressing model the LUN
> | 0 or the REPORT LUNS well-known logical unit shall be the logical
> | unit that an application client addresses to determine
> | information about the SCSI target device and the logical units
> | contained within the SCSI target device.
> 
> Nab, I think not having LUN0 configured as long as REPORT LUNS says
> which luns are available is fine. Target seems to work on linux without
> it and SAM4 does no claim otherwise unless I miss interpret it. Any
> opinion on this from your side?
> 

So we use a special RAMDISK-MCP @ target_core_device.c:g_lun0_dev along
with a se_lun (located @ se_portal_group->tpg_virt_lun0) to always
service REPORT_LUNS to LUN=0, regardless of LUN=0 configfs fabric
endpoint layout.

Note this happens within target_core_device.c:transport_lookup_cmd_lun()
once no active se_node_acl->device_list[unpacked_lun] entry can be
located.

> >
> >> With the tcm gadget I get:
> >>
> >> |scsi 0:0:0:2: Direct-Access     LIO-ORG  RAMDISK-MCP      4.0  PQ: 0
> >> ANSI: 5
> >> |scsi 0:0:0:3: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0
> >> ANSI: 5
> >>
> >> You notice :2 and :3 instead :0 and :1. While should be there something
> >> wrong with this?
> >
> > It may be that it works on Linux but fails on some other systems (or
> > even older Linux kernels).  Like I've said, I'm not SCSI expert, so my
> > knowledge of it is (embarrassingly) minimal.
> 
> Sure but still. You limit the user to create lunX folders where X can
> be 0..255 for instance. If the user chooses not create lun0, why force
> him?
> 

It's certainly easier for the user if REPORT_LUNS always 'just works' to
LUN=0.

--nab

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

On Tuesday, November 27, 2012 5:00 PM Sebastian Andrzej Siewior wrote:
> On 11/27/2012 09:57 AM, Andrzej Pietrasiewicz wrote:
> >> |mkdir -p $FABRIC/naa.6001405c3214b06a/tpgt_1
> >> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_0
> >> |mkdir $FABRIC/naa.6001405c3214b06a/tpgt_1/lun/lun_1
> >>
> >> So you setup two luns without this patch. Would that work for you?
> >>
> > I think we _still_ need a way to programmatically create/remove
> > configfs directories. Without it, this: "After name is written it will
> > request the module and special configuration related files pop up."
> > (http://www.spinics.net/lists/linux-usb/msg75118.html)
> >
> > is only possible for a static, predefined number of configuration
> > subdirectories. Can you guarantee there will be only such a need?
> 
> No I can't but until now I don't such a need.
> 
> > Are you sure lun# directories will not be created programmatically?
> > https://lkml.org/lkml/2012/11/26/584
> 
> they are not by target and they are not complaining. We need it if we use
> the num_luns file which I don't think is useful.
> 
> > And there are problems to be addressed right now, not possibly in the
> > future: take the intrefaceXX (read-only) directory, which contains
> > information about altsetting, interface class, interface number,
> > endpoints etc. It can be created only after the gadget has actually
> > been bound. But before the gadget is bound it is being configured and
> > the configfs directories must already be there, so any default_groups
> > are already created.
> 
> Here I understand it. This is to some point a limitation of the gadget
> framework. We do know the number of interface that will be available
> before we bind. We simply don't know the endpoint number. There are two
> exceptions to what I just wrote:
> - g_zero drops the ISO endpoints if the UDC has no UDC support for it.
>    This should not happen on-the-fly.
> - UAC2 may want to make the number interfaces (and therefore configure
>    able) and function (play / record) configurable.
> 

So do we know everything before bind or we don't?

> > So the interfaceXX directory cannot be implemented as a default group,
> > but must be created programmatically.
> >
> > Also, there is an idea to unbind the gadget with just doing rmdir
> > /cfg/...../gadgetX:
> > http://www.spinics.net/lists/linux-usb/msg74893.html
> 
> Since we link the gadget to the function, we should unlink it.
> 
> > This implies doing a recursive rmdir first on its subdirectories - a
> > programmatic rmdir.
> 
> Hmm. On target I have to rmdir manually
> 
> |unlink naa.6001405c3214b06a/tpgt_1/lun/lun_2/virtual_scsi_port
> |unlink naa.6001405c3214b06a/tpgt_1/lun/lun_3/virtual_scsi_port
> |rmdir naa.6001405c3214b06a/tpgt_1/lun/lun_2
> |rmdir naa.6001405c3214b06a/tpgt_1/lun/lun_3
> |rmdir naa.6001405c3214b06a/tpgt_1/
> |rmdir naa.6001405c3214b06a/
> |cd ..
> |rmdir usb_gadget
> 
> to make it all go away. Couldn't we have a tool to manage all this?
> target has such a thing, you have just select a few things via a CLI tool
> and the tool creates the directories for you _and_ removes them later on.
> I don't want to push python on anyone but the removal magic is simply
> straight forward: unlink the disk ports, rmdir luns, tpgt,.
> 
> > Taken all this into account, I would like to have a way to
> > programmatically create and remove configfs directories.
> > Right now creating them is like scratching the left ear with the hand
> > under the right knee. And it leads to comments like: "Looking at this:
> > full_name_hash(), d_alloc(), d_add(), d_drop(), dput(). Do you write a
> > filesystem?"
> > http://www.spinics.net/lists/linux-usb/msg74841.html
> 
> That was wrong. Pushing it into configs is better but I am not sure we
> need it. I understand the need for things that pop later like interfaceXX
> but couldn't the user manually create them if he needs them?
> 

What name shall the user use? How to know which user-created directory
should correspond to which actual interface? If there are, say,
3 interfaces, what would:

$ mkdir interface873246

mean?

And in general, what would

$ mkdir rykcq1234

mean?

Let's go one directory deeper in the hierarchy and suppose there is
no programmatic directories creation. So we

$ cd interface<something>

so that we can create the endpoint directories.
And now what? What names shall the user use for the endpoint
directories? Oh, that's simple: just see what the endpoint
directories' names are. But wait, aren't we just creating them?

Please also see Michał's point about user interface.

Andrzej

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> On Tue, Nov 27 2012, Sebastian Andrzej Siewior wrote:
>> I don't want to push python on anyone but the removal magic is simply
>> straight forward: unlink the disk ports, rmdir luns, tpgt,…
>
> How should a generic tool know what kind of actions are needed for given
> function to be removed?  If you ask me, there should be a way to unbind
> gadget and unload all modules without any specific knowledge of the
> functions.  If there is no such mechanism, then it's a bad user
> interface.

Well. You need only to remove the directories you created. An unbind
would be simply an unlink of the gadget which is linked to the udc.
All configurations remain so you can link it at a later point without
touching the configuration because it is as it was.

>> I understand the need for things that pop later like interfaceXX but
>> couldn't the user manually create them if he needs them?
>
> I think the question is of information flow direction.  If user gives
> some information to the kernel, she should be the one creating any
> necessary directories.  But if the information comes from kernel to the
> user, the kernel should create the structure.

Yes that is a point. But the "name" can go away if we use it in the
directory name. That is what other configfs user do. The same is true
for luns for instance. I just want to avoid adding features because we
do something different compared to every other configfs user.

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/28/2012 09:10 AM, Andrzej Pietrasiewicz wrote:
>> Here I understand it. This is to some point a limitation of the gadget
>> framework. We do know the number of interface that will be available
>> before we bind. We simply don't know the endpoint number. There are two
>> exceptions to what I just wrote:
>> - g_zero drops the ISO endpoints if the UDC has no UDC support for it.
>>     This should not happen on-the-fly.
>> - UAC2 may want to make the number interfaces (and therefore configure
>>     able) and function (play / record) configurable.
>>
>
> So do we know everything before bind or we don't?

After some sleep I think we do.

>> That was wrong. Pushing it into configs is better but I am not sure we
>> need it. I understand the need for things that pop later like interfaceXX
>> but couldn't the user manually create them if he needs them?
>>
>
> What name shall the user use? How to know which user-created directory
> should correspond to which actual interface? If there are, say,
> 3 interfaces, what would:
>
> $ mkdir interface873246
>
> mean?
>
> And in general, what would
>
> $ mkdir rykcq1234
>
> mean?
>
> Let's go one directory deeper in the hierarchy and suppose there is
> no programmatic directories creation. So we
>
> $ cd interface<something>
>
> so that we can create the endpoint directories.
> And now what? What names shall the user use for the endpoint
> directories? Oh, that's simple: just see what the endpoint
> directories' names are. But wait, aren't we just creating them?
>
> Please also see Michał's point about user interface.

Yeah I did. Now I'm okay with creating new directories but we should
keep this to a minimum and encode as much information possible in
directory's name.

>
> Andrzej

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

> On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
>> How should a generic tool know what kind of actions are needed for given
>> function to be removed?  If you ask me, there should be a way to unbind
>> gadget and unload all modules without any specific knowledge of the
>> functions.  If there is no such mechanism, then it's a bad user
>> interface.

On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
> Well. You need only to remove the directories you created.

My point is that there should be a way to write a script that is unaware
of the way function is configured, ie. which directories were created
and which were not.

Besides, if you rmdir lun0, is the function still supposed to work with
all LUNs present?  In my opinion, while gadget is bound, it should not
be possible to modify such things.

> An unbind would be  simply an unlink of the gadget  which is linked to
> the udc.   All configurations  remain so  you can link  it at  a later
> point without touching the configuration because it is as it was.

Yes, but that's not my concern.  My concern is that I should be able to
put a relatively simple code in my shutdown script (or whatever) which
unbinds all gadgets, without knowing what kind of functions are used.

And I'm proposing that this could be done by allowing user to just do:

	cd /cfs/...
	rmdir gadgets/*		# unbind and remove all gadgets
	rmdir functions/*/*	# unbind and remove all function instances
	rmdir functions/*	# unload all functions
	rmdir udcs/*		# unload all UDCs
        
>> I think the question is of information flow direction.  If user gives
>> some information to the kernel, she should be the one creating any
>> necessary directories.  But if the information comes from kernel to the
>> user, the kernel should create the structure.

> Yes that is a point. But the "name" can go away if we use it in the
> directory name. That is what other configfs user do. The same is true
> for luns for instance. I just want to avoid adding features because we
> do something different compared to every other configfs user.

You've lost me here.  What are we talking about again?  What “name” are
you referring to?

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
>> On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
>>> How should a generic tool know what kind of actions are needed for given
>>> function to be removed?  If you ask me, there should be a way to unbind
>>> gadget and unload all modules without any specific knowledge of the
>>> functions.  If there is no such mechanism, then it's a bad user
>>> interface.
>
> On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
>> Well. You need only to remove the directories you created.
>
> My point is that there should be a way to write a script that is unaware
> of the way function is configured, ie. which directories were created
> and which were not.

I get this. If you recursively rmdir each directory then you clean it
up.

> Besides, if you rmdir lun0, is the function still supposed to work with
> all LUNs present?  In my opinion, while gadget is bound, it should not
> be possible to modify such things.

That is correct. The configuration should remain frozen as long as the
gadget is active because in most cases you can't propagate the change.

>> An unbind would be  simply an unlink of the gadget  which is linked to
>> the udc.   All configurations  remain so  you can link  it at  a later
>> point without touching the configuration because it is as it was.
>
> Yes, but that's not my concern.  My concern is that I should be able to
> put a relatively simple code in my shutdown script (or whatever) which
> unbinds all gadgets, without knowing what kind of functions are used.
>
> And I'm proposing that this could be done by allowing user to just do:
>
> 	cd /cfs/...
> 	rmdir gadgets/*		# unbind and remove all gadgets
> 	rmdir functions/*/*	# unbind and remove all function instances
> 	rmdir functions/*	# unload all functions

Yes, you push for simple rmdir API. That would avoid the need for an
user land tool at some point and you end up in shell scripts.
I'm not against it but others do have user tools to handle such things.
Anyway, for this to work we have to go through Joel.

> 	rmdir udcs/*		# unload all UDCs

No, for this you still have to rmmod :)


>>> I think the question is of information flow direction.  If user gives
>>> some information to the kernel, she should be the one creating any
>>> necessary directories.  But if the information comes from kernel to the
>>> user, the kernel should create the structure.
>
>> Yes that is a point. But the "name" can go away if we use it in the
>> directory name. That is what other configfs user do. The same is true
>> for luns for instance. I just want to avoid adding features because we
>> do something different compared to every other configfs user.
>
> You've lost me here.  What are we talking about again?  What “name” are
> you referring to?

            <function_name>-<common_name>
/functions/acm-function/

instead of
            <common_name>
/functions/function1/
                      +name
with attribute file named "name" which contains the name of the
function (i.e. acm). My point is to keep "name" in the directory name
instead having an extra attribute.

Sebastian

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

On Wednesday, November 28, 2012 9:39 AM Sebastian Andrzej Siewior wrote:

<snip>

> >
> > so that we can create the endpoint directories.
> > And now what? What names shall the user use for the endpoint
> > directories? Oh, that's simple: just see what the endpoint
> > directories' names are. But wait, aren't we just creating them?
> >
> > Please also see Michał's point about user interface.
> 
> Yeah I did. Now I'm okay with creating new directories but we should keep
> this to a minimum and encode as much information possible in directory's
> name.
> 

I think I've identified one more case where programmatic creation/removal
of configfs directories is required. There is a general agreement that
binding/unbinding the gadgets will be achieved with using symlinks between
configfs representations of udcs and gadgets. So we need to represent
udcs in configfs. Suppose that the udc driver for user's platform
is modular, e.g. s3c-hsotg.ko. Now, after:

$ modprobe s3c-hsotg

I would _very_ much like the s3c-hsotg or something similar to appear
_automatically_ under $CONFIGFS_ROOT/udcs, e.g.:

$ ls $CONFIGFS_ROOT/udcs

s3c-hsotg

If there can be more instances than 1, then probably I would want

s3c-hsotg.0 	s3c-hsotg.1 ....

or something like that.

It would be _very_ frustrating for the user to have to guess
what name to use _if_ the relevant directory were to be
created manually with mkdir. Conversely, what would

$ rmdir s3c-hsotg

mean? Should it cause the s3c-hsotg.ko to unload?

All the above problems are elegantly solved with programmatic
creation and removal of configfs directories: in the udcs
config_group there is no make_item nor make_group, so mkdir
is not allowed, but instead the directories appear and
disappear as udcs come and go.

Andrzej

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Michal Nazarewicz]

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

On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
>             <function_name>-<common_name>
> /functions/acm-function/
>
> instead of
>             <common_name>
> /functions/function1/
>                       +name
> with attribute file named "name" which contains the name of the
> function (i.e. acm). My point is to keep "name" in the directory name
> instead having an extra attribute.

Right.  But as I've suggested:

	mkdir functions/<function-name>
	mkdir functions/<function-name>/<common-name>

which IMO is easier to handle in kernel (no parsing) and looks nicer in
user space.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 11/28/2012 03:24 PM, Michal Nazarewicz wrote:
> On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
>>              <function_name>-<common_name>
>> /functions/acm-function/
>>
>> instead of
>>              <common_name>
>> /functions/function1/
>>                        +name
>> with attribute file named "name" which contains the name of the
>> function (i.e. acm). My point is to keep "name" in the directory name
>> instead having an extra attribute.
>
> Right.  But as I've suggested:
>
> 	mkdir functions/<function-name>
> 	mkdir functions/<function-name>/<common-name>
>
> which IMO is easier to handle in kernel (no parsing) and looks nicer in
> user space.

Just posted an example. Please tell me what you think.

Sebastian

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Joel Becker]

Hey Guys,
	Sorry I missed this for a while.  I'll make a couple of inline
comments, and then I'll summarize my (incomplete) thoughts at the
bottom.

On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej Siewior wrote:
> On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> >>>How should a generic tool know what kind of actions are needed for given
> >>>function to be removed?  If you ask me, there should be a way to unbind
> >>>gadget and unload all modules without any specific knowledge of the
> >>>functions.  If there is no such mechanism, then it's a bad user
> >>>interface.

	Please remember that configfs is not a "user" interface, it's a
userspace<->kernelspace interface.  Like sysfs, it's not required to be
convenient for someone at a bash prompt.  My goal is that it is *usable*
from a bash prompt.  So it must be that you can
create/destroy/configure objects via mkdir/rmkdir/cat/echo, but you
might have a lot of those mkdir/echo combos to configure something.
When it comes to the "user" interface, a wrapper script or library
should be converting a user intention into all that boilerplate.

> >
> >On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
> >>Well. You need only to remove the directories you created.
> >
> >My point is that there should be a way to write a script that is unaware
> >of the way function is configured, ie. which directories were created
> >and which were not.

	As I stated above, I expect that tools will know which is which.
But having said that, a major goal of configfs is that it is discoverable and
transparent.  So you have to be able to distinguish between default
groups and created directories.  When you rmdir a configfs directory,
EACCESS means you don't have permission, ENOTEMPTY means there are children,
EBUSY means there is a depend or a link, and EPERM means it is a default
group.

> I get this. If you recursively rmdir each directory then you clean it
> up.
> 
> >Besides, if you rmdir lun0, is the function still supposed to work with
> >all LUNs present?  In my opinion, while gadget is bound, it should not
> >be possible to modify such things.
> 
> That is correct. The configuration should remain frozen as long as the
> gadget is active because in most cases you can't propagate the change.
> 
> >>An unbind would be  simply an unlink of the gadget  which is linked to
> >>the udc.   All configurations  remain so  you can link  it at  a later
> >>point without touching the configuration because it is as it was.
> >
> >Yes, but that's not my concern.  My concern is that I should be able to
> >put a relatively simple code in my shutdown script (or whatever) which
> >unbinds all gadgets, without knowing what kind of functions are used.
> >
> >And I'm proposing that this could be done by allowing user to just do:
> >
> >	cd /cfs/...
> >	rmdir gadgets/*		# unbind and remove all gadgets
> >	rmdir functions/*/*	# unbind and remove all function instances
> >	rmdir functions/*	# unload all functions
> 
> Yes, you push for simple rmdir API. That would avoid the need for an
> user land tool at some point and you end up in shell scripts.
> I'm not against it but others do have user tools to handle such things.

	Yeah, user tools are expected (and should be).

> Anyway, for this to work we have to go through Joel.
> 
> >	rmdir udcs/*		# unload all UDCs
> 
> No, for this you still have to rmmod :)
> 
> 
> >>>I think the question is of information flow direction.  If user gives
> >>>some information to the kernel, she should be the one creating any
> >>>necessary directories.  But if the information comes from kernel to the
> >>>user, the kernel should create the structure.

	This last paragraph actually describes the distinction between
configfs and sysfs.  More specifically, if userspace wants to create an
object in the kernel, it should use configfs.  If the kernel has created
an object on its own, it exposes it via sysfs.  It is a deliberate
non-goal for configfs to replicate sysfs behavior.

[General Thoughts]

	First let me restate your problem to see if I understand it.
	You want to expose e.g. five LUNs.  They should eventually appear
in configfs as five items to configure (.../{lun0,lun1,...lun4}/.  The
current configfs way to do this is to have your setup script do:

	cd /cfg/.../mass_storage
	mkdir lun0
	echo blah >lun0/attr1
	echo blahh >lun0/attr2
	mkdir lun1
	echo blag >lun1/attr1
	echo blagg >lun1/attr1
	...

I think the primary concern expressed by Andrzej is that a random user
could come along and say "mkdir lun8", even though the gadget cannot
support it.  A secondary concern from Michal is that userspace has to
run all of those mkdirs.  The thread has described varying solutions.
	If these original directories were default_groups, you could
disallow any child creation and not require the setup script to create
directories.  Here I will point out that you *can* create default_groups
programmatically.  The default_groups pointer on each configfs_group
can be different.  ->make_group() can attach whatever groups it wants.
If this would work, it would fit both of your concerns, but you do not
have a priori knowledge of the LUN count.
	Your original email proposed that the max lun be set like so:

	cd /cfg/.../mass_storage
	echo 5 >luns_allowed

There are then a couple of proposed ways to enforce the limit.  Your
->create_group() idea wants luns_allowed to be able to create subdirs in
the ->store() function.  No ->mkdir() is provided, so a lunN cannot be
created by hand.
	The ->create_group() idea has three challenges.  First, it
creates "default" groups *after* the object is created.  This makes no
sense if they are attributes of the toplevel object.  Second, it volates
the configfs mantra that user-generated objects are explicitly created.
The kicker, however, is the technical problem.  configfs explicitly
forbids tree changes inside attribute operations for deadlock reasons.
Trying to create new tree parts in luns_allowed->store() is exactly
that.
	Another suggestion (I think I read this in the thread) would be
to have ->mkdir() function parse the name and check that it is lunN,
where N is less than the limit.  I think that this was shot down because
of parsing the LUN name.  I don't think that's too terrible, but there's
certainly room for argument.

[Possible Solutions]

	Before coming back to the proposed patch, let's look at what I
might do if I were fitting this into configfs.
	I would actually do what I described at first:

	cd /cfg/.../mass_storage
	mkdir lun0
	echo blah >lun0/attr1
	...

This is idiomatic configfs usage.  Sebastian noted this earlier in the
thread.  Michal thought the repeated mkdirs were a problem ("I think
that's suboptimal, and we can do better").  This is Michal's objection
to making the user do work.  This is where I point out we are not
designing configfs interfaces for ease of end-user use.  We're defining
them for transparent and discoverable interfaces for tools to create
things in the kernel.  So the repeated mkdir()s are actually a good
thing from my perspective; userspace is telling the kernel to create LUN
objects.
	But this doesn't alleviate Andrzej's primary concern, nor does
it answer Michal's concerns about corner cases.  Let's see if we can
solve those.
	First, what about preventing extraneous LUNs?  There are two
approaches: the prevent approach, and the ignore approach.  In the
Target module, they take the ignore approach.  If you create a LUN that
is outside the scope, they just ignore it.  So a user can make 10 LUNs,
but if only five are allowed, the other five are ignored.  In the
prevent approach, we try to fail the mkdir() when we go over the limit.
	So you've set a limit with "echo 5 >luns_allowed".  How do you
restrict?  If you don't care about contiguous LUN numbering, you could
literally do this in your make_group():

	if (count_luns(parent) >= parent->luns_allowed)
		return -ENOSPC;

Note that we don't check the LUN number; SCSI does not require
contiguous numbering.  It does require LUN 0, which should probably be a
default group.  So your lun count starts at 1.
	What if you decide you really want contiguous LUN numbers, which
is considered nice to have?  Then we get to Michal's concern about
parsing the name.  If you "mkdir lun1", the code has to confirm that it
a) starts with "lun", b) finishes with a parsable number, c) that number
is < luns_allowed.
	But you need to do the parsing anyway!  If the LUN is created
via mkdir(), somehow the gadget system needs to know what LUN number to
advertise.  I would actually decouple it from the name.  The name should
be free-form; if it happens to be comprehensible, that's good.  Imagine
this:
	cd /cfg/.../mass_storage
	echo 4 >max_lun_id
	mkdir lun100
	echo 1 >lun100/lun_id

The LUN actually has the ID of 1.  The fact that the directory name is
wrong is irrelevant to the gadget processing; non-hackers use the
tooling, and the tooling should  make sane names.  Then your
->make_group() function can choose either approach to avoiding LUN IDs
that are too large.  It can prevent:

	lun_id->store() {
		if (lun_id_exists(parent, value))
			return ERR_PTR(-EINVAL);
		if (value > parent->max_lun_id)
			return ERR_PTR(-ENOSPC) /* or EINVAL */
		setup_lun();
		make_live();
	}

Or it can ignore:

	lun_id->store() {
		if (lun_id_exists(parent, value))
			return ERR_PTR(-EINVAL);
		if (value <= parent->max_lun_id) {
			setup_lun();
			make_live();
		}
		/* Do nothing for the ignored sucker */
	}

	I think that I, personally, would go with this <name>/lun_id
scheme.  Given a requirement for contiguous LUNs and the max_lun_id
restriction, I'd go with the "prevent" option myself.
	But is max LUNs a requirement?  Why not just have the LUN count
be the number of LUNs created?  In that case, you can allow or disallow
discontiguous LUN IDs without worrying about a max ID.

[Really Hate Mkdir]

	If you considered it appropriate for the LUN objects to be
created by the kernel, the standard way would to have
"echo 5 >luns_allowed" create LUN objects in sysfs.  Yes, sysfs.
There's no reason you can't have things in configfs *and* sysfs.  My
understanding of your code says to me that the mkdir approach is the way
to go, but if you hate it, this works.

[What About the Patch?]

	In general, attribute setting that turns into created configfs
objects is against the theory of configfs.  In practice, it's a
nightmare locking change.  Coming into this discussion, I though you
were doing dymanic things at ->make_group() time, but that is already
supported.
	But I want to hear your thoughts.  I've dumped a bunch of
alternate approaches here.  Do any seem to fit?  What other challenges
do you see?

Joel

-- 

"Sometimes when reading Goethe I have the paralyzing suspicion
 that he is trying to be funny."
         - Guy Davenport

			http://www.jlbec.org/
			jlbec@evilplan.org

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

Hello Joel,

So you are alive, I'm glad to hear from you ;) Thank you for your response.

On Saturday, December 08, 2012 12:18 AM Joel Becker wrote:
> Subject: Re: [RFC][PATCH] fs: configfs: programmatically create config
> groups
> 
> Hey Guys,
> 	Sorry I missed this for a while.  I'll make a couple of inline
> comments, and then I'll summarize my (incomplete) thoughts at the bottom.
> 
> On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej Siewior wrote:
> > On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> > >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> > >>>How should a generic tool know what kind of actions are needed for
> > >>>given function to be removed?  If you ask me, there should be a way
> > >>>to unbind gadget and unload all modules without any specific
> > >>>knowledge of the functions.  If there is no such mechanism, then
> > >>>it's a bad user interface.
> 
> 	Please remember that configfs is not a "user" interface, it's a
> userspace<->kernelspace interface.  Like sysfs, it's not required to be
> convenient for someone at a bash prompt.  My goal is that it is *usable*
> from a bash prompt.  So it must be that you can create/destroy/configure
> objects via mkdir/rmkdir/cat/echo, but you might have a lot of those
> mkdir/echo combos to configure something.
> When it comes to the "user" interface, a wrapper script or library should
> be converting a user intention into all that boilerplate.
> 

If you say so there is little we can do, is there? :O

<snip>

> 
> 	Yeah, user tools are expected (and should be).
> 

ditto

> > Anyway, for this to work we have to go through Joel.
> >
> > >	rmdir udcs/*		# unload all UDCs
> >
> > No, for this you still have to rmmod :)
> >
> >
> > >>>I think the question is of information flow direction.  If user
> > >>>gives some information to the kernel, she should be the one
> > >>>creating any necessary directories.  But if the information comes
> > >>>from kernel to the user, the kernel should create the structure.
> 
> 	This last paragraph actually describes the distinction between
> configfs and sysfs.  More specifically, if userspace wants to create an
> object in the kernel, it should use configfs.  If the kernel has created
> an object on its own, it exposes it via sysfs.  It is a deliberate non-
> goal for configfs to replicate sysfs behavior.

So if the lifetime of some object is controlled by the user, it belongs
to configfs. If, on the other hand, the lifetime is controlled by the
kernel, it belongs to sysfs. I think that the trouble with e.g. luns
is that they might want to behave like both (at least in the "more
automated"
approach where they are programmatically created): they are created
by the kernel (=> sysfs) but their lifetime is controlled by the user
(=>configfs).

> 
> [General Thoughts]
> 
> 	First let me restate your problem to see if I understand it.
> 	You want to expose e.g. five LUNs.  They should eventually appear in
> configfs as five items to configure (.../{lun0,lun1,...lun4}/.  The
> current configfs way to do this is to have your setup script do:
> 
> 	cd /cfg/.../mass_storage
> 	mkdir lun0
> 	echo blah >lun0/attr1
> 	echo blahh >lun0/attr2
> 	mkdir lun1
> 	echo blag >lun1/attr1
> 	echo blagg >lun1/attr1
> 	...
> 
> I think the primary concern expressed by Andrzej is that a random user
> could come along and say "mkdir lun8", even though the gadget cannot
> support it.  A secondary concern from Michal is that userspace has to run
> all of those mkdirs.  The thread has described varying solutions.
> 	If these original directories were default_groups, you could
> disallow any child creation and not require the setup script to create
> directories.  Here I will point out that you *can* create default_groups
> programmatically.  The default_groups pointer on each configfs_group can
> be different.  ->make_group() can attach whatever groups it wants.
> If this would work, it would fit both of your concerns, but you do not
> have a priori knowledge of the LUN count.
> 	Your original email proposed that the max lun be set like so:
> 
> 	cd /cfg/.../mass_storage
> 	echo 5 >luns_allowed
> 
> There are then a couple of proposed ways to enforce the limit.  Your
> ->create_group() idea wants luns_allowed to be able to create subdirs in
> the ->store() function.  No ->mkdir() is provided, so a lunN cannot be
> created by hand.
> 	The ->create_group() idea has three challenges.  First, it creates
> "default" groups *after* the object is created.  This makes no sense if
> they are attributes of the toplevel object.  Second, it volates the
> configfs mantra that user-generated objects are explicitly created.
> The kicker, however, is the technical problem.  configfs explicitly
> forbids tree changes inside attribute operations for deadlock reasons.
> Trying to create new tree parts in luns_allowed->store() is exactly that.
> 	Another suggestion (I think I read this in the thread) would be to
> have ->mkdir() function parse the name and check that it is lunN, where N
> is less than the limit.  I think that this was shot down because of
> parsing the LUN name.  I don't think that's too terrible, but there's
> certainly room for argument.
> 
> [Possible Solutions]
> 
> 	Before coming back to the proposed patch, let's look at what I might
> do if I were fitting this into configfs.
> 	I would actually do what I described at first:
> 
> 	cd /cfg/.../mass_storage
> 	mkdir lun0
> 	echo blah >lun0/attr1
> 	...
> 
> This is idiomatic configfs usage.  Sebastian noted this earlier in the
> thread.  Michal thought the repeated mkdirs were a problem ("I think
> that's suboptimal, and we can do better").  This is Michal's objection to
> making the user do work.  This is where I point out we are not designing
> configfs interfaces for ease of end-user use.  We're defining them for
> transparent and discoverable interfaces for tools to create things in the
> kernel.  So the repeated mkdir()s are actually a good thing from my
> perspective; userspace is telling the kernel to create LUN objects.
> 	But this doesn't alleviate Andrzej's primary concern, nor does it
> answer Michal's concerns about corner cases.  Let's see if we can solve
> those.
> 	First, what about preventing extraneous LUNs?  There are two
> approaches: the prevent approach, and the ignore approach.  In the Target
> module, they take the ignore approach.  If you create a LUN that is
> outside the scope, they just ignore it.  So a user can make 10 LUNs, but
> if only five are allowed, the other five are ignored.  In the prevent
> approach, we try to fail the mkdir() when we go over the limit.

I think I like the prevent approach better, because I don't know what to
do with the unused luns in the ignore approach. I also think that it would
be nice if configfs described the actual state of the system instead of
presenting all the possible user-created garbage (i.e. something which is
ignored).

> 	So you've set a limit with "echo 5 >luns_allowed".  How do you
> restrict?  If you don't care about contiguous LUN numbering, you could
> literally do this in your make_group():
> 
> 	if (count_luns(parent) >= parent->luns_allowed)
> 		return -ENOSPC;
> 
> Note that we don't check the LUN number; SCSI does not require contiguous
> numbering.  It does require LUN 0, which should probably be a default
> group.  So your lun count starts at 1.
> 	What if you decide you really want contiguous LUN numbers, which is
> considered nice to have?  Then we get to Michal's concern about parsing
> the name.  If you "mkdir lun1", the code has to confirm that it
> a) starts with "lun", b) finishes with a parsable number, c) that number
> is < luns_allowed.
> 	But you need to do the parsing anyway!  If the LUN is created via
> mkdir(), somehow the gadget system needs to know what LUN number to
> advertise.  I would actually decouple it from the name.  The name should
> be free-form; if it happens to be comprehensible, that's good.  Imagine
> this:
> 	cd /cfg/.../mass_storage
> 	echo 4 >max_lun_id
> 	mkdir lun100
> 	echo 1 >lun100/lun_id
> 
> The LUN actually has the ID of 1.  The fact that the directory name is
> wrong is irrelevant to the gadget processing; non-hackers use the tooling,
> and the tooling should  make sane names.  Then your
> ->make_group() function can choose either approach to avoiding LUN IDs
> that are too large.  It can prevent:
> 
> 	lun_id->store() {
> 		if (lun_id_exists(parent, value))
> 			return ERR_PTR(-EINVAL);
> 		if (value > parent->max_lun_id)
> 			return ERR_PTR(-ENOSPC) /* or EINVAL */
> 		setup_lun();
> 		make_live();
> 	}
> 
> Or it can ignore:
> 
> 	lun_id->store() {
> 		if (lun_id_exists(parent, value))
> 			return ERR_PTR(-EINVAL);
> 		if (value <= parent->max_lun_id) {
> 			setup_lun();
> 			make_live();
> 		}
> 		/* Do nothing for the ignored sucker */
> 	}
> 
> 	I think that I, personally, would go with this <name>/lun_id scheme.
> Given a requirement for contiguous LUNs and the max_lun_id restriction,
> I'd go with the "prevent" option myself.

Taking into account what you say about the expected userspace tooling
using a free-form name plus the lun_id attribute could be attractive,
because the lun_id attribute's value can be analyzed at ->store() and,
if it doesn't fit, an appropriate error can be easily returned using
just what we already have in configfs. Prevent seems better to me, too.

> 	But is max LUNs a requirement?  Why not just have the LUN count be
> the number of LUNs created?  In that case, you can allow or disallow
> discontiguous LUN IDs without worrying about a max ID.
> 

I am not sure, but I think that Michal would say that he wouldn't like
things to change once the gadget is up and running, so the number of
luns should be fixed up front. On the other hand, after the gadget is
set up, we can fail in ->make_group() to prevent the user from creating
additional luns with which we don't know what to do. So what you suggest
seems fine, but I would ask Michal to be on a safe side.

> [Really Hate Mkdir]
> 
> 	If you considered it appropriate for the LUN objects to be created
> by the kernel, the standard way would to have "echo 5 >luns_allowed"
> create LUN objects in sysfs.  Yes, sysfs.
> There's no reason you can't have things in configfs *and* sysfs.  My
> understanding of your code says to me that the mkdir approach is the way
> to go, but if you hate it, this works.
> 

I guess you are right, but see my point about luns' lifetime being
controlled by the user and their creation being performed by the kernel.

> [What About the Patch?]
> 
> 	In general, attribute setting that turns into created configfs
> objects is against the theory of configfs.

This looks like an authoritative answer.

> In practice, it's a nightmare
> locking change.  Coming into this discussion, I though you were doing
> dymanic things at ->make_group() time, but that is already supported.
> 	But I want to hear your thoughts.  I've dumped a bunch of alternate
> approaches here.  Do any seem to fit?  What other challenges do you see?
> 
> Joel
> 

Once upon a time, Felipe expressed interest in having the information
about endpoints and interfaces (mostly read-only stuff, but not all of it)
accessible in the gadget subsystem in configfs. This kind of information
is fully available only after the gadget has been bound, that is, after
the user created all the groups/items and filled all the attributes
and told the gadget to start (with e.g. filling some attribute or
creating a symlink). So this looks like a programmatic creation of
configfs directories. On the other hand, Felipe wanted it for debugging
purposes, so the user could just as well create the required directories
by hand when they need to, as Sebastian once suggested. Again, this requires
->make_group() to fail if all the information is not available yet.
But isn't it kind of similar to the luns' case? It is a bunch of objects
whose lifetime is controlled by the user (bind the gadget, unbind the
gadget),
but which are created by the kernel.
Another problem with this is that the user must at some point _guess_
what name to use in mkdir. Example:

For each interface there should be a folder. So the user must know how many
interfaces' folders to create and how to name them, like:

$ mkdir ...../some_usb_function/interface00
$ mkdir ...../some_usb_function/interface01
...
...
...

In each interface folder, there should be some endpoint folders and the user
must know how many endpoints there are and how to name them:

$ mkdir ...../some_usb_function/interface00/endpoint02
$ mkdir ...../some_usb_function/interface00/endpoint81

So probably there should be some attribute in some_usb_function,
which contains a list of available interface names, and some attribute in
interface#, which contains a list of available endpoint names.
Workable, but not so convenient. And there could be a problem, too,
if the gadget is unbound: there is no way to remove the user-created
directories other than by the user themselves, so there will be times
(i.e. between gadget unbind and manual rmdir) when these interface/endpoint
directories will not correspond to anything present in the system.

So maybe this kind of information should live in sysfs? But I don't
know now if it would be any easier. Joel? Felipe?

AP

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

@Joel in particular: please see my comment in the bottom.

On Monday, December 10, 2012 12:57 PM I wrote:
> Subject: RE: [RFC][PATCH] fs: configfs: programmatically create config
> groups
> 
> Hello Joel,
> 
> So you are alive, I'm glad to hear from you ;) Thank you for your
response.
> 
> On Saturday, December 08, 2012 12:18 AM Joel Becker wrote:
> > Subject: Re: [RFC][PATCH] fs: configfs: programmatically create config
> > groups
> >
> > Hey Guys,
> > 	Sorry I missed this for a while.  I'll make a couple of inline
> > comments, and then I'll summarize my (incomplete) thoughts at the
bottom.
> >
> > On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej Siewior
> wrote:
> > > On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> > > >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> > > >>>How should a generic tool know what kind of actions are needed for
> > > >>>given function to be removed?  If you ask me, there should be a way
> > > >>>to unbind gadget and unload all modules without any specific
> > > >>>knowledge of the functions.  If there is no such mechanism, then
> > > >>>it's a bad user interface.
> >
> > 	Please remember that configfs is not a "user" interface, it's a
> > userspace<->kernelspace interface.  Like sysfs, it's not required to be
> > convenient for someone at a bash prompt.  My goal is that it is *usable*
> > from a bash prompt.  So it must be that you can create/destroy/configure
> > objects via mkdir/rmkdir/cat/echo, but you might have a lot of those
> > mkdir/echo combos to configure something.
> > When it comes to the "user" interface, a wrapper script or library
> should
> > be converting a user intention into all that boilerplate.
> >
> 
> If you say so there is little we can do, is there? :O
> 
> <snip>
> 
> >
> > 	Yeah, user tools are expected (and should be).
> >
> 
> ditto
> 
> > > Anyway, for this to work we have to go through Joel.
> > >
> > > >	rmdir udcs/*		# unload all UDCs
> > >
> > > No, for this you still have to rmmod :)
> > >
> > >
> > > >>>I think the question is of information flow direction.  If user
> > > >>>gives some information to the kernel, she should be the one
> > > >>>creating any necessary directories.  But if the information comes
> > > >>>from kernel to the user, the kernel should create the structure.
> >
> > 	This last paragraph actually describes the distinction between
> > configfs and sysfs.  More specifically, if userspace wants to create an
> > object in the kernel, it should use configfs.  If the kernel has created
> > an object on its own, it exposes it via sysfs.  It is a deliberate non-
> > goal for configfs to replicate sysfs behavior.
> 
> So if the lifetime of some object is controlled by the user, it belongs
> to configfs. If, on the other hand, the lifetime is controlled by the
> kernel, it belongs to sysfs. I think that the trouble with e.g. luns
> is that they might want to behave like both (at least in the "more
> automated"
> approach where they are programmatically created): they are created
> by the kernel (=> sysfs) but their lifetime is controlled by the user
> (=>configfs).
> 
> >
> > [General Thoughts]
> >
> > 	First let me restate your problem to see if I understand it.
> > 	You want to expose e.g. five LUNs.  They should eventually appear in
> > configfs as five items to configure (.../{lun0,lun1,...lun4}/.  The
> > current configfs way to do this is to have your setup script do:
> >
> > 	cd /cfg/.../mass_storage
> > 	mkdir lun0
> > 	echo blah >lun0/attr1
> > 	echo blahh >lun0/attr2
> > 	mkdir lun1
> > 	echo blag >lun1/attr1
> > 	echo blagg >lun1/attr1
> > 	...
> >
> > I think the primary concern expressed by Andrzej is that a random user
> > could come along and say "mkdir lun8", even though the gadget cannot
> > support it.  A secondary concern from Michal is that userspace has to
> run
> > all of those mkdirs.  The thread has described varying solutions.
> > 	If these original directories were default_groups, you could
> > disallow any child creation and not require the setup script to create
> > directories.  Here I will point out that you *can* create default_groups
> > programmatically.  The default_groups pointer on each configfs_group can
> > be different.  ->make_group() can attach whatever groups it wants.
> > If this would work, it would fit both of your concerns, but you do not
> > have a priori knowledge of the LUN count.
> > 	Your original email proposed that the max lun be set like so:
> >
> > 	cd /cfg/.../mass_storage
> > 	echo 5 >luns_allowed
> >
> > There are then a couple of proposed ways to enforce the limit.  Your
> > ->create_group() idea wants luns_allowed to be able to create subdirs in
> > the ->store() function.  No ->mkdir() is provided, so a lunN cannot be
> > created by hand.
> > 	The ->create_group() idea has three challenges.  First, it creates
> > "default" groups *after* the object is created.  This makes no sense if
> > they are attributes of the toplevel object.  Second, it volates the
> > configfs mantra that user-generated objects are explicitly created.
> > The kicker, however, is the technical problem.  configfs explicitly
> > forbids tree changes inside attribute operations for deadlock reasons.
> > Trying to create new tree parts in luns_allowed->store() is exactly
that.
> > 	Another suggestion (I think I read this in the thread) would be to
> > have ->mkdir() function parse the name and check that it is lunN, where
> N
> > is less than the limit.  I think that this was shot down because of
> > parsing the LUN name.  I don't think that's too terrible, but there's
> > certainly room for argument.
> >
> > [Possible Solutions]
> >
> > 	Before coming back to the proposed patch, let's look at what I might
> > do if I were fitting this into configfs.
> > 	I would actually do what I described at first:
> >
> > 	cd /cfg/.../mass_storage
> > 	mkdir lun0
> > 	echo blah >lun0/attr1
> > 	...
> >
> > This is idiomatic configfs usage.  Sebastian noted this earlier in the
> > thread.  Michal thought the repeated mkdirs were a problem ("I think
> > that's suboptimal, and we can do better").  This is Michal's objection
> to
> > making the user do work.  This is where I point out we are not designing
> > configfs interfaces for ease of end-user use.  We're defining them for
> > transparent and discoverable interfaces for tools to create things in
> the
> > kernel.  So the repeated mkdir()s are actually a good thing from my
> > perspective; userspace is telling the kernel to create LUN objects.
> > 	But this doesn't alleviate Andrzej's primary concern, nor does it
> > answer Michal's concerns about corner cases.  Let's see if we can solve
> > those.
> > 	First, what about preventing extraneous LUNs?  There are two
> > approaches: the prevent approach, and the ignore approach.  In the
> Target
> > module, they take the ignore approach.  If you create a LUN that is
> > outside the scope, they just ignore it.  So a user can make 10 LUNs, but
> > if only five are allowed, the other five are ignored.  In the prevent
> > approach, we try to fail the mkdir() when we go over the limit.
> 
> I think I like the prevent approach better, because I don't know what to
> do with the unused luns in the ignore approach. I also think that it would
> be nice if configfs described the actual state of the system instead of
> presenting all the possible user-created garbage (i.e. something which is
> ignored).
> 
> > 	So you've set a limit with "echo 5 >luns_allowed".  How do you
> > restrict?  If you don't care about contiguous LUN numbering, you could
> > literally do this in your make_group():
> >
> > 	if (count_luns(parent) >= parent->luns_allowed)
> > 		return -ENOSPC;
> >
> > Note that we don't check the LUN number; SCSI does not require
> contiguous
> > numbering.  It does require LUN 0, which should probably be a default
> > group.  So your lun count starts at 1.
> > 	What if you decide you really want contiguous LUN numbers, which is
> > considered nice to have?  Then we get to Michal's concern about parsing
> > the name.  If you "mkdir lun1", the code has to confirm that it
> > a) starts with "lun", b) finishes with a parsable number, c) that number
> > is < luns_allowed.
> > 	But you need to do the parsing anyway!  If the LUN is created via
> > mkdir(), somehow the gadget system needs to know what LUN number to
> > advertise.  I would actually decouple it from the name.  The name should
> > be free-form; if it happens to be comprehensible, that's good.  Imagine
> > this:
> > 	cd /cfg/.../mass_storage
> > 	echo 4 >max_lun_id
> > 	mkdir lun100
> > 	echo 1 >lun100/lun_id
> >
> > The LUN actually has the ID of 1.  The fact that the directory name is
> > wrong is irrelevant to the gadget processing; non-hackers use the
> tooling,
> > and the tooling should  make sane names.  Then your
> > ->make_group() function can choose either approach to avoiding LUN IDs
> > that are too large.  It can prevent:
> >
> > 	lun_id->store() {
> > 		if (lun_id_exists(parent, value))
> > 			return ERR_PTR(-EINVAL);
> > 		if (value > parent->max_lun_id)
> > 			return ERR_PTR(-ENOSPC) /* or EINVAL */
> > 		setup_lun();
> > 		make_live();
> > 	}
> >
> > Or it can ignore:
> >
> > 	lun_id->store() {
> > 		if (lun_id_exists(parent, value))
> > 			return ERR_PTR(-EINVAL);
> > 		if (value <= parent->max_lun_id) {
> > 			setup_lun();
> > 			make_live();
> > 		}
> > 		/* Do nothing for the ignored sucker */
> > 	}
> >
> > 	I think that I, personally, would go with this <name>/lun_id scheme.
> > Given a requirement for contiguous LUNs and the max_lun_id restriction,
> > I'd go with the "prevent" option myself.
> 
> Taking into account what you say about the expected userspace tooling
> using a free-form name plus the lun_id attribute could be attractive,
> because the lun_id attribute's value can be analyzed at ->store() and,
> if it doesn't fit, an appropriate error can be easily returned using
> just what we already have in configfs. Prevent seems better to me, too.
> 
> > 	But is max LUNs a requirement?  Why not just have the LUN count be
> > the number of LUNs created?  In that case, you can allow or disallow
> > discontiguous LUN IDs without worrying about a max ID.
> >
> 
> I am not sure, but I think that Michal would say that he wouldn't like
> things to change once the gadget is up and running, so the number of
> luns should be fixed up front. On the other hand, after the gadget is
> set up, we can fail in ->make_group() to prevent the user from creating
> additional luns with which we don't know what to do. So what you suggest
> seems fine, but I would ask Michal to be on a safe side.
> 
> > [Really Hate Mkdir]
> >
> > 	If you considered it appropriate for the LUN objects to be created
> > by the kernel, the standard way would to have "echo 5 >luns_allowed"
> > create LUN objects in sysfs.  Yes, sysfs.
> > There's no reason you can't have things in configfs *and* sysfs.  My
> > understanding of your code says to me that the mkdir approach is the way
> > to go, but if you hate it, this works.
> >
> 
> I guess you are right, but see my point about luns' lifetime being
> controlled by the user and their creation being performed by the kernel.
> 
> > [What About the Patch?]
> >
> > 	In general, attribute setting that turns into created configfs
> > objects is against the theory of configfs.
> 
> This looks like an authoritative answer.
> 
> > In practice, it's a nightmare
> > locking change.  Coming into this discussion, I though you were doing
> > dymanic things at ->make_group() time, but that is already supported.
> > 	But I want to hear your thoughts.  I've dumped a bunch of alternate
> > approaches here.  Do any seem to fit?  What other challenges do you see?
> >
> > Joel
> >
> 
> Once upon a time, Felipe expressed interest in having the information
> about endpoints and interfaces (mostly read-only stuff, but not all of it)
> accessible in the gadget subsystem in configfs. This kind of information
> is fully available only after the gadget has been bound, that is, after
> the user created all the groups/items and filled all the attributes
> and told the gadget to start (with e.g. filling some attribute or
> creating a symlink). So this looks like a programmatic creation of
> configfs directories. On the other hand, Felipe wanted it for debugging
> purposes, so the user could just as well create the required directories
> by hand when they need to, as Sebastian once suggested. Again, this
> requires
> ->make_group() to fail if all the information is not available yet.
> But isn't it kind of similar to the luns' case? It is a bunch of objects
> whose lifetime is controlled by the user (bind the gadget, unbind the
> gadget),
> but which are created by the kernel.
> Another problem with this is that the user must at some point _guess_
> what name to use in mkdir. Example:
> 
> For each interface there should be a folder. So the user must know how
> many
> interfaces' folders to create and how to name them, like:
> 
> $ mkdir ...../some_usb_function/interface00
> $ mkdir ...../some_usb_function/interface01
> ...
> ...
> ...
> 
> In each interface folder, there should be some endpoint folders and the
> user
> must know how many endpoints there are and how to name them:
> 
> $ mkdir ...../some_usb_function/interface00/endpoint02
> $ mkdir ...../some_usb_function/interface00/endpoint81
> 
> So probably there should be some attribute in some_usb_function,
> which contains a list of available interface names, and some attribute in
> interface#, which contains a list of available endpoint names.
> Workable, but not so convenient. And there could be a problem, too,
> if the gadget is unbound: there is no way to remove the user-created
> directories other than by the user themselves, so there will be times
> (i.e. between gadget unbind and manual rmdir) when these
> interface/endpoint
> directories will not correspond to anything present in the system.
> 
> So maybe this kind of information should live in sysfs? But I don't
> know now if it would be any easier. Joel? Felipe?
> 

I forgot to mention, representing udcs (USB Device Controllers) in
configfs is similar to interfaces/endpoints: the user needs to guess
what name to use in mkdir, e.g. in:

$ mkdir ....../udcs/s3c-hsotg

the poor user needs to know that the udc is actually called s3c-hsotg.
And after the udc goes away from the system, the s3c-hsotg directory
remains in the filesystem until the user deletes it (as nobody else
but the user can do it). Again, an attribute like available_udcs in
the udcs directory can help. But still, there will be times when
its contents will not be consistent with the directories actually
present.

@Joel: Any thoughts about it? Commitable items? E.g. the user creates
whatever they want, so the "uncommitted" directory contains "garbage",
that is directories which do not have to correspond to anything
actually present in the system, but on commiting the gadget's item
(group in fact) everything is verified and only "correct" gadgets
are actually bound? Then, in the "uncommitted" directory the user
can still do whatever they want, but this is not effective until
uncommitting the old gadget and committing it again? There is just
a tiny issue with committable items: Are they implemented? :O

AP

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Felipe Balbi]

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

Hi,

On Mon, Dec 10, 2012 at 03:17:34PM +0100, Andrzej Pietrasiewicz wrote:
> @Joel in particular: please see my comment in the bottom.
> 
> On Monday, December 10, 2012 12:57 PM I wrote:
> > Subject: RE: [RFC][PATCH] fs: configfs: programmatically create config
> > groups
> > 
> > Hello Joel,
> > 
> > So you are alive, I'm glad to hear from you ;) Thank you for your
> response.
> > 
> > On Saturday, December 08, 2012 12:18 AM Joel Becker wrote:
> > > Subject: Re: [RFC][PATCH] fs: configfs: programmatically create config
> > > groups
> > >
> > > Hey Guys,
> > > 	Sorry I missed this for a while.  I'll make a couple of inline
> > > comments, and then I'll summarize my (incomplete) thoughts at the
> bottom.
> > >
> > > On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej Siewior
> > wrote:
> > > > On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> > > > >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> > > > >>>How should a generic tool know what kind of actions are needed for
> > > > >>>given function to be removed?  If you ask me, there should be a way
> > > > >>>to unbind gadget and unload all modules without any specific
> > > > >>>knowledge of the functions.  If there is no such mechanism, then
> > > > >>>it's a bad user interface.
> > >
> > > 	Please remember that configfs is not a "user" interface, it's a
> > > userspace<->kernelspace interface.  Like sysfs, it's not required to be
> > > convenient for someone at a bash prompt.  My goal is that it is *usable*
> > > from a bash prompt.  So it must be that you can create/destroy/configure
> > > objects via mkdir/rmkdir/cat/echo, but you might have a lot of those
> > > mkdir/echo combos to configure something.
> > > When it comes to the "user" interface, a wrapper script or library
> > should
> > > be converting a user intention into all that boilerplate.
> > >
> > 
> > If you say so there is little we can do, is there? :O
> > 
> > <snip>
> > 
> > >
> > > 	Yeah, user tools are expected (and should be).
> > >
> > 
> > ditto
> > 
> > > > Anyway, for this to work we have to go through Joel.
> > > >
> > > > >	rmdir udcs/*		# unload all UDCs
> > > >
> > > > No, for this you still have to rmmod :)
> > > >
> > > >
> > > > >>>I think the question is of information flow direction.  If user
> > > > >>>gives some information to the kernel, she should be the one
> > > > >>>creating any necessary directories.  But if the information comes
> > > > >>>from kernel to the user, the kernel should create the structure.
> > >
> > > 	This last paragraph actually describes the distinction between
> > > configfs and sysfs.  More specifically, if userspace wants to create an
> > > object in the kernel, it should use configfs.  If the kernel has created
> > > an object on its own, it exposes it via sysfs.  It is a deliberate non-
> > > goal for configfs to replicate sysfs behavior.
> > 
> > So if the lifetime of some object is controlled by the user, it belongs
> > to configfs. If, on the other hand, the lifetime is controlled by the
> > kernel, it belongs to sysfs. I think that the trouble with e.g. luns
> > is that they might want to behave like both (at least in the "more
> > automated"
> > approach where they are programmatically created): they are created
> > by the kernel (=> sysfs) but their lifetime is controlled by the user
> > (=>configfs).
> > 
> > >
> > > [General Thoughts]
> > >
> > > 	First let me restate your problem to see if I understand it.
> > > 	You want to expose e.g. five LUNs.  They should eventually appear in
> > > configfs as five items to configure (.../{lun0,lun1,...lun4}/.  The
> > > current configfs way to do this is to have your setup script do:
> > >
> > > 	cd /cfg/.../mass_storage
> > > 	mkdir lun0
> > > 	echo blah >lun0/attr1
> > > 	echo blahh >lun0/attr2
> > > 	mkdir lun1
> > > 	echo blag >lun1/attr1
> > > 	echo blagg >lun1/attr1
> > > 	...
> > >
> > > I think the primary concern expressed by Andrzej is that a random user
> > > could come along and say "mkdir lun8", even though the gadget cannot
> > > support it.  A secondary concern from Michal is that userspace has to
> > run
> > > all of those mkdirs.  The thread has described varying solutions.
> > > 	If these original directories were default_groups, you could
> > > disallow any child creation and not require the setup script to create
> > > directories.  Here I will point out that you *can* create default_groups
> > > programmatically.  The default_groups pointer on each configfs_group can
> > > be different.  ->make_group() can attach whatever groups it wants.
> > > If this would work, it would fit both of your concerns, but you do not
> > > have a priori knowledge of the LUN count.
> > > 	Your original email proposed that the max lun be set like so:
> > >
> > > 	cd /cfg/.../mass_storage
> > > 	echo 5 >luns_allowed
> > >
> > > There are then a couple of proposed ways to enforce the limit.  Your
> > > ->create_group() idea wants luns_allowed to be able to create subdirs in
> > > the ->store() function.  No ->mkdir() is provided, so a lunN cannot be
> > > created by hand.
> > > 	The ->create_group() idea has three challenges.  First, it creates
> > > "default" groups *after* the object is created.  This makes no sense if
> > > they are attributes of the toplevel object.  Second, it volates the
> > > configfs mantra that user-generated objects are explicitly created.
> > > The kicker, however, is the technical problem.  configfs explicitly
> > > forbids tree changes inside attribute operations for deadlock reasons.
> > > Trying to create new tree parts in luns_allowed->store() is exactly
> that.
> > > 	Another suggestion (I think I read this in the thread) would be to
> > > have ->mkdir() function parse the name and check that it is lunN, where
> > N
> > > is less than the limit.  I think that this was shot down because of
> > > parsing the LUN name.  I don't think that's too terrible, but there's
> > > certainly room for argument.
> > >
> > > [Possible Solutions]
> > >
> > > 	Before coming back to the proposed patch, let's look at what I might
> > > do if I were fitting this into configfs.
> > > 	I would actually do what I described at first:
> > >
> > > 	cd /cfg/.../mass_storage
> > > 	mkdir lun0
> > > 	echo blah >lun0/attr1
> > > 	...
> > >
> > > This is idiomatic configfs usage.  Sebastian noted this earlier in the
> > > thread.  Michal thought the repeated mkdirs were a problem ("I think
> > > that's suboptimal, and we can do better").  This is Michal's objection
> > to
> > > making the user do work.  This is where I point out we are not designing
> > > configfs interfaces for ease of end-user use.  We're defining them for
> > > transparent and discoverable interfaces for tools to create things in
> > the
> > > kernel.  So the repeated mkdir()s are actually a good thing from my
> > > perspective; userspace is telling the kernel to create LUN objects.
> > > 	But this doesn't alleviate Andrzej's primary concern, nor does it
> > > answer Michal's concerns about corner cases.  Let's see if we can solve
> > > those.
> > > 	First, what about preventing extraneous LUNs?  There are two
> > > approaches: the prevent approach, and the ignore approach.  In the
> > Target
> > > module, they take the ignore approach.  If you create a LUN that is
> > > outside the scope, they just ignore it.  So a user can make 10 LUNs, but
> > > if only five are allowed, the other five are ignored.  In the prevent
> > > approach, we try to fail the mkdir() when we go over the limit.
> > 
> > I think I like the prevent approach better, because I don't know what to
> > do with the unused luns in the ignore approach. I also think that it would
> > be nice if configfs described the actual state of the system instead of
> > presenting all the possible user-created garbage (i.e. something which is
> > ignored).
> > 
> > > 	So you've set a limit with "echo 5 >luns_allowed".  How do you
> > > restrict?  If you don't care about contiguous LUN numbering, you could
> > > literally do this in your make_group():
> > >
> > > 	if (count_luns(parent) >= parent->luns_allowed)
> > > 		return -ENOSPC;
> > >
> > > Note that we don't check the LUN number; SCSI does not require
> > contiguous
> > > numbering.  It does require LUN 0, which should probably be a default
> > > group.  So your lun count starts at 1.
> > > 	What if you decide you really want contiguous LUN numbers, which is
> > > considered nice to have?  Then we get to Michal's concern about parsing
> > > the name.  If you "mkdir lun1", the code has to confirm that it
> > > a) starts with "lun", b) finishes with a parsable number, c) that number
> > > is < luns_allowed.
> > > 	But you need to do the parsing anyway!  If the LUN is created via
> > > mkdir(), somehow the gadget system needs to know what LUN number to
> > > advertise.  I would actually decouple it from the name.  The name should
> > > be free-form; if it happens to be comprehensible, that's good.  Imagine
> > > this:
> > > 	cd /cfg/.../mass_storage
> > > 	echo 4 >max_lun_id
> > > 	mkdir lun100
> > > 	echo 1 >lun100/lun_id
> > >
> > > The LUN actually has the ID of 1.  The fact that the directory name is
> > > wrong is irrelevant to the gadget processing; non-hackers use the
> > tooling,
> > > and the tooling should  make sane names.  Then your
> > > ->make_group() function can choose either approach to avoiding LUN IDs
> > > that are too large.  It can prevent:
> > >
> > > 	lun_id->store() {
> > > 		if (lun_id_exists(parent, value))
> > > 			return ERR_PTR(-EINVAL);
> > > 		if (value > parent->max_lun_id)
> > > 			return ERR_PTR(-ENOSPC) /* or EINVAL */
> > > 		setup_lun();
> > > 		make_live();
> > > 	}
> > >
> > > Or it can ignore:
> > >
> > > 	lun_id->store() {
> > > 		if (lun_id_exists(parent, value))
> > > 			return ERR_PTR(-EINVAL);
> > > 		if (value <= parent->max_lun_id) {
> > > 			setup_lun();
> > > 			make_live();
> > > 		}
> > > 		/* Do nothing for the ignored sucker */
> > > 	}
> > >
> > > 	I think that I, personally, would go with this <name>/lun_id scheme.
> > > Given a requirement for contiguous LUNs and the max_lun_id restriction,
> > > I'd go with the "prevent" option myself.
> > 
> > Taking into account what you say about the expected userspace tooling
> > using a free-form name plus the lun_id attribute could be attractive,
> > because the lun_id attribute's value can be analyzed at ->store() and,
> > if it doesn't fit, an appropriate error can be easily returned using
> > just what we already have in configfs. Prevent seems better to me, too.
> > 
> > > 	But is max LUNs a requirement?  Why not just have the LUN count be
> > > the number of LUNs created?  In that case, you can allow or disallow
> > > discontiguous LUN IDs without worrying about a max ID.
> > >
> > 
> > I am not sure, but I think that Michal would say that he wouldn't like
> > things to change once the gadget is up and running, so the number of
> > luns should be fixed up front. On the other hand, after the gadget is
> > set up, we can fail in ->make_group() to prevent the user from creating
> > additional luns with which we don't know what to do. So what you suggest
> > seems fine, but I would ask Michal to be on a safe side.
> > 
> > > [Really Hate Mkdir]
> > >
> > > 	If you considered it appropriate for the LUN objects to be created
> > > by the kernel, the standard way would to have "echo 5 >luns_allowed"
> > > create LUN objects in sysfs.  Yes, sysfs.
> > > There's no reason you can't have things in configfs *and* sysfs.  My
> > > understanding of your code says to me that the mkdir approach is the way
> > > to go, but if you hate it, this works.
> > >
> > 
> > I guess you are right, but see my point about luns' lifetime being
> > controlled by the user and their creation being performed by the kernel.
> > 
> > > [What About the Patch?]
> > >
> > > 	In general, attribute setting that turns into created configfs
> > > objects is against the theory of configfs.
> > 
> > This looks like an authoritative answer.
> > 
> > > In practice, it's a nightmare
> > > locking change.  Coming into this discussion, I though you were doing
> > > dymanic things at ->make_group() time, but that is already supported.
> > > 	But I want to hear your thoughts.  I've dumped a bunch of alternate
> > > approaches here.  Do any seem to fit?  What other challenges do you see?
> > >
> > > Joel
> > >
> > 
> > Once upon a time, Felipe expressed interest in having the information
> > about endpoints and interfaces (mostly read-only stuff, but not all of it)
> > accessible in the gadget subsystem in configfs. This kind of information
> > is fully available only after the gadget has been bound, that is, after
> > the user created all the groups/items and filled all the attributes
> > and told the gadget to start (with e.g. filling some attribute or
> > creating a symlink). So this looks like a programmatic creation of
> > configfs directories. On the other hand, Felipe wanted it for debugging
> > purposes, so the user could just as well create the required directories
> > by hand when they need to, as Sebastian once suggested. Again, this
> > requires
> > ->make_group() to fail if all the information is not available yet.
> > But isn't it kind of similar to the luns' case? It is a bunch of objects
> > whose lifetime is controlled by the user (bind the gadget, unbind the
> > gadget),
> > but which are created by the kernel.
> > Another problem with this is that the user must at some point _guess_
> > what name to use in mkdir. Example:
> > 
> > For each interface there should be a folder. So the user must know how
> > many
> > interfaces' folders to create and how to name them, like:
> > 
> > $ mkdir ...../some_usb_function/interface00
> > $ mkdir ...../some_usb_function/interface01
> > ...
> > ...
> > ...
> > 
> > In each interface folder, there should be some endpoint folders and the
> > user
> > must know how many endpoints there are and how to name them:
> > 
> > $ mkdir ...../some_usb_function/interface00/endpoint02
> > $ mkdir ...../some_usb_function/interface00/endpoint81
> > 
> > So probably there should be some attribute in some_usb_function,
> > which contains a list of available interface names, and some attribute in
> > interface#, which contains a list of available endpoint names.
> > Workable, but not so convenient. And there could be a problem, too,
> > if the gadget is unbound: there is no way to remove the user-created
> > directories other than by the user themselves, so there will be times
> > (i.e. between gadget unbind and manual rmdir) when these
> > interface/endpoint
> > directories will not correspond to anything present in the system.
> > 
> > So maybe this kind of information should live in sysfs? But I don't
> > know now if it would be any easier. Joel? Felipe?
> > 
> 
> I forgot to mention, representing udcs (USB Device Controllers) in
> configfs is similar to interfaces/endpoints: the user needs to guess
> what name to use in mkdir, e.g. in:
> 
> $ mkdir ....../udcs/s3c-hsotg

why would you even require a UDC directory to be created ? You
*register* the UDC with the udc-class and that should be enough to
expose all UDCs under /sys/kernel/config/..../udcs/*

The fact is that in most cases you will have multiple instances of the
*same IP* not different UDCs with different names as you seem to think.
While that's possible, it'll be a rare situation.

So what we will have in 95% of the cases will be:

/...../udcs/dwc3.0
/...../udcs/dwc3.1
...
/...../udcs/dwc3.N

The only thing the poor user needs to know is that *.0 is the first
controller, *.1 is the second controller and so on. Since that's
hardcoded in HW anyway (every instance has its own IRQ, address space,
etc, and each of them is physically attached to a single port), you can
kinda make that assumption in code too.

-- 
balbi

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

RE: [RFC][PATCH] fs: configfs: programmatically create config groups

[Andrzej Pietrasiewicz]

On Monday, December 10, 2012 3:34 PM Felipe Balbi wrote:
> Subject: Re: [RFC][PATCH] fs: configfs: programmatically create config
> groups
> 
> Hi,
> 
> On Mon, Dec 10, 2012 at 03:17:34PM +0100, Andrzej Pietrasiewicz wrote:
> > @Joel in particular: please see my comment in the bottom.
> >
> > On Monday, December 10, 2012 12:57 PM I wrote:
> > > Subject: RE: [RFC][PATCH] fs: configfs: programmatically create
> > > config groups
> > >
> > > Hello Joel,
> > >
> > > So you are alive, I'm glad to hear from you ;) Thank you for your
> > response.
> > >
> > > On Saturday, December 08, 2012 12:18 AM Joel Becker wrote:
> > > > Subject: Re: [RFC][PATCH] fs: configfs: programmatically create
> > > > config groups
> > > >
> > > > Hey Guys,
> > > > 	Sorry I missed this for a while.  I'll make a couple of
inline
> > > > comments, and then I'll summarize my (incomplete) thoughts at the
> > bottom.
> > > >
> > > > On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej
> > > > Siewior
> > > wrote:
> > > > > On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> > > > > >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> > > > > >>>How should a generic tool know what kind of actions are
> > > > > >>>needed for given function to be removed?  If you ask me,
> > > > > >>>there should be a way to unbind gadget and unload all modules
> > > > > >>>without any specific knowledge of the functions.  If there is
> > > > > >>>no such mechanism, then it's a bad user interface.
> > > >
> > > > 	Please remember that configfs is not a "user" interface,
it's
> a
> > > > userspace<->kernelspace interface.  Like sysfs, it's not required
> > > > to be convenient for someone at a bash prompt.  My goal is that it
> > > > is *usable* from a bash prompt.  So it must be that you can
> > > > create/destroy/configure objects via mkdir/rmkdir/cat/echo, but
> > > > you might have a lot of those mkdir/echo combos to configure
> something.
> > > > When it comes to the "user" interface, a wrapper script or library
> > > should
> > > > be converting a user intention into all that boilerplate.
> > > >
> > >
> > > If you say so there is little we can do, is there? :O
> > >
> > > <snip>
> > >
> > > >
> > > > 	Yeah, user tools are expected (and should be).
> > > >
> > >
> > > ditto
> > >
> > > > > Anyway, for this to work we have to go through Joel.
> > > > >
> > > > > >	rmdir udcs/*		# unload all UDCs
> > > > >
> > > > > No, for this you still have to rmmod :)
> > > > >
> > > > >
> > > > > >>>I think the question is of information flow direction.  If
> > > > > >>>user gives some information to the kernel, she should be the
> > > > > >>>one creating any necessary directories.  But if the
> > > > > >>>information comes from kernel to the user, the kernel should
> create the structure.
> > > >
> > > > 	This last paragraph actually describes the distinction
between
> > > > configfs and sysfs.  More specifically, if userspace wants to
> > > > create an object in the kernel, it should use configfs.  If the
> > > > kernel has created an object on its own, it exposes it via sysfs.
> > > > It is a deliberate non- goal for configfs to replicate sysfs
> behavior.
> > >
> > > So if the lifetime of some object is controlled by the user, it
> > > belongs to configfs. If, on the other hand, the lifetime is
> > > controlled by the kernel, it belongs to sysfs. I think that the
> > > trouble with e.g. luns is that they might want to behave like both
> > > (at least in the "more automated"
> > > approach where they are programmatically created): they are created
> > > by the kernel (=> sysfs) but their lifetime is controlled by the
> > > user (=>configfs).
> > >
> > > >
> > > > [General Thoughts]
> > > >
> > > > 	First let me restate your problem to see if I understand it.
> > > > 	You want to expose e.g. five LUNs.  They should eventually
> appear
> > > > in configfs as five items to configure (.../{lun0,lun1,...lun4}/.
> > > > The current configfs way to do this is to have your setup script do:
> > > >
> > > > 	cd /cfg/.../mass_storage
> > > > 	mkdir lun0
> > > > 	echo blah >lun0/attr1
> > > > 	echo blahh >lun0/attr2
> > > > 	mkdir lun1
> > > > 	echo blag >lun1/attr1
> > > > 	echo blagg >lun1/attr1
> > > > 	...
> > > >
> > > > I think the primary concern expressed by Andrzej is that a random
> > > > user could come along and say "mkdir lun8", even though the gadget
> > > > cannot support it.  A secondary concern from Michal is that
> > > > userspace has to
> > > run
> > > > all of those mkdirs.  The thread has described varying solutions.
> > > > 	If these original directories were default_groups, you could
> > > > disallow any child creation and not require the setup script to
> > > > create directories.  Here I will point out that you *can* create
> > > > default_groups programmatically.  The default_groups pointer on
> > > > each configfs_group can be different.  ->make_group() can attach
> whatever groups it wants.
> > > > If this would work, it would fit both of your concerns, but you do
> > > > not have a priori knowledge of the LUN count.
> > > > 	Your original email proposed that the max lun be set like
so:
> > > >
> > > > 	cd /cfg/.../mass_storage
> > > > 	echo 5 >luns_allowed
> > > >
> > > > There are then a couple of proposed ways to enforce the limit.
> > > > Your
> > > > ->create_group() idea wants luns_allowed to be able to create
> > > > ->subdirs in
> > > > the ->store() function.  No ->mkdir() is provided, so a lunN
> > > > cannot be created by hand.
> > > > 	The ->create_group() idea has three challenges.  First, it
> > > > creates "default" groups *after* the object is created.  This
> > > > makes no sense if they are attributes of the toplevel object.
> > > > Second, it volates the configfs mantra that user-generated objects
> are explicitly created.
> > > > The kicker, however, is the technical problem.  configfs
> > > > explicitly forbids tree changes inside attribute operations for
> deadlock reasons.
> > > > Trying to create new tree parts in luns_allowed->store() is
> > > > exactly
> > that.
> > > > 	Another suggestion (I think I read this in the thread) would
> be
> > > > to have ->mkdir() function parse the name and check that it is
> > > > lunN, where
> > > N
> > > > is less than the limit.  I think that this was shot down because
> > > > of parsing the LUN name.  I don't think that's too terrible, but
> > > > there's certainly room for argument.
> > > >
> > > > [Possible Solutions]
> > > >
> > > > 	Before coming back to the proposed patch, let's look at what
I
> > > > might do if I were fitting this into configfs.
> > > > 	I would actually do what I described at first:
> > > >
> > > > 	cd /cfg/.../mass_storage
> > > > 	mkdir lun0
> > > > 	echo blah >lun0/attr1
> > > > 	...
> > > >
> > > > This is idiomatic configfs usage.  Sebastian noted this earlier in
> > > > the thread.  Michal thought the repeated mkdirs were a problem ("I
> > > > think that's suboptimal, and we can do better").  This is Michal's
> > > > objection
> > > to
> > > > making the user do work.  This is where I point out we are not
> > > > designing configfs interfaces for ease of end-user use.  We're
> > > > defining them for transparent and discoverable interfaces for
> > > > tools to create things in
> > > the
> > > > kernel.  So the repeated mkdir()s are actually a good thing from
> > > > my perspective; userspace is telling the kernel to create LUN
> objects.
> > > > 	But this doesn't alleviate Andrzej's primary concern, nor
does
> it
> > > > answer Michal's concerns about corner cases.  Let's see if we can
> > > > solve those.
> > > > 	First, what about preventing extraneous LUNs?  There are two
> > > > approaches: the prevent approach, and the ignore approach.  In the
> > > Target
> > > > module, they take the ignore approach.  If you create a LUN that
> > > > is outside the scope, they just ignore it.  So a user can make 10
> > > > LUNs, but if only five are allowed, the other five are ignored.
> > > > In the prevent approach, we try to fail the mkdir() when we go over
> the limit.
> > >
> > > I think I like the prevent approach better, because I don't know
> > > what to do with the unused luns in the ignore approach. I also think
> > > that it would be nice if configfs described the actual state of the
> > > system instead of presenting all the possible user-created garbage
> > > (i.e. something which is ignored).
> > >
> > > > 	So you've set a limit with "echo 5 >luns_allowed".  How do
you
> > > > restrict?  If you don't care about contiguous LUN numbering, you
> > > > could literally do this in your make_group():
> > > >
> > > > 	if (count_luns(parent) >= parent->luns_allowed)
> > > > 		return -ENOSPC;
> > > >
> > > > Note that we don't check the LUN number; SCSI does not require
> > > contiguous
> > > > numbering.  It does require LUN 0, which should probably be a
> > > > default group.  So your lun count starts at 1.
> > > > 	What if you decide you really want contiguous LUN numbers,
> which
> > > > is considered nice to have?  Then we get to Michal's concern about
> > > > parsing the name.  If you "mkdir lun1", the code has to confirm
> > > > that it
> > > > a) starts with "lun", b) finishes with a parsable number, c) that
> > > > number is < luns_allowed.
> > > > 	But you need to do the parsing anyway!  If the LUN is
created
> via
> > > > mkdir(), somehow the gadget system needs to know what LUN number
> > > > to advertise.  I would actually decouple it from the name.  The
> > > > name should be free-form; if it happens to be comprehensible,
> > > > that's good.  Imagine
> > > > this:
> > > > 	cd /cfg/.../mass_storage
> > > > 	echo 4 >max_lun_id
> > > > 	mkdir lun100
> > > > 	echo 1 >lun100/lun_id
> > > >
> > > > The LUN actually has the ID of 1.  The fact that the directory
> > > > name is wrong is irrelevant to the gadget processing; non-hackers
> > > > use the
> > > tooling,
> > > > and the tooling should  make sane names.  Then your
> > > > ->make_group() function can choose either approach to avoiding LUN
> > > > ->IDs
> > > > that are too large.  It can prevent:
> > > >
> > > > 	lun_id->store() {
> > > > 		if (lun_id_exists(parent, value))
> > > > 			return ERR_PTR(-EINVAL);
> > > > 		if (value > parent->max_lun_id)
> > > > 			return ERR_PTR(-ENOSPC) /* or EINVAL */
> > > > 		setup_lun();
> > > > 		make_live();
> > > > 	}
> > > >
> > > > Or it can ignore:
> > > >
> > > > 	lun_id->store() {
> > > > 		if (lun_id_exists(parent, value))
> > > > 			return ERR_PTR(-EINVAL);
> > > > 		if (value <= parent->max_lun_id) {
> > > > 			setup_lun();
> > > > 			make_live();
> > > > 		}
> > > > 		/* Do nothing for the ignored sucker */
> > > > 	}
> > > >
> > > > 	I think that I, personally, would go with this <name>/lun_id
> scheme.
> > > > Given a requirement for contiguous LUNs and the max_lun_id
> > > > restriction, I'd go with the "prevent" option myself.
> > >
> > > Taking into account what you say about the expected userspace
> > > tooling using a free-form name plus the lun_id attribute could be
> > > attractive, because the lun_id attribute's value can be analyzed at
> > > ->store() and, if it doesn't fit, an appropriate error can be easily
> > > returned using just what we already have in configfs. Prevent seems
> better to me, too.
> > >
> > > > 	But is max LUNs a requirement?  Why not just have the LUN
> count
> > > > be the number of LUNs created?  In that case, you can allow or
> > > > disallow discontiguous LUN IDs without worrying about a max ID.
> > > >
> > >
> > > I am not sure, but I think that Michal would say that he wouldn't
> > > like things to change once the gadget is up and running, so the
> > > number of luns should be fixed up front. On the other hand, after
> > > the gadget is set up, we can fail in ->make_group() to prevent the
> > > user from creating additional luns with which we don't know what to
> > > do. So what you suggest seems fine, but I would ask Michal to be on a
> safe side.
> > >
> > > > [Really Hate Mkdir]
> > > >
> > > > 	If you considered it appropriate for the LUN objects to be
> > > > created by the kernel, the standard way would to have "echo 5
> >luns_allowed"
> > > > create LUN objects in sysfs.  Yes, sysfs.
> > > > There's no reason you can't have things in configfs *and* sysfs.
> > > > My understanding of your code says to me that the mkdir approach
> > > > is the way to go, but if you hate it, this works.
> > > >
> > >
> > > I guess you are right, but see my point about luns' lifetime being
> > > controlled by the user and their creation being performed by the
> kernel.
> > >
> > > > [What About the Patch?]
> > > >
> > > > 	In general, attribute setting that turns into created
configfs
> > > > objects is against the theory of configfs.
> > >
> > > This looks like an authoritative answer.
> > >
> > > > In practice, it's a nightmare
> > > > locking change.  Coming into this discussion, I though you were
> > > > doing dymanic things at ->make_group() time, but that is already
> supported.
> > > > 	But I want to hear your thoughts.  I've dumped a bunch of
> > > > alternate approaches here.  Do any seem to fit?  What other
> challenges do you see?
> > > >
> > > > Joel
> > > >
> > >
> > > Once upon a time, Felipe expressed interest in having the
> > > information about endpoints and interfaces (mostly read-only stuff,
> > > but not all of it) accessible in the gadget subsystem in configfs.
> > > This kind of information is fully available only after the gadget
> > > has been bound, that is, after the user created all the groups/items
> > > and filled all the attributes and told the gadget to start (with
> > > e.g. filling some attribute or creating a symlink). So this looks
> > > like a programmatic creation of configfs directories. On the other
> > > hand, Felipe wanted it for debugging purposes, so the user could
> > > just as well create the required directories by hand when they need
> > > to, as Sebastian once suggested. Again, this requires
> > > ->make_group() to fail if all the information is not available yet.
> > > But isn't it kind of similar to the luns' case? It is a bunch of
> > > objects whose lifetime is controlled by the user (bind the gadget,
> > > unbind the gadget), but which are created by the kernel.
> > > Another problem with this is that the user must at some point
> > > _guess_ what name to use in mkdir. Example:
> > >
> > > For each interface there should be a folder. So the user must know
> > > how many interfaces' folders to create and how to name them, like:
> > >
> > > $ mkdir ...../some_usb_function/interface00
> > > $ mkdir ...../some_usb_function/interface01
> > > ...
> > > ...
> > > ...
> > >
> > > In each interface folder, there should be some endpoint folders and
> > > the user must know how many endpoints there are and how to name
> > > them:
> > >
> > > $ mkdir ...../some_usb_function/interface00/endpoint02
> > > $ mkdir ...../some_usb_function/interface00/endpoint81
> > >
> > > So probably there should be some attribute in some_usb_function,
> > > which contains a list of available interface names, and some
> > > attribute in interface#, which contains a list of available endpoint
> names.
> > > Workable, but not so convenient. And there could be a problem, too,
> > > if the gadget is unbound: there is no way to remove the user-created
> > > directories other than by the user themselves, so there will be
> > > times (i.e. between gadget unbind and manual rmdir) when these
> > > interface/endpoint directories will not correspond to anything
> > > present in the system.
> > >
> > > So maybe this kind of information should live in sysfs? But I don't
> > > know now if it would be any easier. Joel? Felipe?
> > >
> >
> > I forgot to mention, representing udcs (USB Device Controllers) in
> > configfs is similar to interfaces/endpoints: the user needs to guess
> > what name to use in mkdir, e.g. in:
> >
> > $ mkdir ....../udcs/s3c-hsotg
> 
> why would you even require a UDC directory to be created ? You
> *register* the UDC with the udc-class and that should be enough to expose
> all UDCs under /sys/kernel/config/..../udcs/*

Because in configfs the only way for something to appear under e.g.
/...../config/udcs is to create a directory with mkdir. This is
what Joel says.

Except perhaps for default groups which *must* be known at the
moment of creation of their parent directory. So, when the
/....../config/udcs wants to appear, its default groups,
if any, must be known at this point. I'm not sure if it can be
guaranteed that when the /...../config/udcs is created all
the UDCs are known (*registered*, if you will). Plus, UDCs
can be compiled as modules. If a UDC driver module is inserted, e.g.:

$ modprobe s3c-hsotg

but the /...../config/udcs directory already exists, then
there is no way for the new udc to appear there if there is no
programmatic directory creation. And it seems there will not
be. Ergo, mkdir must be used and the user *needs* to know
what name to use.

> 
> The fact is that in most cases you will have multiple instances of the
> *same IP* not different UDCs with different names as you seem to think.
> While that's possible, it'll be a rare situation.
> 

It doesn't matter if their directories need to be manually created with
mkdir.

> So what we will have in 95% of the cases will be:
> 
> /...../udcs/dwc3.0
> /...../udcs/dwc3.1
> ...
> /...../udcs/dwc3.N
> 
> The only thing the poor user needs to know is that *.0 is the first
> controller, *.1 is the second controller and so on.

But only once the directories are already there. But they need
to be created before.

AP

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Joel Becker]

On Mon, Dec 10, 2012 at 12:57:02PM +0100, Andrzej Pietrasiewicz wrote:
> Hello Joel,
> 
> So you are alive, I'm glad to hear from you ;) Thank you for your response.

	Yeah, sorry for missing the thread for so long.

> On Saturday, December 08, 2012 12:18 AM Joel Becker wrote:
> > Subject: Re: [RFC][PATCH] fs: configfs: programmatically create config
> > groups
> > 
> > Hey Guys,
> > 	Sorry I missed this for a while.  I'll make a couple of inline
> > comments, and then I'll summarize my (incomplete) thoughts at the bottom.
> > 
> > On Wed, Nov 28, 2012 at 02:50:13PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 11/28/2012 02:05 PM, Michal Nazarewicz wrote:
> > > >>On 11/27/2012 05:23 PM, Michal Nazarewicz wrote:
> > > >>>How should a generic tool know what kind of actions are needed for
> > > >>>given function to be removed?  If you ask me, there should be a way
> > > >>>to unbind gadget and unload all modules without any specific
> > > >>>knowledge of the functions.  If there is no such mechanism, then
> > > >>>it's a bad user interface.
> > 
> > 	Please remember that configfs is not a "user" interface, it's a
> > userspace<->kernelspace interface.  Like sysfs, it's not required to be
> > convenient for someone at a bash prompt.  My goal is that it is *usable*
> > from a bash prompt.  So it must be that you can create/destroy/configure
> > objects via mkdir/rmkdir/cat/echo, but you might have a lot of those
> > mkdir/echo combos to configure something.
> > When it comes to the "user" interface, a wrapper script or library should
> > be converting a user intention into all that boilerplate.
> > 
> 
> If you say so there is little we can do, is there? :O

	I don't want to make your life harder.  I'm describing the
motivations so that we can come to a consensus.  But configfs isn't an
end-user interface.  Consider the RAID ioctls
(/usr/include/linux/md/md_u.h).  Do you really expect that users will
manage their MD devices by writing programs that do "ioctl(/dev/md0,
GET_ARRAY_INFO)"?  No, you don't.  You expect that mdadm will exist.
	But ioctls are a pain.  Want to do something in a script or code
that mdadm doesn't handle?  Get ready to write ioctls.  In configfs, you
can do it in a shell script.  What about debugging?  Sometimes hackers
like us and sysadmins debugging problems like "mdadm doesn't work" need
to poke around.  If mdadm isn't working, they have to strace the ioctl
calls, then unpack the arguments and see what is happening.  With
configfs, instead, you have a filesystem tree to poke at.
	That's why I wrote it.  sysfs is the same way.  You and I can
poke about sysfs to get info, but most people just want udev to convert
sysfs info into a working system.
	Now, none of this means we shouldn't come up with the simplest
configfs interface for your gadgets.  But if we want to add features to
configfs (rather than to your use of it), I want to understand how this
helps us as userspace<->kernelspace API designers, not how it helps a
sysadmin type fewer commands.

> > > Anyway, for this to work we have to go through Joel.
> > >
> > > >	rmdir udcs/*		# unload all UDCs
> > >
> > > No, for this you still have to rmmod :)
> > >
> > >
> > > >>>I think the question is of information flow direction.  If user
> > > >>>gives some information to the kernel, she should be the one
> > > >>>creating any necessary directories.  But if the information comes
> > > >>>from kernel to the user, the kernel should create the structure.
> > 
> > 	This last paragraph actually describes the distinction between
> > configfs and sysfs.  More specifically, if userspace wants to create an
> > object in the kernel, it should use configfs.  If the kernel has created
> > an object on its own, it exposes it via sysfs.  It is a deliberate non-
> > goal for configfs to replicate sysfs behavior.
> 
> So if the lifetime of some object is controlled by the user, it belongs
> to configfs. If, on the other hand, the lifetime is controlled by the
> kernel, it belongs to sysfs. I think that the trouble with e.g. luns
> is that they might want to behave like both (at least in the "more
> automated"
> approach where they are programmatically created): they are created
> by the kernel (=> sysfs) but their lifetime is controlled by the user
> (=>configfs).

	I think your point is correct, but not quite aligned.  The
gadget endpoint is created by the user (configfs).  It, in turn, causes
the kernel to detect and create LUNs (sysfs).  I don't think this is a
conflict.
	Is your concern that I can then remove the gadget endpoint,
which should trigger the LUNs to disappear, but they are in use?  This
is why the depend_item() interface exists.  When the LUN is opened, the
LUN should mark depend_item() on the gadget endpoint.  Now rmdir() will
fail.

> > 	But this doesn't alleviate Andrzej's primary concern, nor does it
> > answer Michal's concerns about corner cases.  Let's see if we can solve
> > those.
> > 	First, what about preventing extraneous LUNs?  There are two
> > approaches: the prevent approach, and the ignore approach.  In the Target
> > module, they take the ignore approach.  If you create a LUN that is
> > outside the scope, they just ignore it.  So a user can make 10 LUNs, but
> > if only five are allowed, the other five are ignored.  In the prevent
> > approach, we try to fail the mkdir() when we go over the limit.
> 
> I think I like the prevent approach better, because I don't know what to
> do with the unused luns in the ignore approach. I also think that it would
> be nice if configfs described the actual state of the system instead of
> presenting all the possible user-created garbage (i.e. something which is
> ignored).

	Both good arguments.  I described both because they were
mentioned in the thread; I don't have a horse in that race from a
configfs perspective.

> > 	But you need to do the parsing anyway!  If the LUN is created via
> > mkdir(), somehow the gadget system needs to know what LUN number to
> > advertise.  I would actually decouple it from the name.  The name should
> > be free-form; if it happens to be comprehensible, that's good.  Imagine
> > this:
> > 	cd /cfg/.../mass_storage
> > 	echo 4 >max_lun_id
> > 	mkdir lun100
> > 	echo 1 >lun100/lun_id
> > 
> > The LUN actually has the ID of 1.  The fact that the directory name is
> > wrong is irrelevant to the gadget processing; non-hackers use the tooling,
> > and the tooling should  make sane names.  Then your
> > ->make_group() function can choose either approach to avoiding LUN IDs
> > that are too large.  It can prevent:
> > 
> > 	lun_id->store() {
> > 		if (lun_id_exists(parent, value))
> > 			return ERR_PTR(-EINVAL);
> > 		if (value > parent->max_lun_id)
> > 			return ERR_PTR(-ENOSPC) /* or EINVAL */
> > 		setup_lun();
> > 		make_live();
> > 	}
<snip>
> 
> Taking into account what you say about the expected userspace tooling
> using a free-form name plus the lun_id attribute could be attractive,
> because the lun_id attribute's value can be analyzed at ->store() and,
> if it doesn't fit, an appropriate error can be easily returned using
> just what we already have in configfs. Prevent seems better to me, too.

	Yeah, that's the behavior I was driving at.

> > 	But is max LUNs a requirement?  Why not just have the LUN count be
> > the number of LUNs created?  In that case, you can allow or disallow
> > discontiguous LUN IDs without worrying about a max ID.
> > 
> 
> I am not sure, but I think that Michal would say that he wouldn't like
> things to change once the gadget is up and running, so the number of
> luns should be fixed up front. On the other hand, after the gadget is
> set up, we can fail in ->make_group() to prevent the user from creating
> additional luns with which we don't know what to do. So what you suggest
> seems fine, but I would ask Michal to be on a safe side.

	Yeah, if you have a live/not-live toggle, failing new LUN
creation once live is fine.

> > [Really Hate Mkdir]
> > 
> > 	If you considered it appropriate for the LUN objects to be created
> > by the kernel, the standard way would to have "echo 5 >luns_allowed"
> > create LUN objects in sysfs.  Yes, sysfs.
> > There's no reason you can't have things in configfs *and* sysfs.  My
> > understanding of your code says to me that the mkdir approach is the way
> > to go, but if you hate it, this works.
> > 
> 
> I guess you are right, but see my point about luns' lifetime being
> controlled by the user and their creation being performed by the kernel.

	I hope I answered that.

> Once upon a time, Felipe expressed interest in having the information
> about endpoints and interfaces (mostly read-only stuff, but not all of it)
> accessible in the gadget subsystem in configfs. This kind of information
> is fully available only after the gadget has been bound, that is, after
> the user created all the groups/items and filled all the attributes
> and told the gadget to start (with e.g. filling some attribute or
> creating a symlink). So this looks like a programmatic creation of
> configfs directories. On the other hand, Felipe wanted it for debugging
> purposes, so the user could just as well create the required directories
> by hand when they need to, as Sebastian once suggested. Again, this requires
> ->make_group() to fail if all the information is not available yet.

	Some of this might belong in debugfs.  Some of it might belong
in a single attribute file, not in a hierarchy (so you can cat the
.../debug attribute; it is empty before start and full after start).
Just throwing out ideas.

> But isn't it kind of similar to the luns' case? It is a bunch of objects
> whose lifetime is controlled by the user (bind the gadget, unbind the
> gadget),
> but which are created by the kernel.
> Another problem with this is that the user must at some point _guess_
> what name to use in mkdir. Example:
> 
> For each interface there should be a folder. So the user must know how many
> interfaces' folders to create and how to name them, like:
> 
> $ mkdir ...../some_usb_function/interface00
> $ mkdir ...../some_usb_function/interface01
> ...
> ...
> ...
> 
> In each interface folder, there should be some endpoint folders and the user
> must know how many endpoints there are and how to name them:
> 
> $ mkdir ...../some_usb_function/interface00/endpoint02
> $ mkdir ...../some_usb_function/interface00/endpoint81
> 
> So probably there should be some attribute in some_usb_function,
> which contains a list of available interface names, and some attribute in
> interface#, which contains a list of available endpoint names.

	This is worth discussing.  I repeat that the goals of configfs
are transparency and discoverability.  If I just have to *know* to
"mkdirx interface01", it is not discoverable.  If mkdir fails for magic
reasons, it is not transparent.
	Your proposed attribute of available interface names is one of
the usual ways to solve this in configfs.  It makes for clean code, too.
If "mkdir interface_name" is not on the list described by "cat
available_interfaces", you can return an error.  In this fashion, the
interface (and endpoint) names are discoverable, and why your mkdir
succeeds or fails is transparent.
	The other approach is the one I described for LUNs: the path
name is free-form, and an attribute is set to make it appropriate.  eg,
rather than a directory named interface00, you have:

  $ mkdir .../some_usb_function/free-interface-name
  $ echo 00 > .../some_usb_function/free-interface-name/interface_id

> Workable, but not so convenient. And there could be a problem, too,

	Yeah, you are right, we are sacrificing convenience in the name
of transparency and discoverability.

> if the gadget is unbound: there is no way to remove the user-created
> directories other than by the user themselves, so there will be times
> (i.e. between gadget unbind and manual rmdir) when these interface/endpoint
> directories will not correspond to anything present in the system.

	Sure.  After unbind, they are a potential configuration, just
like they were before bind while you were setting them up.

Joel

 

-- 

Life's Little Instruction Book #157 

	"Take time to smell the roses."

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Joel Becker]

On Mon, Dec 10, 2012 at 03:17:34PM +0100, Andrzej Pietrasiewicz wrote:
> @Joel in particular: please see my comment in the bottom.

<snip>

> I forgot to mention, representing udcs (USB Device Controllers) in
> configfs is similar to interfaces/endpoints: the user needs to guess
> what name to use in mkdir, e.g. in:
> 
> $ mkdir ....../udcs/s3c-hsotg

	Again, discoverability.  Userspace should not need to guess.  So
some part of the system (configfs, sysfs, procfs, $dontcare) needs to
know what the available choices are.

> the poor user needs to know that the udc is actually called s3c-hsotg.
> And after the udc goes away from the system, the s3c-hsotg directory
> remains in the filesystem until the user deletes it (as nobody else
> but the user can do it). Again, an attribute like available_udcs in
> the udcs directory can help. But still, there will be times when
> its contents will not be consistent with the directories actually
> present.

	How does the system know there is an s3c-hsotg?  Is this from
scanning?

> @Joel: Any thoughts about it? Commitable items? E.g. the user creates
> whatever they want, so the "uncommitted" directory contains "garbage",
> that is directories which do not have to correspond to anything
> actually present in the system, but on commiting the gadget's item
> (group in fact) everything is verified and only "correct" gadgets
> are actually bound? Then, in the "uncommitted" directory the user
> can still do whatever they want, but this is not effective until
> uncommitting the old gadget and committing it again? There is just
> a tiny issue with committable items: Are they implemented? :O

	Committable items are not implemented, because everyone uses the
'enable' pattern:

  $ mkdir foo
  $ echo 1024 >foo/attr1
  $ echo 2048 >foo/attr2
  $ echo 1 >foo/enable

In o2hb, we don't have an explicit "enable" attribute.  Instead, we
enable when the block device's file descriptor is stored.  If the other
attributes aren't set yet, we fail the store.

Joel

-- 

Life's Little Instruction Book #451

	"Don't be afraid to say, 'I'm sorry.'"

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC][PATCH] fs: configfs: programmatically create config groups

[Sebastian Andrzej Siewior]

On 12/08/2012 12:18 AM, Joel Becker wrote:
> Hey Guys,

Hi Joel,

you took quite some time to write this down.

> 	Please remember that configfs is not a "user" interface, it's a
> userspace<->kernelspace interface.  Like sysfs, it's not required to be
> convenient for someone at a bash prompt.  My goal is that it is *usable*
> from a bash prompt.  So it must be that you can
> create/destroy/configure objects via mkdir/rmkdir/cat/echo, but you
> might have a lot of those mkdir/echo combos to configure something.
> When it comes to the "user" interface, a wrapper script or library
> should be converting a user intention into all that boilerplate.

It is good that you say such things so people that want this things know 
the reasons why it is not done.

>>>>> I think the question is of information flow direction.  If user gives
>>>>> some information to the kernel, she should be the one creating any
>>>>> necessary directories.  But if the information comes from kernel to the
>>>>> user, the kernel should create the structure.
>
> 	This last paragraph actually describes the distinction between
> configfs and sysfs.  More specifically, if userspace wants to create an
> object in the kernel, it should use configfs.  If the kernel has created
> an object on its own, it exposes it via sysfs.  It is a deliberate
> non-goal for configfs to replicate sysfs behavior.

So you are saying that I should expose my UDC hardware device via sysfs
after the hardware is available because the kernel created it.
How should I get then into configfs? mkdir a directory which is named
like the HW? This would be painful to do manually but as you said, we
should have a tool.

> [What About the Patch?]
>
> 	In general, attribute setting that turns into created configfs
> objects is against the theory of configfs.  In practice, it's a
> nightmare locking change.  Coming into this discussion, I though you
> were doing dymanic things at ->make_group() time, but that is already
> supported.
> 	But I want to hear your thoughts.  I've dumped a bunch of
> alternate approaches here.  Do any seem to fit?  What other challenges
> do you see?

I'm mostly fine with this. Part of the problem was/is that the user
could create lun1 without lun0. Now, after looking at target they don't
have this limitation so I don't think we should have it either. As nab
explained, lun0 is always handled implicit for certain requests and not
exposed as a lun if the user did not create it. So I think we can live
with this. After all we can still return with -EINVAL if the user
creates lun1 before lun0.

Now one little question remains: We plan to expose interface numbers
and endpoints. The current idea is to have one directory for each
endpoint something like endpointXX where XX is the endpoint number.
We create the gadget upfront and assign it later to the UDC via a
symlink. We learn the interface number after the gadget has been
assigned. I think you suggest that we create the directory via the tool
once we exposed its details via sysfs or so. This might be okay since we 
want it probably only for debugging. On the hand if we drop the
endpooint number we don't have this. Got to keep thinking about this.

Anyway, thank you for time and arguments pro / contra self-created
directories.

>
> Joel
>

Sebastian