configfs: Add struct configfs_item_operations->check_link() in configfs_unlink()

From: Nicholas Bellinger <[email protected]>

This patch adds a optional struct configfs_item_operations->check_link() check called in fs/configfs/symlink.c:configfs_unlink() that can be used by configfs consumers to check for an explict struct config_group dependence with active symlink and fail with -EPERM before the unlink(2) syscall is allowed to occur.

Currently without this patch, there is not a method that a consumer can tell configfs_unlink() that it needs to fail for this particular case. Allowing ->check_link() to propigate up the errno to VFS is also another option for the call, but currently for TCM using the existing -EPERM in configfs_unlink() is fine here.

Note this patch is used by TCM v4 generic configfs fabric module infrastructure to allow explict Initiator Port MappedLUNs symlinks to create a dependency for the fabric TPG Port LUNs living in a configfs group that is not a direct struct config_group parent.

Signed-off-by Nicholas A. Bellinger <[email protected]>

---
 fs/configfs/symlink.c    |   13 +++++++++++++
 include/linux/configfs.h |    1 +
 2 files changed, 14 insertions(+), 0 deletions(-)

fs/configfs/symlink.c +13 -0

index 0f3eb41..b8010de 100644
@@ -205,6 +205,19 @@ int configfs_unlink(struct inode *dir, struct dentry
 *dentry) parent_item = configfs_get_config_item(dentry->d_parent);
 	type = parent_item->ci_type;
 
+	/*
+	 * See if the underlying struct config_item has dependent
+	 * symlinks, and should return -EPERM here.
+	 */
+	if (type && type->ct_item_ops &&
+	    type->ct_item_ops->check_link) {
+		if (type->ct_item_ops->check_link(parent_item,
+					sl->sl_target) != 0) {
+			config_item_put(parent_item);
+			goto out;
+		}
+	}
+
 	spin_lock(&configfs_dirent_lock);
 	list_del_init(&sd->s_sibling);
 	spin_unlock(&configfs_dirent_lock);

include/linux/configfs.h +0 -0

index ddb7a97..7c01d86 100644
@@ -226,6 +226,7 @@ struct configfs_item_operations {
 	ssize_t	(*show_attribute)(struct config_item *, struct configfs_attribute
 *,char *); ssize_t	(*store_attribute)(struct config_item *,struct

configfs_attribute *,const char *, size_t); int (*allow_link)(struct
config_item *src, struct config_item *target); + int (*check_link)(struct
config_item *src, struct config_item *target); int (*drop_link)(struct config_item *src, struct config_item *target); };

Quoted 14 lines · On Monday 30 August 2010 05:20:25 Nicholas A. Bellinger wrote:

From: Nicholas Bellinger <[email protected]>

This patch adds a optional struct configfs_item_operations->check_link() check called in fs/configfs/symlink.c:configfs_unlink() that can be used by configfs consumers to check for an explict struct config_group dependence with active symlink and fail with -EPERM before the unlink(2) syscall is allowed to occur.

Currently without this patch, there is not a method that a consumer can tell configfs_unlink() that it needs to fail for this particular case. Allowing ->check_link() to propigate up the errno to VFS is also another option for the call, but currently for TCM using the existing -EPERM in configfs_unlink() is fine here.

Quoted 6 lines

Note this patch is used by TCM v4 generic configfs fabric module infrastructure to allow explict Initiator Port MappedLUNs symlinks to create a dependency for the fabric TPG Port LUNs living in a configfs group that is not a direct struct config_group parent.

Signed-off-by Nicholas A. Bellinger <[email protected]>

Quoted 20 lines · Konrad Rzeszutek Wilk

Quoted 19 lines · On Monday 30 August 2010 05:20:25 Nicholas A. Bellinger wrote:

From: Nicholas Bellinger <[email protected]>

This patch adds a optional struct configfs_item_operations->check_link() check called in fs/configfs/symlink.c:configfs_unlink() that can be used by configfs consumers to check for an explict struct config_group dependence with active symlink and fail with -EPERM before the unlink(2) syscall is allowed to occur.

Currently without this patch, there is not a method that a consumer can tell configfs_unlink() that it needs to fail for this particular case. Allowing ->check_link() to propigate up the errno to VFS is also another option for the call, but currently for TCM using the existing -EPERM in configfs_unlink() is fine here.

Note this patch is used by TCM v4 generic configfs fabric module infrastructure to allow explict Initiator Port MappedLUNs symlinks to create a dependency for the fabric TPG Port LUNs living in a configfs group that is not a direct struct config_group parent.

