Page MenuHomeFreeBSD

nvme: Be less verbose when cancelling I/O or admin commands
ClosedPublic

Authored by imp on Aug 3 2023, 10:41 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 1, 5:01 PM
Unknown Object (File)
Tue, Sep 24, 8:45 AM
Unknown Object (File)
Tue, Sep 24, 3:26 AM
Unknown Object (File)
Mon, Sep 23, 12:24 PM
Unknown Object (File)
Sat, Sep 21, 3:21 PM
Unknown Object (File)
Tue, Sep 10, 3:07 PM
Unknown Object (File)
Tue, Sep 10, 1:09 PM
Unknown Object (File)
Tue, Sep 10, 8:18 AM
Subscribers

Details

Summary

When we're resetting, and there's outstanding I/O that we're cancelling,
only report we're cancelling the I/O once rather than once per
I/O. Likewise when we reschedule the I/O. We don't need to say for each
one that we're cancelling/rescheduling something, and then report the
I/O that we're doing. Likewise with cancelling admin commands (we never
retry them here, so a similar change isn't needed).

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 52972
Build 49863: arc lint + arc unit

Event Timeline

imp requested review of this revision.Aug 3 2023, 10:41 PM

I'd just use !TAILQ_EMPTY() instead of TAILQ_FIRST() != NULL. Also while there I would remove extra STAILQ_EMPTY() before STAILQ_FIRST().

This revision is now accepted and ready to land.Aug 4 2023, 12:46 AM
sys/dev/nvme/nvme_qpair.c
1254

I would also maybe go ahead and spell out report.

sys/dev/nvme/nvme_qpair.c
1254

OK. I'd already made the first change... And I spelled it out here and elsewhere.

chuck added inline comments.
sys/dev/nvme/nvme_qpair.c
1293

Would it make sense to have the logging similar for Admin and I/O? I.e., either change this to done aborting outstanding i/o or change the Admin case above to done aborting?

sys/dev/nvme/nvme_qpair.c
1293

Yes. I think it would.
Thanks!