[PATCH 0/3] nvmet: add buffered_io support for bdev-ns

[Chaitanya Kulkarni]

Christoph,

On 1/26/22 8:27 AM, Christoph Hellwig wrote:
> External email: Use caution opening links or attachments
> 
> 
> Shouldn't something like the patch below be all we need to use
> buffered I/O on a block device?
> 
> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
> index 70ca9dfc1771a..7d9da2f77b83a 100644
> --- a/drivers/nvme/target/io-cmd-bdev.c
> +++ b/drivers/nvme/target/io-cmd-bdev.c
> @@ -76,6 +76,10 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>   {
>          int ret;
> 
> +       /* force fallback to the file backend for buffered I/O mode */
> +       if (ns->buffered_io)
> +               return -ENOTBLK;
> +
>          ns->bdev = blkdev_get_by_path(ns->device_path,
>                          FMODE_READ | FMODE_WRITE, NULL);
>          if (IS_ERR(ns->bdev)) {
> 

This is exactly how I stared this patch-series, but above doesn't
work since it errors out in the nvme_identify_ns() due to ncap value
set to 0 (set from ns->size in id-ns admin on target side) see [1],
since ns->size value we get is 0, that is set using call to
vfs_getattr(STATX_SIZE) in the nvmet_file_ns_revalidate()
for file backend.


[1]
[  579.444478] nvme nvme0: 48/0/0 default/read/poll queues
[  581.430805] nvmet: adding nsid 1 to subsystem testnqn
[  582.439296] nvmet: creating nvm controller 1 for subsystem testnqn 
for NQN 
nqn.2014-08.org.nvmexpress:uuid:23f0992c-7ef7-4fbd-80f7-209cda6d872a.
[  582.439429] nvme nvme1: creating 48 I/O queues.
[  582.444036] nvme nvme1: new ctrl: "testnqn"
**[  582.444095] nvmet: nvmet_execute_identify_ns 520 req->ns->size 0 >> 
req->ns->blksize_shift 12 = 0**
**[  582.444103]  ERROR nvme_identify_ns 1473**
++ sleep 5

Debug diff :-

root at dev nvme (nvme-5.17) # git diff
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dd18861f77c0..5a33b3f73b3b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1469,8 +1469,10 @@ static int nvme_identify_ns(struct nvme_ctrl 
*ctrl, unsigned nsid,
         }

         error = NVME_SC_INVALID_NS | NVME_SC_DNR;
-       if ((*id)->ncap == 0) /* namespace not allocated or attached */
+       if ((*id)->ncap == 0) { /* namespace not allocated or attached */
+               pr_info(" ERROR %s %d\n", __func__, __LINE__);
                 goto out_free_id;
+       }

         if (ctrl->vs >= NVME_VS(1, 1, 0) &&
             !memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
diff --git a/drivers/nvme/target/admin-cmd.c 
b/drivers/nvme/target/admin-cmd.c
index 6fb24746de06..94f158aa7bfa 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -517,6 +517,9 @@ static void nvmet_execute_identify_ns(struct 
nvmet_req *req)
          * nuse = ncap = nsze isn't always true, but we have no way to find
          * that out from the underlying device.
          */
+       pr_info("%s %d req->ns->size %u >> req->ns->blksize_shift %u = 
%u\n",
+                       __func__, __LINE__, req->ns->size, 
req->ns->blksize_shift,
+                       req->ns->size >> req->ns->blksize_shift);
         id->ncap = id->nsze =
                 cpu_to_le64(req->ns->size >> req->ns->blksize_shift);
         switch (req->port->ana_state[req->ns->anagrpid]) {
diff --git a/drivers/nvme/target/io-cmd-bdev.c 
b/drivers/nvme/target/io-cmd-bdev.c
index 70ca9dfc1771..7d9da2f77b83 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -76,6 +76,10 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
  {
         int ret;

+       /* force fallback to the file backend for buffered I/O mode */
+       if (ns->buffered_io)
+               return -ENOTBLK;
+
         ns->bdev = blkdev_get_by_path(ns->device_path,
                         FMODE_READ | FMODE_WRITE, NULL);
         if (IS_ERR(ns->bdev)) {
root at dev nvme (nvme-5.17) #