2009-04-17 10:51:19

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Daniel Debonzi, on 04/16/2009 10:23 PM wrote:
> Vladislav Bolkhovitin wrote:
>> Hi All,
>>
>> Below is proposal for the SCST sysfs layout, which will replace existing procfs-based infrastructure. Any comments, questions and suggestions are welcome!
>>
>> I. SCST sysfs layout.
>>
>> Root would be /sys/scsi_tgt.
>>
>> In the root there would be the following files and subdirectories:
>>
>> - targets - subdirectory listing names of all registered target drivers.
>>
>> - devices - subdirectory listing all registered backend devices.
>>
>> - sgv - subdirectory listing all existing SGV pools.
>>
>> - drivers - subdirectory listing all loaded target and backend drivers (dev handlers).
>>
>> - threads - RW file listing number of global SCST threads. Writing to that file would allow to change that value.
>>
>> - trace_level - RW file listing SCST core logging level. Writing to that file would allow to change that. Example content:
>> "out_of_mem | minor | pid | line | function | special | mgmt | mgmt_minor | mgmt_dbg | retry". See current procfs implementation of this file for more info.
>>
>> - version - RO file listing version of SCST core and enabled compile time features. Example content: "1.0.2, EXTRACHECKS, DEBUG"
>
> Based on all I read this last days, I believe we are not allowed to
> include the directory scsi_tgt on /sys root. I think it has to be in a
> existent directory reserved for this sort of application. I just didn't
> figured out which one it would be.

/sys/class? It already has scsi_device, scsi_disk, scsi_generic and
scsi_host.

>> III. Implementation considerations
>>
>> 1. Top level subdirectories and files should be created by init_scst().
>>
>> 2. targets/target_driver_name drivers/target_driver_name and should be created by scst_register_target_template() and removed by scst_unregister_target_template().
>>
>> 3. targets/target_driver_name/target with sessions, luns and ini_groups subdirectories should be created by scst_register() and removed by scst_unregister().
>>
>> 4. targets/target_driver_name/target/sessions/session and below should be created by scst_init_session() and removed by scst_free_session().
>>
>> 5. Pass-through devices should be added to devices/ by scst_register_device() and removed by scst_unregister_device(). Initially they should have "handler" link pointed to "none".
>>
>> 6. Virtual devices should be added to devices/ by scst_register_virtual_device() and removed by scst_unregister_virtual_device().
>>
>> 7. drivers/dev_handler_name should be added by scst_register_dev_driver() or scst_register_virtual_dev_driver() and removed by scst_unregister_dev_driver() or scst_unregister_virtual_dev_driver().
>>
>> 8. It isn't clear how to best combine standard and custom entries in targets/target_driver_name/target, devices/device, drivers/target_driver_name and drivers/dev_handler_name, I don't know sysfs interfaces sufficiently well. I can only suggest places, where it should be done:
>>
>> - For targets/target_driver_name/target a target driver after scst_register() register call should call new function scst_get_target_root() and add there new entries. Before scst_unregister() call the target driver should remove them at first. Similarly it should be done for drivers/target_driver_name and drivers/dev_handler_name.
>>
>> - For devices/device a dev handler should do it in attach() callback and remove in detach() callback. Similarly to scst_get_target_root(), the dev handler should get its sysfs root by calling scst_get_dev_root().
>>
>> Both should be made in some generic way to minimize duplicated code between target drivers and between dev handlers.
>
> Also based on what I read, the way to have data structures reflected on
> sysfs is using kobjecs. I feel that the expected approach to have it is
> to include a kobject (or kset depending on the case) on the existent
> structures which will be reflected on the sysfs. I notice that on the
> actual implementation all the proc interface is implemented on
> scst_proc.c and I don't know if it will be possible to keep having the
> access interfaces on a separated source file. We could possible have the
> callback functions on a separated file but I can't visualize it without
> start to touch it more deeply. Probably you guys have a better view of
> this implementation possibilities.

All the above sysfs layout reflects internal SCST objects:

1. targets/target_driver_name and drivers/target_driver_name -> struct
scst_tgt_template

2. targets/target_driver_name/target -> struct scst_tgt

3. targets/target_driver_name/target/sessions/session -> struct scst_session

