Re: [PATCH 11/15] target: add sysfs support

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, May 11, 2020 at 12:15:12PM -0500, Mike Christie wrote:
> On 5/11/20 1:30 AM, Greg Kroah-Hartman wrote:
> > On Sun, May 10, 2020 at 04:57:40PM -0500, Mike Christie wrote:
> >> These next two patches add a sysfs interface that reports the target
> >> layer's I_T nexuses/sessions. For the non-SCSI people cc'd, this just means
> >> we are reporting a server's connections to remote clients.
> >>
> >> This patch adds the upper level dirs which shows/organizes our local port
> >> (tpgts below) and the connection (session below). The next patch will then
> >> add the dirs/files for each connection/session which exports info like
> >> ACL/permissions and SCSI port values.
> >>
> >> Here is the general layout:
> >>
> >> [sys]# tree scsi_target/
> >> scsi_target/
> >> |-- fabric/target module
> >> |   `-- target name
> >> |       `-- tpgt_$target_port_group_number
> >> |           `-- sessions
> >> |               `-- initiator name - session ID number
> >> |                   |-- acl
> >> |                   `-- transport_id
> >> |                       |-- name
> >> |                       |-- proto
> >> |                       `-- session_id
> >>
> >> Here is an example with the scsi target layer's iSCSI driver:
> >>
> >> scsi_target/
> >> |-- iscsi
> >> |   `-- iqn.1999-09.com.tcmu:minna
> >> |       `-- tpgt_1
> >> |           `-- sessions
> >> |               `-- iqn.2005-03.com.ceph:ini1-1
> >> |                   |-- acl
> >> |                   `-- transport_id
> >> |                       |-- name
> >> |                       |-- proto
> >> |                       `-- session_id
> >> |-- fc
> >> |-- loopback
> >> |-- qla2xxx_tcm
> >>
> >>
> >> Note/Question for Greg:
> >>
> >> We are not exporting info in the upper level dirs like "fabric/target
> >> module", "target name", tpgt, etc and just need those dirs to be able to
> >> organize/view the endpoints of the session. So, in this patch I made a new
> >> top level dir scsi_target and made the other dirs with
> >> kobject_create_and_add. It looks like we could also add device structs in
> >> the target related structs, use classes, and build the tree/hierarchy that
> >> way too. It was not clear to me when to use one or the other.
> > 
> > Never use kobject calls in a driver subsystem like you have here, as
> > those objects will not be seen by userspace tools that get uevents.
> > Just use real 'struct devices' if you really really need a deep
> > directory tree.
> > 
> > But I would push back here, why do you feel you want such a deep tree?
> > What are you getting from this?  Why do you need that "sessions"
> > directory at all?
> 
> I do not need the sessions under the tpgt dir and will drop it. The
> target subsystem does not have a bus or use device structs at all right
> now.

Then fix that and everything shows up automatically "for free".

> And, I saw how other code that does not have a bus like btrfs/ext4
> use kobject_create_and_add() to group/organize objects and went wild
> with dirs :)

file systems are not devices with busses and drivers :)

> And, I can move the code to device structs, but had a question about the
> deep tree question.

Don't do it.

> A common operation will be that userspace knows the names of the objects
> above the session in the tree already. Apps then want to read in a
> specific session or scan the specific sessions in a target or a tpgt.
> The deep tree makes it easy to build the sysfs path and scan/read the
> specific object/objects.

Again, a "deep tree" with raw kobjects will be invisible to the normal
userspace tools that monitor changes in sysfs for devices because they
will not notice them at all.

> I wanted to avoid the issue where apps have with the flat layout where
> they have to scan every session when they just want something specific.

There should not be any difference in "speed" for something like that,
it's an in-ram filesystem with no i/o times.

> Will a deep tree be ok for this type of reason?

Nope :(

greg k-h