* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
@ 2019-04-24 18:43 Sagi Grimberg
2019-04-25 0:42 ` Minwoo Im
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sagi Grimberg @ 2019-04-24 18:43 UTC (permalink / raw)
When the backing file is a tempfile for example, the inode i_blkbits
can be 1M in size which causes problems for hosts to support as the
disk block size. Instead, expose the minimum between i_blkbits and
12 (4K sector size).
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
Changes from v1:
- expose block size 4096 instead of PAGE_SIZE
drivers/nvme/target/io-cmd-file.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index bc6ebb51b0bf..66013d8ad94b 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -49,7 +49,12 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
goto err;
ns->size = stat.size;
- ns->blksize_shift = file_inode(ns->file)->i_blkbits;
+ /*
+ * inode i_blkbits can be greater than the universally accepted upper
+ * bound so make sure we export we export a sane namespace lba_shift.
+ */
+ ns->blksize_shift = min_t(u8,
+ file_inode(ns->file)->i_blkbits, 12);
ns->bvec_cache = kmem_cache_create("nvmet-bvec",
NVMET_MAX_MPOOL_BVEC * sizeof(struct bio_vec),
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
2019-04-24 18:43 [PATCH v2] nvmet-file: clamp-down file namespace lba_shift Sagi Grimberg
@ 2019-04-25 0:42 ` Minwoo Im
2019-04-25 0:57 ` Chaitanya Kulkarni
2019-04-25 14:53 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: Minwoo Im @ 2019-04-25 0:42 UTC (permalink / raw)
On 4/25/19 3:43 AM, Sagi Grimberg wrote:
> When the backing file is a tempfile for example, the inode i_blkbits
> can be 1M in size which causes problems for hosts to support as the
> disk block size. Instead, expose the minimum between i_blkbits and
> 12 (4K sector size).
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> Changes from v1:
> - expose block size 4096 instead of PAGE_SIZE
>
> drivers/nvme/target/io-cmd-file.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index bc6ebb51b0bf..66013d8ad94b 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -49,7 +49,12 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
> goto err;
>
> ns->size = stat.size;
> - ns->blksize_shift = file_inode(ns->file)->i_blkbits;
> + /*
> + * inode i_blkbits can be greater than the universally accepted upper
> + * bound so make sure we export we export a sane namespace lba_shift.
'we export' is duplicated here. :)
Thanks,
> + */
> + ns->blksize_shift = min_t(u8,
> + file_inode(ns->file)->i_blkbits, 12);
>
> ns->bvec_cache = kmem_cache_create("nvmet-bvec",
> NVMET_MAX_MPOOL_BVEC * sizeof(struct bio_vec),
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
2019-04-24 18:43 [PATCH v2] nvmet-file: clamp-down file namespace lba_shift Sagi Grimberg
2019-04-25 0:42 ` Minwoo Im
@ 2019-04-25 0:57 ` Chaitanya Kulkarni
2019-04-25 14:53 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2019-04-25 0:57 UTC (permalink / raw)
Looks good from the duplicate "we export".
Reviewed-by:- Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 04/24/2019 11:43 AM, Sagi Grimberg wrote:
> When the backing file is a tempfile for example, the inode i_blkbits
> can be 1M in size which causes problems for hosts to support as the
> disk block size. Instead, expose the minimum between i_blkbits and
> 12 (4K sector size).
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> Changes from v1:
> - expose block size 4096 instead of PAGE_SIZE
>
> drivers/nvme/target/io-cmd-file.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index bc6ebb51b0bf..66013d8ad94b 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -49,7 +49,12 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
> goto err;
>
> ns->size = stat.size;
> - ns->blksize_shift = file_inode(ns->file)->i_blkbits;
> + /*
> + * inode i_blkbits can be greater than the universally accepted upper
> + * bound so make sure we export we export a sane namespace lba_shift.
> + */
> + ns->blksize_shift = min_t(u8,
> + file_inode(ns->file)->i_blkbits, 12);
>
> ns->bvec_cache = kmem_cache_create("nvmet-bvec",
> NVMET_MAX_MPOOL_BVEC * sizeof(struct bio_vec),
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
2019-04-24 18:43 [PATCH v2] nvmet-file: clamp-down file namespace lba_shift Sagi Grimberg
2019-04-25 0:42 ` Minwoo Im
2019-04-25 0:57 ` Chaitanya Kulkarni
@ 2019-04-25 14:53 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-04-25 14:53 UTC (permalink / raw)
Thanks,
applied to nvme-5.2.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
2019-03-09 2:13 Sagi Grimberg
@ 2019-03-11 14:12 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-03-11 14:12 UTC (permalink / raw)
On Fri, Mar 08, 2019@06:13:40PM -0800, Sagi Grimberg wrote:
> When the backing file is a tempfile for example, the inode i_blkbits
> can be 1M in size which causes problems for hosts to support as the
> disk block size.
Can you please look into the other review comments as well?
Also this will need a code comment describing why we do this clamping.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvmet-file: clamp-down file namespace lba_shift
@ 2019-03-09 2:13 Sagi Grimberg
2019-03-11 14:12 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2019-03-09 2:13 UTC (permalink / raw)
When the backing file is a tempfile for example, the inode i_blkbits
can be 1M in size which causes problems for hosts to support as the
disk block size.
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
Changes from v1:
- don't cross 80 char limit
drivers/nvme/target/io-cmd-file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 517522305e5c..9876751fed05 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -49,7 +49,8 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
goto err;
ns->size = stat.size;
- ns->blksize_shift = file_inode(ns->file)->i_blkbits;
+ ns->blksize_shift = min_t(u8,
+ file_inode(ns->file)->i_blkbits, PAGE_SHIFT);
ns->bvec_cache = kmem_cache_create("nvmet-bvec",
NVMET_MAX_MPOOL_BVEC * sizeof(struct bio_vec),
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-25 14:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 18:43 [PATCH v2] nvmet-file: clamp-down file namespace lba_shift Sagi Grimberg
2019-04-25 0:42 ` Minwoo Im
2019-04-25 0:57 ` Chaitanya Kulkarni
2019-04-25 14:53 ` Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2019-03-09 2:13 Sagi Grimberg
2019-03-11 14:12 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.