4. targets/target_driver_name/target/ini_groups/group -> struct
scst_acg. (For targets/target_driver_name/target/luns each struct
scst_tgt would have internal struct scst_acg.)

5.
targets/target_driver_name/target/ini_groups/group/initiators/initiator
-> struct scst_acn

6. targets/target_driver_name/target/ini_groups/group/luns/lun and
targets/target_driver_name/target/luns/lun -> struct scst_acg_dev

7. devices/device -> struct scst_device

8. drivers/dev_handler_name -> struct scst_dev_type

9. sgv/pool -> struct sgv_pool

So, we should simply add kobjects in them. To manipulate with then
already there is an API, used by procfs, and should be also used by
sysfs. This API is completely out of scst_proc.c


LAYOUT UPDATE.

Looks like it would be better to move entries from
drivers/target_driver_name to targets/target_driver_name to keep all the
related entries in one place. Then to reflect new state rename drivers/
to back_drivers/.

Thanks,
Vlad


2009-04-17 13:25:31

by Daniel Debonzi

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Vladislav Bolkhovitin wrote:
> Daniel Debonzi, on 04/16/2009 10:23 PM wrote:
>> Vladislav Bolkhovitin wrote:
>>> Hi All,
>>>
>>> Below is proposal for the SCST sysfs layout, which will replace
>>> existing procfs-based infrastructure. Any comments, questions and
>>> suggestions are welcome!
>>>
>>> I. SCST sysfs layout.
>>>
>>> Root would be /sys/scsi_tgt.
>>>
>>> In the root there would be the following files and subdirectories:
>>>
>>> - targets - subdirectory listing names of all registered target
>>> drivers.
>>>
>>> - devices - subdirectory listing all registered backend devices.
>>>
>>> - sgv - subdirectory listing all existing SGV pools.
>>>
>>> - drivers - subdirectory listing all loaded target and backend
>>> drivers (dev handlers).
>>>
>>> - threads - RW file listing number of global SCST threads. Writing
>>> to that file would allow to change that value.
>>>
>>> - trace_level - RW file listing SCST core logging level. Writing to
>>> that file would allow to change that. Example content: "out_of_mem |
>>> minor | pid | line | function | special | mgmt | mgmt_minor |
>>> mgmt_dbg | retry". See current procfs implementation of this file for
>>> more info.
>>>
>>> - version - RO file listing version of SCST core and enabled compile
>>> time features. Example content: "1.0.2, EXTRACHECKS,
>>> DEBUG"
>>
>>
>> Based on all I read this last days, I believe we are not allowed to
>> include the directory scsi_tgt on /sys root. I think it has to be in a
>> existent directory reserved for this sort of application. I just
>> didn't figured out which one it would be.
>
> /sys/class? It already has scsi_device, scsi_disk, scsi_generic and
> scsi_host.

I don't think so because all the directories on /sys/class have symlinks
to the files somewhere else. However I noticed that many of them on my
system are on /sys/device/virtual