Quoted 1 line

Quoted 1 line

Signed-off-by Nicholas A. Bellinger <[email protected]>

I NAK'd this a while back. I'm willing to be convinced, but so
far it remains that way.

Quoted 2 lines

I NAK'd this a while back. I'm willing to be convinced, but so
far it remains that way.

Hi Joel,

Thanks for bringing this point up again. So a brief refresh on why this is currently required in the fabric independent configfs handlers in drivers/target/target_core_fabric_configfs.c (patch #19).

Quoted 8 lines

Quoted 2 lines

I NAK'd this a while back. I'm willing to be convinced, but so
far it remains that way.

Hi Joel,

Thanks for bringing this point up again. So a brief refresh on why this is currently required in the fabric independent configfs handlers in drivers/target/target_core_fabric_configfs.c (patch #19).

Well, that is great that you mentioned your requirements. But I don't see a quote of Joel's concerns? Is there an LKML link for it perhaps?

Quoted 11 lines · Konrad Rzeszutek Wilk

Quoted 8 lines

Quoted 2 lines

I NAK'd this a while back. I'm willing to be convinced, but so
far it remains that way.

Hi Joel,

Thanks for bringing this point up again. So a brief refresh on why this is currently required in the fabric independent configfs handlers in drivers/target/target_core_fabric_configfs.c (patch #19).

Well, that is great that you mentioned your requirements. But I don't see a quote of Joel's concerns? Is there an LKML link for it perhaps?

It was a while back. Essentially, the concern is that configfs
is defined to be userspace-driven. If the user asks you to remove something, the module should be handling safe teardown rather than rejecting the user's request.
Now, the world isn't always clean-cut. Code outside of the
filesystem representation can lay a claim on a configfs item to say "I'm busy with this." An example is ocfs2 pinning the configfs item
describing its cluster heartbeat device. But this is ocfs2 - a
separate thing - claiming it. There is a defined API to take and release these claims.
This API doesn't cover symlinks, as symlinks are simply linkage

between config items. It may be, as Nick suggested at the bottom of his reply, that we want the API extended to cover that case. But he hasn't yet convinced me that safe teardown is impossible or disasterous. That's what I'd like to see. It's not obvious on the face of all the file trees in the email.
Nick, can you provide some form of description, not long
pathnames, that explains a) what breaks when the symlink is removed b) why that can't be allowed if the user is dumb enough to request it?

Quoted 4 lines · Joel Becker

Nick, can you provide some form of description, not long
pathnames, that explains a) what breaks when the symlink is removed b) why that can't be allowed if the user is dumb enough to request it?

So, the case where configfs will actually OOPs without the ->check_link() patch (or without some other internal solution) is on the unlink(2) path is when the symlink is created to a destination outside of the source struct config_group. This may have not been exactly apparent in my LIO-Target example, but here is another shot at an example without the other complexities of target mode invovled.

Say we have two different struct config_subsystem in two different LKM sub_parent and sub_child. I will spare the actual mkdir(2) and ln(2) calls here, but (I hope) these are obvious:

First, we start out with the parent source struct config_group from sub_parent module:

/sys/kernel/config/sub_parent/group1/parent/

Next, we have a symlink from sub_parent/group1/parent to a different LKM in sub_child:

	/sys/kernel/config/sub_child/group1/src_0/src_link -> ../../../../sub_parent/group1/parent

And then a second symlink from sub_child/group1/src_0/src_link to a sstuct config_group outside of group1, but still within sub_child:

	/sys/kernel/config/sub_child/group2/dst_0/dst_link -> ../../../group1/src_0/

So once the sub_child/group2/dest_0/dst_link has been created to back to sub_child/group1/src_0/src_link, the oops will appear any time that 'unlink sub_child/group1/src_0/src_link' is called while the second group2/dst_0/dst_link is still present. I don't recall the actual backtrace of the OOPs that occurs when the unlink(2) is called, but it is easily reproducable .

I am really starting to think that fixing this properly below the struct config_item_operations API is going to make the most sense, but I have not realized this in a patch for fs/configfs/ just yet.. I am happy to do this in the next days if you think this would be the cleanest resolution for the above case.

Quoted 6 lines · Nicholas A. Bellinger

Quoted 4 lines · Joel Becker

Nick, can you provide some form of description, not long
pathnames, that explains a) what breaks when the symlink is removed b) why that can't be allowed if the user is dumb enough to request it?

Quoted 6 lines

