[LU-7030] import_sec_validate_get()) import ffff8819c6497800 (FULL) with no sec (again)

• Type: Bug
• Resolution: Incomplete
• Priority: Minor
• Labels: llnl
• Severity: 3
• Assignee: John Hammond
• Reporter: Christopher Morrone (Inactive)

Description

The patch from LU-3353 does not appear to have eliminated the stated error message. We are still seeing it with Lustre 2.5.4 + the patch from LU-3353.

Please try again.

Issue Links

• is related to: LU-3353 import_sec_validate_get() import ffff88061d4a7000 (FULL) with no sec

Comments

[Andreas Dilger]

Jeremy, I'd be happy to drop the reversion patch if you can submit a patch to remove those extra refcounts for the security flavours.

[Jeremy Filizetti]

From what I can tell its reasonable to remove class_import_get from gss_sec_create_common and plain_create_sec as well as class_import_put from the destroy functions. I'm running it through sanity now just to confirm but it does avoid the earlier mentioned problem.

[Gerrit Updater]

Andreas Dilger ([e-mail]) uploaded a new patch: http://review.whamcloud.com/17709
Subject: Revert "LU-7030 security: put imp_sec after all requests drained off"
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1335d385d7e22ae184cb7f3b78fdecc0a5b1e972

[Niu Yawei (Inactive)]

Indeed. The purpose of sec flavor holding import refcount looks not clear to me, if import needs sec flavor until all RPCs drained off and the flavor will be destroyed before freeing import, why should sec flavor holds import refcount?

I suggest to remove the import get/put in the create_sec/destroy_sec, what do you think?

[Jeremy Filizetti]

A proper fix is needed that won't break security flavors other than the default

[Jeremy Filizetti]

Patch 16071 breaks Lustre security for everything but the default flavor. Every security flavor except the default NULL (no security) grabs a reference on the import in create_sec. There is an equivalent destroy_sec but that is called from the sptlrpc_sec_put. With this patch that is no longer called until the import is released but this creates a circular dependency for everything but the default security flavor. We should revert the patch and create one that doesn't affect other security flavors.

[Peter Jones]

Landed for 2.8

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/16071/
Subject: LU-7030 security: put imp_sec after all requests drained off
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 504ca288d99779812495a91345421ad4ad8f7d95

[Gerrit Updater]

Niu Yawei ([e-mail]) uploaded a new patch: http://review.whamcloud.com/16071
Subject: LU-7030 security: put imp_sec after all requests drained off
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: e545cf24ca487432d7093e7bf0e474f8f5b84efe

[Niu Yawei (Inactive)]

Looks the fix of LU-3353 isn't quite right, see client_destroy_import(): It calls sptlrpc_import_sec_put() to clear imp_sec first, then calls class_import_put() to add the import onto zombie list.

Then look at the import_sec_validate_get():

        /* Only output an error when the import is still active */
        if (*sec == NULL) {
                if (list_empty(&imp->imp_zombie_chain))
                        CERROR("import %p (%s) with no sec\n",
                                imp, ptlrpc_import_state_name(imp->imp_state));
                return -EACCES;
        }

When checking imp->imp_zombie_chain, the import may not be put on zombie list yet.

Amir, could you take a look?

[Peter Jones]

Niu

You worked on LU-3353 previously. What do you suggest here?

Peter