>>> III. Implementation considerations
>>>
>>> 1. Top level subdirectories and files should be created by init_scst().
>>>
>>> 2. targets/target_driver_name drivers/target_driver_name and should
>>> be created by scst_register_target_template() and removed by
>>> scst_unregister_target_template().
>>>
>>> 3. targets/target_driver_name/target with sessions, luns and
>>> ini_groups subdirectories should be created by scst_register() and
>>> removed by scst_unregister().
>>>
>>> 4. targets/target_driver_name/target/sessions/session and below
>>> should be created by scst_init_session() and removed by
>>> scst_free_session().
>>>
>>> 5. Pass-through devices should be added to devices/ by
>>> scst_register_device() and removed by scst_unregister_device().
>>> Initially they should have "handler" link pointed to "none".
>>>
>>> 6. Virtual devices should be added to devices/ by
>>> scst_register_virtual_device() and removed by
>>> scst_unregister_virtual_device().
>>>
>>> 7. drivers/dev_handler_name should be added by
>>> scst_register_dev_driver() or scst_register_virtual_dev_driver() and
>>> removed by scst_unregister_dev_driver() or
>>> scst_unregister_virtual_dev_driver().
>>>
>>> 8. It isn't clear how to best combine standard and custom entries in
>>> targets/target_driver_name/target, devices/device,
>>> drivers/target_driver_name and drivers/dev_handler_name, I don't know
>>> sysfs interfaces sufficiently well. I can only suggest places, where
>>> it should be done:
>>>
>>> - For targets/target_driver_name/target a target driver after
>>> scst_register() register call should call new function
>>> scst_get_target_root() and add there new entries. Before
>>> scst_unregister() call the target driver should remove them at first.
>>> Similarly it should be done for drivers/target_driver_name and
>>> drivers/dev_handler_name.
>>>
>>> - For devices/device a dev handler should do it in attach() callback
>>> and remove in detach() callback. Similarly to scst_get_target_root(),
>>> the dev handler should get its sysfs root by calling
>>> scst_get_dev_root().
>>>
>>> Both should be made in some generic way to minimize duplicated code
>>> between target drivers and between dev handlers.
>>
>> Also based on what I read, the way to have data structures reflected
>> on sysfs is using kobjecs. I feel that the expected approach to have
>> it is to include a kobject (or kset depending on the case) on the
>> existent structures which will be reflected on the sysfs. I notice
>> that on the actual implementation all the proc interface is
>> implemented on scst_proc.c and I don't know if it will be possible to
>> keep having the access interfaces on a separated source file. We could
>> possible have the callback functions on a separated file but I can't
>> visualize it without start to touch it more deeply. Probably you guys
>> have a better view of this implementation possibilities.
>
> All the above sysfs layout reflects internal SCST objects:
>
> 1. targets/target_driver_name and drivers/target_driver_name -> struct
> scst_tgt_template
>
> 2. targets/target_driver_name/target -> struct scst_tgt
>
> 3. targets/target_driver_name/target/sessions/session -> struct
> scst_session
>
> 4. targets/target_driver_name/target/ini_groups/group -> struct
> scst_acg. (For targets/target_driver_name/target/luns each struct
> scst_tgt would have internal struct scst_acg.)
>
> 5.
> targets/target_driver_name/target/ini_groups/group/initiators/initiator
> -> struct scst_acn
>
> 6. targets/target_driver_name/target/ini_groups/group/luns/lun and
> targets/target_driver_name/target/luns/lun -> struct scst_acg_dev
>
> 7. devices/device -> struct scst_device
>
> 8. drivers/dev_handler_name -> struct scst_dev_type
>
> 9. sgv/pool -> struct sgv_pool
>
> So, we should simply add kobjects in them. To manipulate with then
> already there is an API, used by procfs, and should be also used by
> sysfs. This API is completely out of scst_proc.c
>

great, this will save some time.

>
> LAYOUT UPDATE.
>
> Looks like it would be better to move entries from
> drivers/target_driver_name to targets/target_driver_name to keep all the
> related entries in one place. Then to reflect new state rename drivers/
> to back_drivers/.

Ack.

Thanks,
Daniel Debonzi

2009-04-17 14:12:28

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Daniel Debonzi, on 04/17/2009 05:25 PM wrote:
>>> Vladislav Bolkhovitin wrote:
>>>> Hi All,
>>>>
>>>> Below is proposal for the SCST sysfs layout, which will replace
>>>> existing procfs-based infrastructure. Any comments, questions and
>>>> suggestions are welcome!
>>>>
>>>> I. SCST sysfs layout.
>>>>
>>>> Root would be /sys/scsi_tgt.
>>>>
>>>> In the root there would be the following files and subdirectories:
>>>>
>>>> - targets - subdirectory listing names of all registered target
>>>> drivers.
>>>>
>>>> - devices - subdirectory listing all registered backend devices.
>>>>
>>>> - sgv - subdirectory listing all existing SGV pools.
>>>>
>>>> - drivers - subdirectory listing all loaded target and backend
>>>> drivers (dev handlers).
>>>>
>>>> - threads - RW file listing number of global SCST threads. Writing
>>>> to that file would allow to change that value.
>>>>
>>>> - trace_level - RW file listing SCST core logging level. Writing to
>>>> that file would allow to change that. Example content: "out_of_mem |
>>>> minor | pid | line | function | special | mgmt | mgmt_minor |
>>>> mgmt_dbg | retry". See current procfs implementation of this file for
>>>> more info.
>>>>
>>>> - version - RO file listing version of SCST core and enabled compile
>>>> time features. Example content: "1.0.2, EXTRACHECKS,
>>>> DEBUG"
>>>
>>> Based on all I read this last days, I believe we are not allowed to
>>> include the directory scsi_tgt on /sys root. I think it has to be in a
>>> existent directory reserved for this sort of application. I just
>>> didn't figured out which one it would be.
>> /sys/class? It already has scsi_device, scsi_disk, scsi_generic and
>> scsi_host.
>
> I don't think so because all the directories on /sys/class have symlinks
> to the files somewhere else. However I noticed that many of them on my
> system are on /sys/device/virtual

