Option to never do BLKRRPART ioctl in nvme format?

Option to never do BLKRRPART ioctl in nvme format?

[Nick Neumann]

The nvme format command, as its documentation states, issues BLKRRPART
ioctl after successful format.

It feels odd that after a successful format, the format command can
still return an error code because BLKRRPART ioctl fails.

When initially added, the BLKRRPART ioctl was issued but its return
value was ignored. Later[1], the return value started being checked
and returned. And over time [2,3], cases were added where BLKRRPART is
not issued.

I'm wondering if all of this, combined with apparent race conditions
I've hit where BLKRRPART can fail after successful format, are a sign
that there should be a way to do an nvme format without BLKRRPART
ioctl.

Maybe format should have an option to not do BLKRRPART and instead
just always do NVME_IOCTL_RESCAN ? Then the caller can issue BLKRRPART
ioctl themselves if they need it to happen?

[1]: https://github.com/linux-nvme/nvme-cli/commit/adf01d2d6a03c5e48380004cb4971a5b9ef89b4a
the return value started being checked and returned.

[2]: https://github.com/linux-nvme/nvme-cli/commit/46e0baea19b7ed8f6f9c639f8da82d2bb944c561
BLKRRPART ioctl is skipped when lbaf is the same or all namespaces are formatted

[3]: https://github.com/linux-nvme/nvme-cli/commit/6420c58b510ddba290500d9b75fef9e9c980e0c1
BLKRRPART ioctl is not done when a character device is used.

Re: Option to never do BLKRRPART ioctl in nvme format?

[Daniel Wagner]

Hi Nick,

On Fri, Jan 06, 2023 at 01:28:52PM -0600, Nick Neumann wrote:
> The nvme format command, as its documentation states, issues BLKRRPART
> ioctl after successful format.
> 
> It feels odd that after a successful format, the format command can
> still return an error code because BLKRRPART ioctl fails.
> 
> When initially added, the BLKRRPART ioctl was issued but its return
> value was ignored. Later[1], the return value started being checked
> and returned. And over time [2,3], cases were added where BLKRRPART is
> not issued.
> 
> I'm wondering if all of this, combined with apparent race conditions
> I've hit where BLKRRPART can fail after successful format, are a sign
> that there should be a way to do an nvme format without BLKRRPART
> ioctl.

I can't really say why BLKRRPART was added initially. The comment which was
added later on

/*
 * If block size has been changed by the format
 * command up there, we should notify it to
 * kernel blkdev to update its own block size
 * to the given one because blkdev will not
 * update by itself without re-opening fd.
 */

indicates we don't need BLKRRPART necessary. I would like to understand first
what's going on before dropping BLKRRPART.

@Keith, do you happen to remember what's the reason for BLKRRPART?

Thanks,
Daniel

Re: Option to never do BLKRRPART ioctl in nvme format?

[Nick Neumann]

On Mon, Jan 30, 2023 at 3:47 AM Daniel Wagner <dwagner@suse.de> wrote:
> I can't really say why BLKRRPART was added initially. The comment which was
> added later on
>
> /*
>  * If block size has been changed by the format
>  * command up there, we should notify it to
>  * kernel blkdev to update its own block size
>  * to the given one because blkdev will not
>  * update by itself without re-opening fd.
>  */
>
> indicates we don't need BLKRRPART necessary. I would like to understand first
> what's going on before dropping BLKRRPART.
>
> @Keith, do you happen to remember what's the reason for BLKRRPART?

Thanks much for the response. In my more meandering original email
(before I composed this one), Keith had said:
> There really is no need for user space to do the BLKRRPART ioctl
> anymore. The kernel takes care of the rescan automatically, so unless
> you've a really old kernel, we might be better off just removing the
> ioctl."

I had added a sleep delay of a few seconds as a workaround for the
intermittent error from BLKRRPART ioctl. It worked great, until today.
After hundreds of formats without issue, I got "failed to re-read
partition table" once again. So I'll resort to allowing the error code
if the error message is precisely that, and hopefully it can be
removed from nvme format sometime soon. :-)