Re: [PATCH v7 1/2] nvme: add per-queue sysfs directories

[Saravanan D <saravanand@crusoe.ai>]

On Wed, Nov 04, 2026 at 02:17:33PM +0000, sashiko-bot@kernel.org wrote:
[...]
> [Severity: Medium]
> If sysfs_create_groups() fails in nvme_add_queue_info() after
> kobject_init_and_add() has already published qinfo->kobj in sysfs,
> should the error path call kobject_del(&qinfo->kobj) before
> kobject_put(&qinfo->kobj), just like nvme_remove_queue_info() does?

I'm surprised this sequence went unnoticed in the previous versions of
this patch, since the error path has not changed since v5. My fault
injection runs used DEBUG_KOBJECT_RELEASE, which would have exposed the
directory left behind, but they set fail-nth on the task writing to
/dev/nvme-fabrics, and fail-nth only fails allocations made by that
task. nvme_tcp_setup_io_queues() starts each I/O queue from an async
worker, so nvme_add_queue_info() runs in a different task and none of
its allocations failed.

I will send a v8 that calls kobject_del() before kobject_put() once
kobject_init_and_add() has succeeded, so a failed sysfs_create_groups()
unlinks the <qid> directory before nvme_add_queue_info() returns, the
same way nvme_remove_queue_info() does.

I have also extended my own testing with a run that uses the failslab
stack filter to fail each allocation under nvme_add_queue_info() in
turn, including those made from the async workers. It checks that a
failed add leaves no queues/<qid> directory behind and that a controller
reset can add the same qid again. It fails on v7 and passes with the
fix.

Thanks,
Saravanan D.