So, the case where configfs will actually OOPs without the ->check_link() patch (or without some other internal solution) is on the unlink(2) path is when the symlink is created to a destination outside of the source struct config_group. This may have not been exactly apparent in my LIO-Target example, but here is another shot at an example without the other complexities of target mode invovled.

I wish you'd mentioned the oops earlier. We need to figure out
who is oopsing.

Quoted 13 lines

Say we have two different struct config_subsystem in two different LKM sub_parent and sub_child. I will spare the actual mkdir(2) and ln(2) calls here, but (I hope) these are obvious:

First, we start out with the parent source struct config_group from sub_parent module:

/sys/kernel/config/sub_parent/group1/parent/

Next, we have a symlink from sub_parent/group1/parent to a different LKM in sub_child:

Quoted 6 lines

	/sys/kernel/config/sub_child/group1/src_0/src_link -> ../../../../sub_parent/group1/parent

And then a second symlink from sub_child/group1/src_0/src_link to a sstuct config_group outside of group1, but still within sub_child:

	/sys/kernel/config/sub_child/group2/dst_0/dst_link -> ../../../group1/src_0/

I think you have too many ../ on your examples. dst_link
pointing to ../../../../group1/src_0 resolves to /sys/kernel/config/group1/src_0, which I don't think you want. I think you have:

    sub_parent/group1/parent
    sub_child/group1/src_0/src_link -> ../../../sub_parent/group1/parent
    sub_child/group2/dst_0/dst_link -> ../../group1/src_0

	If I'm reading this right, 'ls sub_child/group1/src_0/src_link'

gives you the contents of sub_parent/group1/parent, 'ls sub_child/group2/dst_0/dst_link' gives you 'src_link', and 'ls sub_child/group2/dst_0/dst_link/src_link' brings you right back to the contents of sub_parent/group1/parent. Am I right?

Quoted 7 lines

So once the sub_child/group2/dest_0/dst_link has been created to back to sub_child/group1/src_0/src_link, the oops will appear any time that 'unlink sub_child/group1/src_0/src_link' is called while the second group2/dst_0/dst_link is still present. I don't recall the actual backtrace of the OOPs that occurs when the unlink(2) is called, but it is easily reproducable .

I'm confused. Above it appears that
sub_child/group2/dst_0/dst_link -> ../../group1/src_0, but this paragraph suggests that sub_child/group2/dst_0/dst_link -> ../../group1/src_0/src_link. Which is it?

Quoted 6 lines

I am really starting to think that fixing this properly below the struct config_item_operations API is going to make the most sense, but I have not realized this in a patch for fs/configfs/ just yet.. I am happy to do this in the next days if you think this would be the cleanest resolution for the above case.

My big question is whether the oops comes from something
configfs is doing wrong when symlinking symlinks, or if your module is just surprised when things change. If the former, it needs to be fixed in configfs. It shouldn't require a hook for your module to avoid configfs breakage. If it is the latter, and your module needs to know not to break itself, then we discuss how to help it.

On Wed, 2010-09-08 at 12:26 -0700, Joel Becker wrote:
So after re-running this again, I was a bit off about where the OOOPs is actually occuring. So, the OOPs does not occur during in the simple example here with the first unlink(2):

unlink sub_child/group1/src_0/src_link

but rather after the second unlink(2) is called after the first for src_link occurs:

unlink sub_child/group2/dst_0/dst_link

So back to the OOPs with the current TCM code example, on v2.6.36-rc3 this actually triggers a SLUB warning "Object already free" from inside of TCM code. This is attributed to the releasing a specific LUN ACLs from the second unlink(2)'s struct config_item_operations->drop_link(), that the first unlink had already released. This is because the first unlink(2) will currently assume that the remaining LUN ACLs are safe to release because, it still assumes the disabled check_link call.

Quoted 12 lines · Nicholas A. Bellinger

On Wed, 2010-09-08 at 12:26 -0700, Joel Becker wrote:
So after re-running this again, I was a bit off about where the OOOPs is actually occuring. So, the OOPs does not occur during in the simple example here with the first unlink(2):

unlink sub_child/group1/src_0/src_link

but rather after the second unlink(2) is called after the first for src_link occurs:

unlink sub_child/group2/dst_0/dst_link

Quoted 7 lines

So back to the OOPs with the current TCM code example, on v2.6.36-rc3 this actually triggers a SLUB warning "Object already free" from inside of TCM code. This is attributed to the releasing a specific LUN ACLs from the second unlink(2)'s struct config_item_operations->drop_link(), that the first unlink had already released. This is because the first unlink(2) will currently assume that the remaining LUN ACLs are safe to release because, it still assumes the disabled check_link call.

