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(-)
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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
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
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
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
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
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
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
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
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
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
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
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
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