| Message ID | 20221005031701.79077-2-kch@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | null_blk: allow REQ_OP_WRITE_ZEROES and cleanup | expand |
On 10/5/22 12:16, Chaitanya Kulkarni wrote: > Add a helper function to enable the REQ_OP_WRITE_ZEROES operations > when null_blk is configured with the non-membacked operations. > > Since write-zeroes is a non-trivial I/O operation we need this to > add a blktest so we can test the non-trivial I/O path from the > application to the block layer. > > Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> > --- > drivers/block/null_blk/main.c | 13 +++++++++++++ > drivers/block/null_blk/null_blk.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c > index 1f154f92f4c2..fc3e883f7b84 100644 > --- a/drivers/block/null_blk/main.c > +++ b/drivers/block/null_blk/main.c > @@ -209,6 +209,10 @@ static bool g_discard; > module_param_named(discard, g_discard, bool, 0444); > MODULE_PARM_DESC(discard, "Support discard operations (requires memory-backed null_blk device). Default: false"); > > +static bool g_write_zeroes; > +module_param_named(write_zeroes, g_write_zeroes, bool, 0444); > +MODULE_PARM_DESC(write_zeroes, "Support write-zeores operations. Default: false"); Why not make this a number of sectors representing the maximum size of a write zero command (blk_queue_max_write_zeroes_sectors()) ? That would allow exercising split write zeros BIOs. > + > static unsigned long g_cache_size; > module_param_named(cache_size, g_cache_size, ulong, 0444); > MODULE_PARM_DESC(mbps, "Cache size in MiB for memory-backed device. Default: 0 (none)"); > @@ -678,6 +682,7 @@ static struct nullb_device *null_alloc_dev(void) > dev->blocking = g_blocking; > dev->memory_backed = g_memory_backed; > dev->discard = g_discard; > + dev->write_zeroes = g_write_zeroes; > dev->cache_size = g_cache_size; > dev->mbps = g_mbps; > dev->use_per_node_hctx = g_use_per_node_hctx; > @@ -1800,6 +1805,13 @@ static void null_config_discard(struct nullb *nullb) > blk_queue_max_discard_sectors(nullb->q, UINT_MAX >> 9); > } > > +static void null_config_write_zeroes(struct nullb *nullb) > +{ > + if (!nullb->dev->write_zeroes) > + return; > + blk_queue_max_write_zeroes_sectors(nullb->q, UINT_MAX >> 9); > +} > + > static const struct block_device_operations null_bio_ops = { > .owner = THIS_MODULE, > .submit_bio = null_submit_bio, > @@ -2111,6 +2123,7 @@ static int null_add_dev(struct nullb_device *dev) > blk_queue_virt_boundary(nullb->q, PAGE_SIZE - 1); > > null_config_discard(nullb); > + null_config_write_zeroes(nullb); > > if (config_item_name(&dev->item)) { > /* Use configfs dir name as the device name */ > diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h > index 94ff68052b1e..2c0c9c29158f 100644 > --- a/drivers/block/null_blk/null_blk.h > +++ b/drivers/block/null_blk/null_blk.h > @@ -111,6 +111,7 @@ struct nullb_device { > bool power; /* power on/off the device */ > bool memory_backed; /* if data is stored in memory */ > bool discard; /* if support discard */ > + bool write_zeroes; /* if support write_zeroes */ > bool zoned; /* if device is zoned */ > bool virt_boundary; /* virtual boundary on/off for the device */ > bool no_sched; /* no IO scheduler for the device */
On 10/4/22 21:54, Damien Le Moal wrote: > On 10/5/22 12:16, Chaitanya Kulkarni wrote: >> Add a helper function to enable the REQ_OP_WRITE_ZEROES operations >> when null_blk is configured with the non-membacked operations. >> >> Since write-zeroes is a non-trivial I/O operation we need this to >> add a blktest so we can test the non-trivial I/O path from the >> application to the block layer. >> >> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> >> --- >> drivers/block/null_blk/main.c | 13 +++++++++++++ >> drivers/block/null_blk/null_blk.h | 1 + >> 2 files changed, 14 insertions(+) >> >> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c >> index 1f154f92f4c2..fc3e883f7b84 100644 >> --- a/drivers/block/null_blk/main.c >> +++ b/drivers/block/null_blk/main.c >> @@ -209,6 +209,10 @@ static bool g_discard; >> module_param_named(discard, g_discard, bool, 0444); >> MODULE_PARM_DESC(discard, "Support discard operations (requires memory-backed null_blk device). Default: false"); >> >> +static bool g_write_zeroes; >> +module_param_named(write_zeroes, g_write_zeroes, bool, 0444); >> +MODULE_PARM_DESC(write_zeroes, "Support write-zeores operations. Default: false"); > > Why not make this a number of sectors representing the maximum size of a > write zero command (blk_queue_max_write_zeroes_sectors()) ? That would > allow exercising split write zeros BIOs. > I kept the implementation identical to the g_discard. Perhaps it's time to change it so REQ_OP_DISCARD and REQ_OP_WRITE_ZEROES will have same implementation. I'll add a discard patch to match your suggested write-zeroes behavior. -ck
>>> +static bool g_write_zeroes; >>> +module_param_named(write_zeroes, g_write_zeroes, bool, 0444); >>> +MODULE_PARM_DESC(write_zeroes, "Support write-zeores operations. Default: false"); >> >> Why not make this a number of sectors representing the maximum size of a >> write zero command (blk_queue_max_write_zeroes_sectors()) ? That would >> allow exercising split write zeros BIOs. >> > > I kept the implementation identical to the g_discard. > > Perhaps it's time to change it so REQ_OP_DISCARD and > REQ_OP_WRITE_ZEROES will have same implementation. > > I'll add a discard patch to match your suggested write-zeroes > behavior. > > -ck > REQ_OP_DISCARD and REQ_OP_WRITE_ZEORES needs to be consistent when it comes to configuration interface. I did the change you suggested, it is breaking the backward compatibility of discard and if we do it only for write-zeroes it will be inconsistent with the current g_discard behavior. Let's keep the original behavior and not break the backward compatibility ? -ck
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 1f154f92f4c2..fc3e883f7b84 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -209,6 +209,10 @@ static bool g_discard; module_param_named(discard, g_discard, bool, 0444); MODULE_PARM_DESC(discard, "Support discard operations (requires memory-backed null_blk device). Default: false"); +static bool g_write_zeroes; +module_param_named(write_zeroes, g_write_zeroes, bool, 0444); +MODULE_PARM_DESC(write_zeroes, "Support write-zeores operations. Default: false"); + static unsigned long g_cache_size; module_param_named(cache_size, g_cache_size, ulong, 0444); MODULE_PARM_DESC(mbps, "Cache size in MiB for memory-backed device. Default: 0 (none)"); @@ -678,6 +682,7 @@ static struct nullb_device *null_alloc_dev(void) dev->blocking = g_blocking; dev->memory_backed = g_memory_backed; dev->discard = g_discard; + dev->write_zeroes = g_write_zeroes; dev->cache_size = g_cache_size; dev->mbps = g_mbps; dev->use_per_node_hctx = g_use_per_node_hctx; @@ -1800,6 +1805,13 @@ static void null_config_discard(struct nullb *nullb) blk_queue_max_discard_sectors(nullb->q, UINT_MAX >> 9); } +static void null_config_write_zeroes(struct nullb *nullb) +{ + if (!nullb->dev->write_zeroes) + return; + blk_queue_max_write_zeroes_sectors(nullb->q, UINT_MAX >> 9); +} + static const struct block_device_operations null_bio_ops = { .owner = THIS_MODULE, .submit_bio = null_submit_bio, @@ -2111,6 +2123,7 @@ static int null_add_dev(struct nullb_device *dev) blk_queue_virt_boundary(nullb->q, PAGE_SIZE - 1); null_config_discard(nullb); + null_config_write_zeroes(nullb); if (config_item_name(&dev->item)) { /* Use configfs dir name as the device name */ diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h index 94ff68052b1e..2c0c9c29158f 100644 --- a/drivers/block/null_blk/null_blk.h +++ b/drivers/block/null_blk/null_blk.h @@ -111,6 +111,7 @@ struct nullb_device { bool power; /* power on/off the device */ bool memory_backed; /* if data is stored in memory */ bool discard; /* if support discard */ + bool write_zeroes; /* if support write_zeroes */ bool zoned; /* if device is zoned */ bool virt_boundary; /* virtual boundary on/off for the device */ bool no_sched; /* no IO scheduler for the device */
Add a helper function to enable the REQ_OP_WRITE_ZEROES operations when null_blk is configured with the non-membacked operations. Since write-zeroes is a non-trivial I/O operation we need this to add a blktest so we can test the non-trivial I/O path from the application to the block layer. Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> --- drivers/block/null_blk/main.c | 13 +++++++++++++ drivers/block/null_blk/null_blk.h | 1 + 2 files changed, 14 insertions(+)