The trivial solution is to refcount your ACLs. You get both
allow_link() calls, so you should be able to increment a counter there, and then drop them when the last drop_link() call is made. That will keep your consumer structures around until all links are exhausted.

Quoted 5 lines · Joel Becker

The trivial solution is to refcount your ACLs. You get both
allow_link() calls, so you should be able to increment a counter there, and then drop them when the last drop_link() call is made. That will keep your consumer structures around until all links are exhausted.

So I am a bit confused wrt to this last response.. The ->check_link() patch and it's use in the fabric independent code within target_core_fabric_configfs.c does exactly this for the 'MappedLUN' symlink case, eg: requires the consumer to do the allow_link() + drop_link() refcounting, and add the
API check into fs/configfs/symlink.c:configfs_unlink()

Is there another form of configfs consumer refcounting that you had in mind beyond using an atomic_t for this with ->check_link() here..?

So beyond a configfs consumer solution, what do you think about checking for the sub_child/group2/dst_0/dst_link style of symlink
in fs/configfs/symlink.c:configfs_symlink() in order to add some form of internal refcount when the symlink source is within the same consumer LKM, but outside of the parent struct config_group..?

This would involve the conversion of fs/configfs/symlink.c:
configfs_unlink() path to check for the existence of this internal refcount and returning -EPERM when any sub_child/group2/dst_0/dst_link exist when 'unlink sub_child/group1/src_0/src_link' is attempted.

Quoted 7 lines · Nicholas A. Bellinger

Quoted 5 lines · Joel Becker

The trivial solution is to refcount your ACLs. You get both
allow_link() calls, so you should be able to increment a counter there, and then drop them when the last drop_link() call is made. That will keep your consumer structures around until all links are exhausted.

Quoted 6 lines

So I am a bit confused wrt to this last response.. The ->check_link() patch and it's use in the fabric independent code within target_core_fabric_configfs.c does exactly this for the 'MappedLUN' symlink case, eg: requires the consumer to do the allow_link() + drop_link() refcounting, and add the
API check into fs/configfs/symlink.c:configfs_unlink()

You can refcount without check_link().

Quoted 3 lines

Is there another form of configfs consumer refcounting that you had in mind beyond using an atomic_t for this with ->check_link() here..?

I'm saying that you won't crash if you don't free the ACLs on
the first drop_link(). That is, the drop_link() goes through as configfs wants it to, but you don't crash.

Quoted 11 lines

So beyond a configfs consumer solution, what do you think about checking for the sub_child/group2/dst_0/dst_link style of symlink
in fs/configfs/symlink.c:configfs_symlink() in order to add some form of internal refcount when the symlink source is within the same consumer LKM, but outside of the parent struct config_group..?

This would involve the conversion of fs/configfs/symlink.c:
configfs_unlink() path to check for the existence of this internal refcount and returning -EPERM when any sub_child/group2/dst_0/dst_link exist when 'unlink sub_child/group1/src_0/src_link' is attempted.

You're still fighting allowing the links to go away. You
haven't explained why that is necessary. You had a problem with a crash because you expected one reference to your ACLs and actually have two, but you can fix that without modifying configfs.

Quoted 1 line · Joel Becker

You can refcount without check_link().

So what do you recommend here..?

The problem is that the 'unlink sub_child/group1/src_0/src_link' can't signal to the other struct config_group to also call an internal 'unlink sub_child/group2/dst_0/dst_link' to drop the child link outside of it's struct config_group.

Quoted 4 lines

You're still fighting allowing the links to go away. You
haven't explained why that is necessary. You had a problem with a crash because you expected one reference to your ACLs and actually have two, but you can fix that without modifying configfs.

If this is the case then I must be mis-understanding what you mean by configfs consumer refcounting from allow_link() and drop_link(). Can you give me a bit more detail where I should be looking..?

[Sorry on the delay, I was out of town]

Quoted 4 lines · Joel Becker

Quoted 1 line

You can refcount without check_link().

So what do you recommend here..?

That your ACL object, or whatever it is that considers itself to
be refcounted by the number of links, keep track of that and only free itself when all are gone rather than freeing itself when the first goes away.

Quoted 4 lines

The problem is that the 'unlink sub_child/group1/src_0/src_link' can't signal to the other struct config_group to also call an internal 'unlink sub_child/group2/dst_0/dst_link' to drop the child link outside of it's struct config_group.