Let's go with root in /sys/class/scsi_tgt. In future, if somebody
objects, we can easily change it.

Vlad

2009-04-17 14:25:25

by Kay Sievers

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

On Fri, Apr 17, 2009 at 15:25, Daniel Debonzi
<[email protected]> wrote:
> Vladislav Bolkhovitin wrote:

>>> Based on all I read this last days, I believe we are not allowed to
>>> include the directory scsi_tgt on /sys root. I think it has to be in a
>>> existent directory reserved for this sort of application. I just didn't
>>> figured out which one it would be.

Right, there will be no new out-of-/sys/devices/ top-level device dir,
unless you convince everybody to have a scsifs, which would be in
/sys/kernel/scsi/, and which would not use the driver core device
stuff at all. :)

>> /sys/class? It already has scsi_device, scsi_disk, scsi_generic and
>> scsi_host.

If it's a bus or a class, it does not matter. What you need to include
is a "struct device" (and not a kobject) if you want them to show up
in the common directories.

> I don't think so because all the directories on /sys/class have symlinks to
> the files somewhere else. However I noticed that many of them on my system
> are on /sys/device/virtual

All "struct device" devices appear in /sys/devices/, that's the single
place the hierarchy is expressed. The classification, meaning the
"collection of devices of the same subsystem" happens in /sys/bus/
/sys/class/, therefore they are only flat lists of links. Virtual are
devices which have no parent assigned. The driver core prepends
virtual to them, when they are registered.

Kay

2009-04-17 14:27:43

by James Smart

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Vladislav Bolkhovitin wrote:
> Let's go with root in /sys/class/scsi_tgt. In future, if somebody
> objects, we can easily change it.
>

This maps too closely to the scsi midlayer names, especially the real
target devices initiators see - which are not yet in sysfs but there has
been discussion of in the past. I would prefer that you used
/sys/class/scst_tgt. And yes, as part of the class level support in the
midlayer/kernel, they are generally symlinks to the actual kobject in
the /sys/devices that the class entity corresponds to.

IMHO - "scsi_tgt" also seems a bit presumptuous to propose if SCST is
not, in it's entirety, going to be the adapter target interface pulled
in upstream. Thus, another reason I prefer the "scst_tgt" name, as it
makes it very specific to SCST itself, and lessens any co-existence
conflicts if there are any in the future.

-- james s

2009-04-17 15:51:00

by Daniel Debonzi

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Hi Kay,

Thanks for the inputs.

Do you mean that uses struct device is the right way to do it instead of
kobjects or it is just a option to get things on right places into sysfs?

I don't know this struct closely but my first impression looking to the
source code is that it is tied with hardware and has some complexity we
probably don't need. What do you think?

Regards,
Daniel Debonzi


