On 21-01-04 00:57:07, Chaitanya Kulkarni wrote:

On 12/30/20 08:03, Minwoo Im wrote:

quoted

Let's say, for example of NVMe device, Format command to change out
LBA format to another logical block size and BLKRRPART to re-read
partition information with a same file descriptor like:

	fd = open("/dev/nvme0n1", O_RDONLY);

	nvme_format(fd, ...);
	if (ioctl(fd, BLKRRPART) < 0)
		..

In this case, ioctl causes invalid Read operations which are triggered
by buffer_head I/O path to re-read partition information.  This is
because it's still playing around with i_blksize and i_blkbits.  So,
512 -> 4096 -> 512 logical block size changes will cause an under-flowed
length of Read operations.

Case for NVMe:
  (LBAF 1 512B, LBAF 0 4K logical block size)

  nvme format /dev/nvme0n1 --lbaf=1 --force  # to 512B LBA
  nvme format /dev/nvme0n1 --lbaf=0 --force  # to 4096B LBA

[dmesg-snip]
  [   10.771740] blk_update_request: operation not supported error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
  [   10.780262] Buffer I/O error on dev nvme0n1, logical block 0, async page read

[event-snip]
  kworker/0:1H-56      [000] ....   913.456922: nvme_setup_cmd: nvme0: disk=nvme0n1, qid=1, cmdid=216, nsid=1, flags=0x0, meta=0x0, cmd=(nvme_cmd_read slba=0, len=65535, ctrl=0x0, dsmgmt=0, reftag=0)
   ksoftirqd/0-9       [000] .Ns.   916.566351: nvme_complete_rq: nvme0: disk=nvme0n1, qid=1, cmdid=216, res=0x0, retries=0, flags=0x0, status=0x4002

As the previous discussion [1], this patch introduced a gendisk flag
to indicate that block size has been changed in the runtime.  This flag
is set when logical block size is changed in the runtime in the block
layer.  It will be cleared when the file descriptor for the
block devie is opened again through __blkdev_get() which updates the block
size via set_init_blocksize().

This patch rejects I/O from the path of add_partitions() to avoid
issuing invalid Read operations to device.  It also sets a flag to
gendisk in blk_queue_logical_block_size to minimize caller-side updates.

[1] https://lore.kernel.org/linux-nvme/20201223183143.GB13354@localhost.localdomain/T/#t (local)

Signed-off-by: Minwoo Im <redacted>

Rewrite the change-log similar to what we have in the repo and fix
the spelling mistakes. Add a cover-letter to explain the testcase
and the execution effect, also I'd move discussion link into
cover-letter.

Thanks for your time.  Will prepare V3 with proper change logs and cover
letter to explain issue and testcase.

Thanks,