Nor should it. I'm asking what is so wrong about a world where
sub_child/group1/src_0/src_link is gone but sub_child/group2/dst_0/dst_link remains? Maybe that target object can't work anymore, but the user broke it by breaking the link.

Quoted 8 lines

Quoted 4 lines

You're still fighting allowing the links to go away. You
haven't explained why that is necessary. You had a problem with a crash because you expected one reference to your ACLs and actually have two, but you can fix that without modifying configfs.

If this is the case then I must be mis-understanding what you mean by configfs consumer refcounting from allow_link() and drop_link(). Can you give me a bit more detail where I should be looking..?

Here's how I sort of understood things. First, you create the
src_link pointing to $object. This somehow allocates some sort of ACL structure hanging off of $object. Then you create dst_link pointing to src_link, which really ends up pointing to the $object. So now you have src_link and dst_link pointing to $object.
Finally, someone unlinks src_link. This triggers $object to
free the ACL structure. When the caller later removes dst_link, it crashes because it was expecting ACL to still be there. Do I have it right?

I'm saying that $object should count how many people are
pointing to it, so that when you remove src_link, ACL is *not* freed. It will only be freed when both src_link and dst_link are removed. This way you do not crash. Perhaps I'm not understanding the ACL object. Perhaps I'm missing the mechanism entirely. But I don't see why the ACL object must necessarily be freed when one symlink is removed but not the other.

Quoted 2 lines · Joel Becker

[Sorry on the delay, I was out of town]

Hi Joel,

Many, thanks for your followup on this item, my comments are below.

Quoted 11 lines · Nicholas A. Bellinger

Quoted 4 lines · Joel Becker

Quoted 1 line

You can refcount without check_link().

So what do you recommend here..?

That your ACL object, or whatever it is that considers itself to
be refcounted by the number of links, keep track of that and only free itself when all are gone rather than freeing itself when the first goes away.

Ok, I see what you mean by internal refcounting within the configfs consumer to handle this case..

Quoted 10 lines

Quoted 4 lines

The problem is that the 'unlink sub_child/group1/src_0/src_link' can't signal to the other struct config_group to also call an internal 'unlink sub_child/group2/dst_0/dst_link' to drop the child link outside of it's struct config_group.

Nor should it. I'm asking what is so wrong about a world where
sub_child/group1/src_0/src_link is gone but sub_child/group2/dst_0/dst_link remains? Maybe that target object can't work anymore, but the user broke it by breaking the link.

Yes, so trying to avoid the unlink alltogether here was my main intention thus far.

Actually leaving the sub_child/group2/dst_0/dst_link in the example here would be acceptable for the TCM MappedLUN case, because really we never expect this case to this unless someone is poking at configfs directly, and our userspace code will never do this intentionally.

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

You're still fighting allowing the links to go away. You
haven't explained why that is necessary. You had a problem with a crash because you expected one reference to your ACLs and actually have two, but you can fix that without modifying configfs.

If this is the case then I must be mis-understanding what you mean by configfs consumer refcounting from allow_link() and drop_link(). Can you give me a bit more detail where I should be looking..?

Quoted 9 lines

Here's how I sort of understood things. First, you create the
src_link pointing to $object. This somehow allocates some sort of ACL structure hanging off of $object. Then you create dst_link pointing to src_link, which really ends up pointing to the $object. So now you have src_link and dst_link pointing to $object.
Finally, someone unlinks src_link. This triggers $object to
free the ACL structure. When the caller later removes dst_link, it crashes because it was expecting ACL to still be there. Do I have it right?

Correct.

Quoted 8 lines

I'm saying that $object should count how many people are
pointing to it, so that when you remove src_link, ACL is *not* freed. It will only be freed when both src_link and dst_link are removed. This way you do not crash. Perhaps I'm not understanding the ACL object. Perhaps I'm missing the mechanism entirely. But I don't see why the ACL object must necessarily be freed when one symlink is removed but not the other.

No, I think your points here make perfect sense. I will look into a patch that leaves the TCM fabric MappedLUNs symlinks in place when the underlying TPG fabric LUN symlink is removed without breaking anything, but still does the necessary accounting to ensure that shutdown with active I/Os still works as expected.

I will plan to drop the
->check_link() patch from the forthcoming RFC v2 series.

In the end I think his is the best approach for .37, eg: no configfs change required. I am still open to the discussion for resolving this within fs/configfs proper, but at this point I don't have a strong preference and will follow your direction here.