Kay Sievers wrote:
> On Fri, Apr 17, 2009 at 15:25, Daniel Debonzi
> <[email protected]> wrote:
>> Vladislav Bolkhovitin wrote:
>
>>>> Based on all I read this last days, I believe we are not allowed to
>>>> include the directory scsi_tgt on /sys root. I think it has to be in a
>>>> existent directory reserved for this sort of application. I just didn't
>>>> figured out which one it would be.
>
> Right, there will be no new out-of-/sys/devices/ top-level device dir,
> unless you convince everybody to have a scsifs, which would be in
> /sys/kernel/scsi/, and which would not use the driver core device
> stuff at all. :)
>
>>> /sys/class? It already has scsi_device, scsi_disk, scsi_generic and
>>> scsi_host.
>
> If it's a bus or a class, it does not matter. What you need to include
> is a "struct device" (and not a kobject) if you want them to show up
> in the common directories.
>
>> I don't think so because all the directories on /sys/class have symlinks to
>> the files somewhere else. However I noticed that many of them on my system
>> are on /sys/device/virtual
>
> All "struct device" devices appear in /sys/devices/, that's the single
> place the hierarchy is expressed. The classification, meaning the
> "collection of devices of the same subsystem" happens in /sys/bus/
> /sys/class/, therefore they are only flat lists of links. Virtual are
> devices which have no parent assigned. The driver core prepends
> virtual to them, when they are registered.
>
> Kay
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2009-04-17 16:12:04

by Kay Sievers

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

On Fri, Apr 17, 2009 at 17:50, Daniel Debonzi
<[email protected]> wrote:
> Do you mean that uses struct device is the right way to do it instead of
> kobjects or it is just a option to get things on right places into sysfs?

You can not put kobjects in /sys/devices/ and there will be no
disconnected kobject tree just for scsi, unless the scsi maintainers
really want want that, and then they should create their own
filesystem instead, and not use kobjects at all.

> I don't know this struct closely but my first impression looking to the
> source code is that it is tied with hardware and has some complexity we
> probably don't need. What do you think?

If you want to integrate with the current scsi devices/objects, which
is the only sensible option I think, you have to use struct device
devices, kobjects will not get a classification in sysfs, and will be
invisible for userspace tools unless you do random readdir() in
/sys/devices/ to find them.

Kay

2009-04-17 17:42:35

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Kay Sievers, on 04/17/2009 08:03 PM wrote:
> On Fri, Apr 17, 2009 at 17:50, Daniel Debonzi
> <[email protected]> wrote:
>> Do you mean that uses struct device is the right way to do it instead of
>> kobjects or it is just a option to get things on right places into sysfs?
>
> You can not put kobjects in /sys/devices/ and there will be no
> disconnected kobject tree just for scsi, unless the scsi maintainers
> really want want that, and then they should create their own
> filesystem instead, and not use kobjects at all.

Perhaps I'm missing something, but why do you think sysfs hierarchy for
a SCSI target subsystem can't be put anywhere, but only in /sys/devices?

Consider /sys/class/scsi_host/ as an example. It is very close to what
SCST needs.

And don't forget a SCSI target subsystem, basically, has from
implementation POV in common with the regular SCSI (initiator) subsystem
only some constants for commands and errors and "SCSI" word in the name.
All the internal processing is completely different, similarly to
difference between Firefox and Apache, mutt and sendmail, NFS client and
server.

>> I don't know this struct closely but my first impression looking to the
>> source code is that it is tied with hardware and has some complexity we
>> probably don't need. What do you think?
>
> If you want to integrate with the current scsi devices/objects, which
> is the only sensible option I think, you have to use struct device
> devices, kobjects will not get a classification in sysfs, and will be
> invisible for userspace tools unless you do random readdir() in
> /sys/devices/ to find them.
>
> Kay
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-04-17 17:43:36

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

James Smart, on 04/17/2009 06:27 PM wrote:
> Vladislav Bolkhovitin wrote:
>> Let's go with root in /sys/class/scsi_tgt. In future, if somebody
>> objects, we can easily change it.
>
> This maps too closely to the scsi midlayer names, especially the real
> target devices initiators see - which are not yet in sysfs but there has
> been discussion of in the past. I would prefer that you used
> /sys/class/scst_tgt. And yes, as part of the class level support in the
> midlayer/kernel, they are generally symlinks to the actual kobject in
> the /sys/devices that the class entity corresponds to.
>
> IMHO - "scsi_tgt" also seems a bit presumptuous to propose if SCST is
> not, in it's entirety, going to be the adapter target interface pulled
> in upstream. Thus, another reason I prefer the "scst_tgt" name, as it
> makes it very specific to SCST itself, and lessens any co-existence
> conflicts if there are any in the future.

James,

Thank you for the suggestion. If nobody objects, we will go with
/sys/class/scst_tgt.

