Page MenuHomeFreeBSD

nvme: nvd call disk_resize() when namespace changed async event happened
AbandonedPublic

Authored by wanpengqian_gmail.com on Tue, Nov 16, 10:26 PM.

Details

Reviewers
chuck
imp
jhb
Summary

nvd register a callback function when namespace changed async event
happened. this will call disk_resize() properly.

Test Plan

when bhyve vm resize disk, verify diskinfo output inside guest.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 42831
Build 39719: arc lint + arc unit

Event Timeline

sys/dev/nvd/nvd.c
163

But we do the following:

} else if (aer->log_page_id == NVME_LOG_CHANGED_NAMESPACE &&
     !nvme_use_nvd) {
         nsl = (struct nvme_ns_list *)aer->log_page_buffer;
         for (i = 0; i < nitems(nsl->ns) && nsl->ns[i] != 0; i++) {
                 if (nsl->ns[i] > NVME_MAX_NAMESPACES)
                         break;
                 nvme_notify_ns(aer->ctrlr, nsl->ns[i]);
         }
 }

so nvd should use this and we should remove the !nvme_use_nvd which is there because we know that nvd doesn't do things right in new_disk. nvd needs some additional work to support name space arrival, though.

Plus the async call is for much much more than just namespace as well...

537

At the very least, we need to filter out all the non NMVE_LOG_CHANGED_NAMESPACE page notifications, but maybe it would be better to adapt the nvd_new_disk cope better, mostly because I want to remove the nvd hack that's in the nvme driver right now.

filter out all the non NMVE_LOG_CHANGED_NAMESPACE

sys/dev/nvd/nvd.c
163

You are right. when I test this, I remove the !nvme_use_nvd make sure async event will let nvd knows.
currently, when NAMESAPCE_CHANGE arrives, nvme_ctrlr didn't query the device, get the new namesapce size. your patch D32963 did the trick, but it create a kernel crash as I mention before.
When nvme_ns_construct() is called, calling nvme_completion_poll(&status) will cause kernel panic.

	nvme_ctrlr_cmd_identify_namespace(ctrlr, id, &ns->data,
	    nvme_completion_poll_cb, &status);
	nvme_completion_poll(&status);
	if (nvme_completion_is_error(&status.cpl)) {
		nvme_printf(ctrlr, "nvme_identify_namespace failed\n");
		return (ENXIO);
	}

I am not idea why it happens.

537

Yes, we should have a switch here.

sys/dev/nvd/nvd.c
163

if nvd & nda implement nvme_constroller_async() callback, do we need this here? it will call nvd_new_disk() for nvd, nvme_sim_ns_change() for nda.
maybe we should let nvd/nda match. first function is new_disk.

} else if (aer->log_page_id == NVME_LOG_CHANGED_NAMESPACE &&
    !nvme_use_nvd) {
        nsl = (struct nvme_ns_list *)aer->log_page_buffer;
        for (i = 0; i < nitems(nsl->ns) && nsl->ns[i] != 0; i++) {
                if (nsl->ns[i] > NVME_MAX_NAMESPACES)
                        break;
                nvme_notify_ns(aer->ctrlr, nsl->ns[i]);
        }
}