Many thanks for your invaluable input Joel!

--nab

Quoted 72 lines · Nicholas A. Bellinger

Quoted 2 lines · Joel Becker

[Sorry on the delay, I was out of town]

Hi Joel,

Many, thanks for your followup on this item, my comments are below.

Quoted 11 lines · Nicholas A. Bellinger

Quoted 4 lines · Joel Becker

Quoted 1 line

You can refcount without check_link().

So what do you recommend here..?

That your ACL object, or whatever it is that considers itself to
be refcounted by the number of links, keep track of that and only free itself when all are gone rather than freeing itself when the first goes away.

Ok, I see what you mean by internal refcounting within the configfs consumer to handle this case..

Quoted 10 lines

Quoted 4 lines

The problem is that the 'unlink sub_child/group1/src_0/src_link' can't signal to the other struct config_group to also call an internal 'unlink sub_child/group2/dst_0/dst_link' to drop the child link outside of it's struct config_group.

Nor should it. I'm asking what is so wrong about a world where
sub_child/group1/src_0/src_link is gone but sub_child/group2/dst_0/dst_link remains? Maybe that target object can't work anymore, but the user broke it by breaking the link.

Yes, so trying to avoid the unlink alltogether here was my main intention thus far.

Actually leaving the sub_child/group2/dst_0/dst_link in the example here would be acceptable for the TCM MappedLUN case, because really we never expect this case to this unless someone is poking at configfs directly, and our userspace code will never do this intentionally.

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

You're still fighting allowing the links to go away. You
haven't explained why that is necessary. You had a problem with a crash because you expected one reference to your ACLs and actually have two, but you can fix that without modifying configfs.

If this is the case then I must be mis-understanding what you mean by configfs consumer refcounting from allow_link() and drop_link(). Can you give me a bit more detail where I should be looking..?

Quoted 9 lines

Here's how I sort of understood things. First, you create the
src_link pointing to $object. This somehow allocates some sort of ACL structure hanging off of $object. Then you create dst_link pointing to src_link, which really ends up pointing to the $object. So now you have src_link and dst_link pointing to $object.
Finally, someone unlinks src_link. This triggers $object to
free the ACL structure. When the caller later removes dst_link, it crashes because it was expecting ACL to still be there. Do I have it right?

Correct.

Quoted 8 lines

I'm saying that $object should count how many people are
pointing to it, so that when you remove src_link, ACL is *not* freed. It will only be freed when both src_link and dst_link are removed. This way you do not crash. Perhaps I'm not understanding the ACL object. Perhaps I'm missing the mechanism entirely. But I don't see why the ACL object must necessarily be freed when one symlink is removed but not the other.

Quoted 5 lines

No, I think your points here make perfect sense. I will look into a patch that leaves the TCM fabric MappedLUNs symlinks in place when the underlying TPG fabric LUN symlink is removed without breaking anything, but still does the necessary accounting to ensure that shutdown with active I/Os still works as expected.

Perhaps a strengthen chmod here. And if then, done by root, a big fat "shoot self in the foot" message in dmsg for the poking where you don't need to. type.

(BTW: could you re establish the link after it's deleted the way you
do at setup?)

Quoted 2 lines · Joel Becker

[Sorry on the delay, I was out of town]

Many, thanks for your followup on this item, my comments are below.

Quoted 2 lines · Nicholas A. Bellinger

Quoted 1 line · Joel Becker

Actually leaving the sub_child/group2/dst_0/dst_link in the example here would be acceptable for the TCM MappedLUN case, because really we never expect this case to this unless someone is poking at configfs directly, and our userspace code will never do this intentionally.

Quoted 8 lines

I'm saying that $object should count how many people are
pointing to it, so that when you remove src_link, ACL is *not* freed. It will only be freed when both src_link and dst_link are removed. This way you do not crash. Perhaps I'm not understanding the ACL object. Perhaps I'm missing the mechanism entirely. But I don't see why the ACL object must necessarily be freed when one symlink is removed but not the other.

No, I think your points here make perfect sense. I will look into a patch that leaves the TCM fabric MappedLUNs symlinks in place when the underlying TPG fabric LUN symlink is removed without breaking anything, but still does the necessary accounting to ensure that shutdown with active I/Os still works as expected. I will plan to drop the ->check_link() patch from the forthcoming RFC v2 series.

In the end I think his is the best approach for .37, eg: no configfs change required. I am still open to the discussion for resolving this within fs/configfs proper, but at this point I don't have a strong preference and will follow your direction here.