Vlad

2009-04-17 17:43:50

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Daniel Debonzi, on 04/17/2009 07:50 PM wrote:
> Hi Kay,
>
> Thanks for the inputs.
>
> Do you mean that uses struct device is the right way to do it instead of
> kobjects or it is just a option to get things on right places into sysfs?
>
> I don't know this struct closely but my first impression looking to the
> source code is that it is tied with hardware and has some complexity we
> probably don't need. What do you think?

I agree, looks like using struct device instead of struct kobject should
additionally complicate the code a lot for not clear gain.

> Regards,
> Daniel Debonzi
>
>
> Kay Sievers wrote:
>> On Fri, Apr 17, 2009 at 15:25, Daniel Debonzi
>> <[email protected]> wrote:
>>> Vladislav Bolkhovitin wrote:
>>>>> Based on all I read this last days, I believe we are not allowed to
>>>>> include the directory scsi_tgt on /sys root. I think it has to be in a
>>>>> existent directory reserved for this sort of application. I just didn't
>>>>> figured out which one it would be.
>> Right, there will be no new out-of-/sys/devices/ top-level device dir,
>> unless you convince everybody to have a scsifs, which would be in
>> /sys/kernel/scsi/, and which would not use the driver core device
>> stuff at all. :)
>>
>>>> /sys/class? It already has scsi_device, scsi_disk, scsi_generic and
>>>> scsi_host.
>> If it's a bus or a class, it does not matter. What you need to include
>> is a "struct device" (and not a kobject) if you want them to show up
>> in the common directories.
>>
>>> I don't think so because all the directories on /sys/class have symlinks to
>>> the files somewhere else. However I noticed that many of them on my system
>>> are on /sys/device/virtual
>> All "struct device" devices appear in /sys/devices/, that's the single
>> place the hierarchy is expressed. The classification, meaning the
>> "collection of devices of the same subsystem" happens in /sys/bus/
>> /sys/class/, therefore they are only flat lists of links. Virtual are
>> devices which have no parent assigned. The driver core prepends
>> virtual to them, when they are registered.
>>
>> Kay
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-04-17 17:56:30

by Kay Sievers

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
> Thank you for the suggestion. If nobody objects, we will go with
> /sys/class/scst_tgt.

On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
> I agree, looks like using struct device instead of struct kobject should
> additionally complicate the code a lot for not clear gain.

Thes both replies together suggest that you miss how sysfs and the
driver core works. Please go and read the current code, and look at
sysfs, before trying anything like this.

There is no way to add any stuff to /sys/class/scst_tgt/ without using
proper "struct device" objects.

For the same reason we will not have a disconnected device tree, we
will not havet any raw kobject in the /sys/devices, /sys/class,
/sys/bus directories.

Thanks,
Kay

2009-04-17 18:25:00

by Kay Sievers

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

On Fri, Apr 17, 2009 at 19:56, Kay Sievers <[email protected]> wrote:
> On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
>> Thank you for the suggestion. If nobody objects, we will go with
>> /sys/class/scst_tgt.
>
> On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
>> I agree, looks like using struct device instead of struct kobject should
>> additionally complicate the code a lot for not clear gain.
>
> Thes both replies together suggest that you miss how sysfs and the
> driver core works. Please go and read the current code, and look at
> sysfs, before trying anything like this.
>
> There is no way to add any stuff to /sys/class/scst_tgt/ without using
> proper "struct device" objects.
>
> For the same reason we will not have a disconnected device tree, we
> will not havet any raw kobject in the /sys/devices, /sys/class,
> /sys/bus directories.

As a starting point, consider creating a "scst" bus_type. Then make
sure all devices you need are uniquely named, so they can be in a flat
list in /sys/bus/scst/devices/.

Then add all the devices as struct_device to the tree, maybe use an
existing parent struct_device (like a pci device) if you have one, or
create a custom one in /sys/devices/ if there is nothing to use.

