[RFC,00/10] target: add configfs interface

Message

Mike Christie June 27, 2020, 4:34 a.m. UTC

The following patches made over linus's current tree (also apply
over Martin's 5.9 scsi queue branch) add a configfs interface to
export LIO's sessions.

With Hannes not liking the refcounting/tricks in the sysfs approach
I took another stab at configfs. This approach works similar to the
loop/vhost/usb/xen nexus interface where there is a special file
that allows userspace to add/remove sessions.

Because the kernel is making sessions for most drivers, we do not
have mkdir/rmdir support for the session dir, but using the special
file approach we can still remove children session in the correct
order and avoid the issues I hit before. The interface is a little
odd configfs wise because we use the special file, but it has the
benefit everything is in the one interface so it's easy to update
the tools.

If this approach is preferred over the sysfs one then I can repost
with some other fixes, the transport id patches and the iscsi
conversion one.

Comments

Bart Van Assche July 3, 2020, 5:37 p.m. UTC | #1

On 2020-06-26 21:34, Mike Christie wrote:
> With Hannes not liking the refcounting/tricks in the sysfs approach
> I took another stab at configfs. This approach works similar to the
> loop/vhost/usb/xen nexus interface where there is a special file
> that allows userspace to add/remove sessions.

Hi Mike,

Are you perhaps referring to the comment in the following message?
https://lore.kernel.org/linux-scsi/[e-mail]/
The duplication of strings in that patch also looks weird to me.
I think kobject_del() waits for ongoing sysfs show and store callbacks to finish.
Since patch 12/15 adds a kobject_del() call in target_sysfs_remove_session(), is
that call perhaps sufficient to guarantee that the
se_sess->se_node_acl->initiatorname pointer and similar pointers are all valid
at least as long the session sysfs object exists?

Thanks,

Bart.

Mike Christie July 3, 2020, 10:12 p.m. UTC | #2

> On Jul 3, 2020, at 12:37 PM, Bart Van Assche <[e-mail]> wrote:
> 
> On 2020-06-26 21:34, Mike Christie wrote:
>> With Hannes not liking the refcounting/tricks in the sysfs approach
>> I took another stab at configfs. This approach works similar to the
>> loop/vhost/usb/xen nexus interface where there is a special file
>> that allows userspace to add/remove sessions.
> 
> Hi Mike,
> 
> Are you perhaps referring to the comment in the following message?
> https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/[e-mail]/__;!!GqivPVa7Brio!L7jRHXS-8N1S4DIyD7kLwn1WPta5_ANJgQpJrI-fdmRlub6ViFCvtW0VCqwhN37bZMtc$ 

Yes.

> The duplication of strings in that patch also looks weird to me.
> I think kobject_del() waits for ongoing sysfs show and store callbacks to finish.

Nice!

I didn’t know that. I added that free_session callback for that reason. When testing I was just verifying that the callback was called when all refs to the file had dropped and didn’t check that it was actually device_del.


> Since patch 12/15 adds a kobject_del() call in target_sysfs_remove_session(), is
> that call perhaps sufficient to guarantee that the

Yes. It’s doable. There might be an issue with one driver, but I can fix that up.


> se_sess->se_node_acl->initiatorname pointer and similar pointers are all valid
> at least as long the session sysfs object exists?
> 
> Thanks,
> 
> Bart.