[LU-7115] fld_client_rpc() may run into deadloop

• Type: Bug
• Resolution: Fixed
• Priority: Major
• Fix Version/s: Lustre 2.10.0
• Affects Version/s: Lustre 2.7.0
• Severity: 3
• Assignee: Yang Sheng
• Reporter: Niu Yawei (Inactive)

Description

In fld_client_rpc():

        if (rc != 0) {
                if (imp->imp_state != LUSTRE_IMP_CLOSED && !imp->imp_deactive) {
                        /* Since LWP is not replayable, so it will keep
                         * trying unless umount happens, otherwise it would
                         * cause unecessary failure of the application. */
                        ptlrpc_req_finished(req);
                        rc = 0;
                        goto again;
                }
                GOTO(out_req, rc);
        }

If the connection is broken, this function will run into an dead loop. I think we'd reshape the function somehow to make it interruptable, otherwise, if connection never being established, caller will stuck in this function forever.

Seems fld_update_from_controller() has similar problem.

Issue Links

• is related to: LU-5312 sanity test_161a: cannot create regular file '...f.sanity.161a': Input/output error
• is related to: LU-6419 Fld client lookup should retry another target if the current target is deactive

Comments

[Niu Yawei (Inactive)]

As Di mentioned, the thread can be terminated by umount target, I think that's fine to me, we can just leave as it is.

And this function will be called by client as well, we may need to check if it's called by client (not from LWP but from mdc device), and break the loop for non-LWP device case.

[Andreas Dilger]

Is this really a bug or could this be closed?

[Di Wang (Inactive)]

Oh, it can break the loop, see

               if (imp->imp_state != LUSTRE_IMP_CLOSED && !imp->imp_deactive) {
                        /* Since LWP is not replayable, so it will keep
                         * trying unless umount happens, otherwise it would
                         * cause unecessary failure of the application. */
                        ptlrpc_req_finished(req);
                        rc = 0;
                        goto again;
                }

It will check the import state here. And also the point right now is that we do not break the connection between MDTs, until umount and admin step in, so this implementation actually fit in here.
Though we still need check the LWP here as I said in the previous comment.

[Niu Yawei (Inactive)]

I mean if the connection is broken, then it'll run into a deadloop, then there is no way to terminate the thread which calls this function, and we won't able to shutdown the MDT/OST at the end.

I think it's not a serious problem, and looks not easy to fix.

[Di Wang (Inactive)]

I thought the point here is to not fail for LWP unless it is being umounted or deactive, not sure how interruptible can help here, since it is the connection between MDTs. I may miss sth?

But we do need check if the import is for LWP, i.e. only do this "try again" for LWP connection, not for MDC or other import.

[Peter Jones]

Yang Sheng

Could you please look into this issue?

Thanks

Peter