All the devices you add will show up in a hierarchy in /sys/devices/
depending on the parent information you supply, and every single
device of your subsystem will be listed in a flat list in
/sys/bus/scst/devices/*. You will also get proper events for userspace
that way.

The question is where the actual block devices hang off, and if they
can use one of the created scst devices, or if they will be virtual
ones?

Thanks,
Kay

2009-04-23 16:11:57

by Daniel Debonzi

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

Hi all,

I was off those days due to a long holiday here in Brazil. Now I am
getting back to it.

Kay Sievers wrote:
> On Fri, Apr 17, 2009 at 19:56, Kay Sievers <[email protected]> wrote:
>> On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
>>> Thank you for the suggestion. If nobody objects, we will go with
>>> /sys/class/scst_tgt.
>> On Fri, Apr 17, 2009 at 19:43, Vladislav Bolkhovitin <[email protected]> wrote:
>>> I agree, looks like using struct device instead of struct kobject should
>>> additionally complicate the code a lot for not clear gain.
>> Thes both replies together suggest that you miss how sysfs and the
>> driver core works. Please go and read the current code, and look at
>> sysfs, before trying anything like this.
>>
>> There is no way to add any stuff to /sys/class/scst_tgt/ without using
>> proper "struct device" objects.
>>
>> For the same reason we will not have a disconnected device tree, we
>> will not havet any raw kobject in the /sys/devices, /sys/class,
>> /sys/bus directories.
>
> As a starting point, consider creating a "scst" bus_type. Then make
> sure all devices you need are uniquely named, so they can be in a flat
> list in /sys/bus/scst/devices/.
>
> Then add all the devices as struct_device to the tree, maybe use an
> existing parent struct_device (like a pci device) if you have one, or
> create a custom one in /sys/devices/ if there is nothing to use.
>
> All the devices you add will show up in a hierarchy in /sys/devices/
> depending on the parent information you supply, and every single
> device of your subsystem will be listed in a flat list in
> /sys/bus/scst/devices/*. You will also get proper events for userspace
> that way.
>
> The question is where the actual block devices hang off, and if they
> can use one of the created scst devices, or if they will be virtual
> ones?

Vlad, how do you think we should move on it?
Do you want me to try to go deeper on it and make a new plan/propose or
would you like to reformulate the RFC you did?

As you all know my knowledge still limited on this subject so I don't
feel comfortable to make this sort of decisions at this moments.

Regards,
Daniel Debonzi

2009-04-28 17:05:50

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Discussion about SCST sysfs layout and implementation.

#include <linux/kobject.h>
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/ctype.h>

#include "scst.h"
#include "scst_priv.h"
#include "scst_mem.h"

#define SCST_SYSFS_BLOCK_SIZE (PAGE_SIZE - 512)

static DEFINE_MUTEX(scst_sysfs_mutex);

/* struct kobject *targets; */
/* struct kobject *devices; */
/* struct kobject *sgv; */
/* struct kobject *drivers; */

struct scst_sysfs_root {
struct kobject kobj;
};

struct scst_sysfs_root *scst_sysfs_root;

static ssize_t scst_threads_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
int count;

TRACE_ENTRY();

count = sprintf(buf, "%d\n", scst_global_threads_count());

TRACE_EXIT();
return count;
}


static ssize_t scst_threads_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
int res = count;
int oldtn, newtn, delta;

TRACE_ENTRY();

if (count > SCST_SYSFS_BLOCK_SIZE) {
res = -EOVERFLOW;
goto out;
}


if (mutex_lock_interruptible(&scst_sysfs_mutex) != 0) {
res = -EINTR;
goto out;
}

mutex_lock(&scst_global_threads_mutex);

oldtn = scst_nr_global_threads;
sscanf(buf, "%du", &newtn);

if (newtn <= 0) {
PRINT_ERROR("Illegal threads num value %d", newtn);
res = -EINVAL;
goto out_up_thr_free;
}
delta = newtn - oldtn;
if (delta < 0)
__scst_del_global_threads(-delta);
else
__scst_add_global_threads(delta);

PRINT_INFO("Changed cmd threads num: old %d, new %d", oldtn, newtn);

out_up_thr_free:
mutex_unlock(&scst_global_threads_mutex);

mutex_unlock(&scst_sysfs_mutex);

out:
TRACE_EXIT_RES(res);
return res;
}


static ssize_t scst_trace_level_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
return sprintf(buf, "stgt show!!\n");
}


