From: SeongJae Park <[email protected]>
Persistent grants feature provides high scalability. On some small
systems, however, it could incur data copy overheads[1] and thus it is
required to be disabled. But, there is no option to disable it. For
the reason, this commit adds module parameters for disabling of the
feature.
[1] https://wiki.xen.org/wiki/Xen_4.3_Block_Protocol_Scalability
Baseline and Complete Git Trees
===============================
The patches are based on the v5.9-rc6. You can also clone the complete
git tree:
$ git clone git://github.com/sjp38/linux -b pgrants_disable_v3
The web is also available:
https://github.com/sjp38/linux/tree/pgrants_disable_v3
Patch History
=============
Changes from v2
(https://lore.kernel.org/linux-block/[email protected]/)
- Avoid race conditions (Roger Pau Monné)
Changes from v1
(https://lore.kernel.org/linux-block/[email protected]/)
- use 'bool' parameter type (Jürgen Groß)
- Let blkfront can also disable the feature from its side
(Roger Pau Monné)
- Avoid unnecessary xenbus_printf (Roger Pau Monné)
- Update frontend parameter doc
SeongJae Park (3):
xen-blkback: add a parameter for disabling of persistent grants
xen-blkfront: add a parameter for disabling of persistent grants
xen-blkfront: Apply changed parameter name to the document
.../ABI/testing/sysfs-driver-xen-blkback | 9 ++++++++
.../ABI/testing/sysfs-driver-xen-blkfront | 11 +++++++++-
drivers/block/xen-blkback/xenbus.c | 22 ++++++++++++++-----
drivers/block/xen-blkfront.c | 20 ++++++++++++-----
4 files changed, 50 insertions(+), 12 deletions(-)
--
2.17.1
From: SeongJae Park <[email protected]>
Persistent grants feature provides high scalability. On some small
systems, however, it could incur data copy overheads[1] and thus it is
required to be disabled. But, there is no option to disable it. For
the reason, this commit adds a module parameter for disabling of the
feature.
[1] https://wiki.xen.org/wiki/Xen_4.3_Block_Protocol_Scalability
Signed-off-by: Anthony Liguori <[email protected]>
Signed-off-by: SeongJae Park <[email protected]>
---
.../ABI/testing/sysfs-driver-xen-blkback | 9 ++++++++
drivers/block/xen-blkback/xenbus.c | 22 ++++++++++++++-----
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-xen-blkback b/Documentation/ABI/testing/sysfs-driver-xen-blkback
index ecb7942ff146..ac2947b98950 100644
--- a/Documentation/ABI/testing/sysfs-driver-xen-blkback
+++ b/Documentation/ABI/testing/sysfs-driver-xen-blkback
@@ -35,3 +35,12 @@ Description:
controls the duration in milliseconds that blkback will not
cache any page not backed by a grant mapping.
The default is 10ms.
+
+What: /sys/module/xen_blkback/parameters/feature_persistent
+Date: September 2020
+KernelVersion: 5.10
+Contact: SeongJae Park <[email protected]>
+Description:
+ Whether to enable the persistent grants feature or not. Note
+ that this option only takes effect on newly created backends.
+ The default is Y (enable).
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index b9aa5d1ac10b..f4c8827fa0ad 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -474,6 +474,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
vbd->bdev = NULL;
}
+/* Enable the persistent grants feature. */
+static bool feature_persistent = true;
+module_param(feature_persistent, bool, 0644);
+MODULE_PARM_DESC(feature_persistent,
+ "Enables the persistent grants feature");
+
static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
unsigned major, unsigned minor, int readonly,
int cdrom)
@@ -519,6 +525,8 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
if (q && blk_queue_secure_erase(q))
vbd->discard_secure = true;
+ vbd->feature_gnt_persistent = feature_persistent ? 1 : 0;
+
pr_debug("Successful creation of handle=%04x (dom=%u)\n",
handle, blkif->domid);
return 0;
@@ -906,7 +914,8 @@ static void connect(struct backend_info *be)
xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support);
- err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u", 1);
+ err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u",
+ be->blkif->vbd.feature_gnt_persistent);
if (err) {
xenbus_dev_fatal(dev, err, "writing %s/feature-persistent",
dev->nodename);
@@ -1067,7 +1076,6 @@ static int connect_ring(struct backend_info *be)
{
struct xenbus_device *dev = be->dev;
struct xen_blkif *blkif = be->blkif;
- unsigned int pers_grants;
char protocol[64] = "";
int err, i;
char *xspath;
@@ -1093,9 +1101,11 @@ static int connect_ring(struct backend_info *be)
xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
return -ENOSYS;
}
- pers_grants = xenbus_read_unsigned(dev->otherend, "feature-persistent",
- 0);
- blkif->vbd.feature_gnt_persistent = pers_grants;
+ if (blkif->vbd.feature_gnt_persistent)
+ blkif->vbd.feature_gnt_persistent =
+ xenbus_read_unsigned(dev->otherend,
+ "feature-persistent", 0);
+
blkif->vbd.overflow_max_grants = 0;
/*
@@ -1118,7 +1128,7 @@ static int connect_ring(struct backend_info *be)
pr_info("%s: using %d queues, protocol %d (%s) %s\n", dev->nodename,
blkif->nr_rings, blkif->blk_protocol, protocol,
- pers_grants ? "persistent grants" : "");
+ blkif->vbd.feature_gnt_persistent ? "persistent grants" : "");
ring_page_order = xenbus_read_unsigned(dev->otherend,
"ring-page-order", 0);
--
2.17.1
From: SeongJae Park <[email protected]>
Commit 14e710fe7897 ("xen-blkfront: rename indirect descriptor
parameter") changed the name of the module parameter for the maximum
amount of segments in indirect requests but missed updating the
document. This commit updates the document.
Signed-off-by: SeongJae Park <[email protected]>
---
Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-xen-blkfront b/Documentation/ABI/testing/sysfs-driver-xen-blkfront
index 9c31334cb2e6..28008905615f 100644
--- a/Documentation/ABI/testing/sysfs-driver-xen-blkfront
+++ b/Documentation/ABI/testing/sysfs-driver-xen-blkfront
@@ -1,4 +1,4 @@
-What: /sys/module/xen_blkfront/parameters/max
+What: /sys/module/xen_blkfront/parameters/max_indirect_segments
Date: June 2013
KernelVersion: 3.11
Contact: Konrad Rzeszutek Wilk <[email protected]>
--
2.17.1
On 22.09.20 16:15, SeongJae Park wrote:
> From: SeongJae Park <[email protected]>
>
> Persistent grants feature provides high scalability. On some small
> systems, however, it could incur data copy overheads[1] and thus it is
> required to be disabled. But, there is no option to disable it. For
> the reason, this commit adds a module parameter for disabling of the
> feature.
>
> [1] https://wiki.xen.org/wiki/Xen_4.3_Block_Protocol_Scalability
>
> Signed-off-by: Anthony Liguori <[email protected]>
> Signed-off-by: SeongJae Park <[email protected]>
> ---
> .../ABI/testing/sysfs-driver-xen-blkback | 9 ++++++++
> drivers/block/xen-blkback/xenbus.c | 22 ++++++++++++++-----
> 2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-xen-blkback b/Documentation/ABI/testing/sysfs-driver-xen-blkback
> index ecb7942ff146..ac2947b98950 100644
> --- a/Documentation/ABI/testing/sysfs-driver-xen-blkback
> +++ b/Documentation/ABI/testing/sysfs-driver-xen-blkback
> @@ -35,3 +35,12 @@ Description:
> controls the duration in milliseconds that blkback will not
> cache any page not backed by a grant mapping.
> The default is 10ms.
> +
> +What: /sys/module/xen_blkback/parameters/feature_persistent
> +Date: September 2020
> +KernelVersion: 5.10
> +Contact: SeongJae Park <[email protected]>
> +Description:
> + Whether to enable the persistent grants feature or not. Note
> + that this option only takes effect on newly created backends.
> + The default is Y (enable).
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index b9aa5d1ac10b..f4c8827fa0ad 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -474,6 +474,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
> vbd->bdev = NULL;
> }
>
> +/* Enable the persistent grants feature. */
> +static bool feature_persistent = true;
> +module_param(feature_persistent, bool, 0644);
> +MODULE_PARM_DESC(feature_persistent,
> + "Enables the persistent grants feature");
> +
> static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
> unsigned major, unsigned minor, int readonly,
> int cdrom)
> @@ -519,6 +525,8 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
> if (q && blk_queue_secure_erase(q))
> vbd->discard_secure = true;
>
> + vbd->feature_gnt_persistent = feature_persistent ? 1 : 0;
Just assign the value instead of using the ternary operator?
With that changed you can add my
Reviewed-by: Juergen Gross <[email protected]>
Juergen
On 22.09.20 16:15, SeongJae Park wrote:
> From: SeongJae Park <[email protected]>
>
> Commit 14e710fe7897 ("xen-blkfront: rename indirect descriptor
> parameter") changed the name of the module parameter for the maximum
> amount of segments in indirect requests but missed updating the
> document. This commit updates the document.
>
> Signed-off-by: SeongJae Park <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Juergen
On Tue, 22 Sep 2020 16:25:57 +0200 "Jürgen Groß" <[email protected]> wrote:
> On 22.09.20 16:15, SeongJae Park wrote:
> > From: SeongJae Park <[email protected]>
> >
> > Persistent grants feature provides high scalability. On some small
> > systems, however, it could incur data copy overheads[1] and thus it is
> > required to be disabled. But, there is no option to disable it. For
> > the reason, this commit adds a module parameter for disabling of the
> > feature.
> >
> > [1] https://wiki.xen.org/wiki/Xen_4.3_Block_Protocol_Scalability
> >
> > Signed-off-by: Anthony Liguori <[email protected]>
> > Signed-off-by: SeongJae Park <[email protected]>
> > ---
> > .../ABI/testing/sysfs-driver-xen-blkback | 9 ++++++++
> > drivers/block/xen-blkback/xenbus.c | 22 ++++++++++++++-----
> > 2 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-driver-xen-blkback b/Documentation/ABI/testing/sysfs-driver-xen-blkback
> > index ecb7942ff146..ac2947b98950 100644
> > --- a/Documentation/ABI/testing/sysfs-driver-xen-blkback
> > +++ b/Documentation/ABI/testing/sysfs-driver-xen-blkback
> > @@ -35,3 +35,12 @@ Description:
> > controls the duration in milliseconds that blkback will not
> > cache any page not backed by a grant mapping.
> > The default is 10ms.
> > +
> > +What: /sys/module/xen_blkback/parameters/feature_persistent
> > +Date: September 2020
> > +KernelVersion: 5.10
> > +Contact: SeongJae Park <[email protected]>
> > +Description:
> > + Whether to enable the persistent grants feature or not. Note
> > + that this option only takes effect on newly created backends.
> > + The default is Y (enable).
> > diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> > index b9aa5d1ac10b..f4c8827fa0ad 100644
> > --- a/drivers/block/xen-blkback/xenbus.c
> > +++ b/drivers/block/xen-blkback/xenbus.c
> > @@ -474,6 +474,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
> > vbd->bdev = NULL;
> > }
> >
> > +/* Enable the persistent grants feature. */
> > +static bool feature_persistent = true;
> > +module_param(feature_persistent, bool, 0644);
> > +MODULE_PARM_DESC(feature_persistent,
> > + "Enables the persistent grants feature");
> > +
> > static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
> > unsigned major, unsigned minor, int readonly,
> > int cdrom)
> > @@ -519,6 +525,8 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
> > if (q && blk_queue_secure_erase(q))
> > vbd->discard_secure = true;
> >
> > + vbd->feature_gnt_persistent = feature_persistent ? 1 : 0;
>
> Just assign the value instead of using the ternary operator?
I will do so in the next version.
>
> With that changed you can add my
>
> Reviewed-by: Juergen Gross <[email protected]>
Thank you for your kind, quick, and detailed review!
Thanks,
SeongJae Park
On Tue, 22 Sep 2020 16:45:56 +0200 "Roger Pau Monné" <[email protected]> wrote:
> On Tue, Sep 22, 2020 at 04:15:46PM +0200, SeongJae Park wrote:
> > From: SeongJae Park <[email protected]>
> >
> > Persistent grants feature provides high scalability. On some small
> > systems, however, it could incur data copy overheads[1] and thus it is
> > required to be disabled. But, there is no option to disable it. For
> > the reason, this commit adds module parameters for disabling of the
> > feature.
>
> With the changes requested by Jürgen you can add my:
>
> Acked-by: Roger Pau Monné <[email protected]>
Thank you for your kind and helpful reviews!
Thanks,
SeongJae Park
On Tue, 22 Sep 2020 16:44:25 +0200 "Roger Pau Monné" <[email protected]> wrote:
> On Tue, Sep 22, 2020 at 04:27:39PM +0200, Jürgen Groß wrote:
> > On 22.09.20 16:15, SeongJae Park wrote:
> > > From: SeongJae Park <[email protected]>
> > >
> > > Commit 14e710fe7897 ("xen-blkfront: rename indirect descriptor
> > > parameter") changed the name of the module parameter for the maximum
> > > amount of segments in indirect requests but missed updating the
> > > document. This commit updates the document.
> > >
> > > Signed-off-by: SeongJae Park <[email protected]>
> >
> > Reviewed-by: Juergen Gross <[email protected]>
>
> Does this need to be backported to stable branches?
I don't think so, as this is not a bug affecting users?
Thanks,
SeongJae Park