static ssize_t scst_trace_level_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
return count;
}


static ssize_t scst_version_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
TRACE_ENTRY();

sprintf(buf, "%s\n", SCST_VERSION_STRING);

#ifdef CONFIG_SCST_STRICT_SERIALIZING
strcat(buf, "Strict serializing enabled\n");
#endif

#ifdef CONFIG_SCST_EXTRACHECKS
strcat(buf, "EXTRACHECKS\n");
#endif

#ifdef CONFIG_SCST_TRACING
strcat(buf, "TRACING\n");
#endif

#ifdef CONFIG_SCST_DEBUG
strcat(buf, "DEBUG\n");
#endif

#ifdef CONFIG_SCST_DEBUG_TM
strcat(buf, "DEBUG_TM\n");
#endif

#ifdef CONFIG_SCST_DEBUG_RETRY
strcat(buf, "DEBUG_RETRY\n");
#endif

#ifdef CONFIG_SCST_DEBUG_OOM
strcat(buf, "DEBUG_OOM\n");
#endif

#ifdef CONFIG_SCST_DEBUG_SN
strcat(buf, "DEBUG_SN\n");
#endif

#ifdef CONFIG_SCST_USE_EXPECTED_VALUES
strcat(buf, "USE_EXPECTED_VALUES\n");
#endif

#ifdef CONFIG_SCST_ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ
strcat(buf, "ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ\n");
#endif

#ifdef CONFIG_SCST_STRICT_SECURITY
strcat(buf, "SCST_STRICT_SECURITY\n");
#endif

TRACE_EXIT();
return strlen(buf);

}


struct kobj_attribute scst_threads_attr =
__ATTR(threads, S_IRUGO | S_IWUSR, scst_threads_show, scst_threads_store);

struct kobj_attribute scst_trace_level_attr =
__ATTR(trace_level, S_IRUGO | S_IWUSR, scst_trace_level_show, scst_trace_level_store);


struct kobj_attribute scst_version_attr =
__ATTR(version, S_IRUGO, scst_version_show, NULL);


static struct attribute *scst_sysfs_root_default_attrs[] = {
&scst_threads_attr.attr,
&scst_trace_level_attr.attr,
&scst_version_attr.attr,
NULL,
};

static void scst_sysfs_root_release(struct kobject *kobj)
{
kfree(scst_sysfs_root);
}

static ssize_t scst_show(struct kobject *kobj, struct attribute *attr,
char *buf)
{
struct kobj_attribute *kobj_attr;
kobj_attr = container_of(attr, struct kobj_attribute, attr);

return kobj_attr->show(kobj, kobj_attr, buf);
}

static ssize_t scst_store(struct kobject *kobj, struct attribute *attr,
const char *buf, size_t count)
{
struct kobj_attribute *kobj_attr;
kobj_attr = container_of(attr, struct kobj_attribute, attr);

return kobj_attr->store(kobj, kobj_attr, buf, count);
}

struct sysfs_ops scst_sysfs_root_kobj_ops = {
.show = scst_show,
.store = scst_store,
};


static struct kobj_type scst_sysfs_root_ktype = {
.sysfs_ops = &scst_sysfs_root_kobj_ops,
.release = scst_sysfs_root_release,
.default_attrs = scst_sysfs_root_default_attrs,
};

int __init scst_sysfs_init(void)
{
int retval = 0;

TRACE_ENTRY();

scst_sysfs_root = kzalloc(sizeof(*scst_sysfs_root), GFP_KERNEL);
if(!scst_sysfs_root)
goto sysfs_root_error;

retval = kobject_init_and_add(&scst_sysfs_root->kobj,
&scst_sysfs_root_ktype, NULL, "%s", "scsi_tgt");
if (retval)
goto sysfs_root_kobj_error;

out:
TRACE_EXIT_RES(retval);
return retval;

sysfs_root_kobj_error:
kfree(scst_sysfs_root);

sysfs_root_error:
retval = -EINVAL;
goto out;
}

void __exit scst_sysfs_cleanup(void)
{
TRACE_ENTRY();

kobject_put(&scst_sysfs_root->kobj);

TRACE_EXIT();
return;
}


Attachments:
scst_sysfs.c (4.87 kB)