-
-
Notifications
You must be signed in to change notification settings - Fork 63
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Backport fix for persistent grants negotiation in blkfront/back
- Loading branch information
Showing
4 changed files
with
219 additions
and
0 deletions.
There are no files selected for viewing
69 changes: 69 additions & 0 deletions
69
0001-xen-blkback-Advertise-feature-persistent-as-user-req.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| From 7c8235975a13f96377213dea0e21b805f129a4ba Mon Sep 17 00:00:00 2001 | ||
| From: SeongJae Park <sj@kernel.org> | ||
| Date: Wed, 31 Aug 2022 16:58:22 +0000 | ||
| Subject: [PATCH 1/3] xen-blkback: Advertise feature-persistent as user | ||
| requested | ||
|
|
||
| The advertisement of the persistent grants feature (writing | ||
| 'feature-persistent' to xenbus) should mean not the decision for using | ||
| the feature but only the availability of the feature. However, commit | ||
| aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent | ||
| grants") made a field of blkback, which was a place for saving only the | ||
| negotiation result, to be used for yet another purpose: caching of the | ||
| 'feature_persistent' parameter value. As a result, the advertisement, | ||
| which should follow only the parameter value, becomes inconsistent. | ||
|
|
||
| This commit fixes the misuse of the semantic by making blkback saves the | ||
| parameter value in a separate place and advertises the support based on | ||
| only the saved value. | ||
|
|
||
| Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") | ||
| Cc: <stable@vger.kernel.org> # 5.10.x | ||
| Suggested-by: Juergen Gross <jgross@suse.com> | ||
| Signed-off-by: SeongJae Park <sj@kernel.org> | ||
| --- | ||
| drivers/block/xen-blkback/common.h | 3 +++ | ||
| drivers/block/xen-blkback/xenbus.c | 6 ++++-- | ||
| 2 files changed, 7 insertions(+), 2 deletions(-) | ||
|
|
||
| diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h | ||
| index bda5c815e441..a28473470e66 100644 | ||
| --- a/drivers/block/xen-blkback/common.h | ||
| +++ b/drivers/block/xen-blkback/common.h | ||
| @@ -226,6 +226,9 @@ struct xen_vbd { | ||
| sector_t size; | ||
| unsigned int flush_support:1; | ||
| unsigned int discard_secure:1; | ||
| + /* Connect-time cached feature_persistent parameter value */ | ||
| + unsigned int feature_gnt_persistent_parm:1; | ||
| + /* Persistent grants feature negotiation result */ | ||
| unsigned int feature_gnt_persistent:1; | ||
| unsigned int overflow_max_grants:1; | ||
| }; | ||
| diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c | ||
| index ee7ad2fb432d..c0227dfa4688 100644 | ||
| --- a/drivers/block/xen-blkback/xenbus.c | ||
| +++ b/drivers/block/xen-blkback/xenbus.c | ||
| @@ -907,7 +907,7 @@ 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", | ||
| - be->blkif->vbd.feature_gnt_persistent); | ||
| + be->blkif->vbd.feature_gnt_persistent_parm); | ||
| if (err) { | ||
| xenbus_dev_fatal(dev, err, "writing %s/feature-persistent", | ||
| dev->nodename); | ||
| @@ -1085,7 +1085,9 @@ static int connect_ring(struct backend_info *be) | ||
| return -ENOSYS; | ||
| } | ||
|
|
||
| - blkif->vbd.feature_gnt_persistent = feature_persistent && | ||
| + blkif->vbd.feature_gnt_persistent_parm = feature_persistent; | ||
| + blkif->vbd.feature_gnt_persistent = | ||
| + blkif->vbd.feature_gnt_persistent_parm && | ||
| xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); | ||
|
|
||
| blkif->vbd.overflow_max_grants = 0; | ||
| -- | ||
| 2.35.3 | ||
|
|
63 changes: 63 additions & 0 deletions
63
0002-xen-blkfront-Advertise-feature-persistent-as-user-re.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| From 23d8fe27a8a5f5ba5182d2ee5b3eb9b3072efdeb Mon Sep 17 00:00:00 2001 | ||
| From: SeongJae Park <sj@kernel.org> | ||
| Date: Wed, 31 Aug 2022 16:58:23 +0000 | ||
| Subject: [PATCH 2/3] xen-blkfront: Advertise feature-persistent as user | ||
| requested | ||
|
|
||
| The advertisement of the persistent grants feature (writing | ||
| 'feature-persistent' to xenbus) should mean not the decision for using | ||
| the feature but only the availability of the feature. However, commit | ||
| 74a852479c68 ("xen-blkfront: add a parameter for disabling of persistent | ||
| grants") made a field of blkfront, which was a place for saving only the | ||
| negotiation result, to be used for yet another purpose: caching of the | ||
| 'feature_persistent' parameter value. As a result, the advertisement, | ||
| which should follow only the parameter value, becomes inconsistent. | ||
|
|
||
| This commit fixes the misuse of the semantic by making blkfront saves | ||
| the parameter value in a separate place and advertises the support based | ||
| on only the saved value. | ||
|
|
||
| Fixes: 74a852479c68 ("xen-blkfront: add a parameter for disabling of persistent grants") | ||
| Cc: <stable@vger.kernel.org> # 5.10.x | ||
| Suggested-by: Juergen Gross <jgross@suse.com> | ||
| Signed-off-by: SeongJae Park <sj@kernel.org> | ||
| --- | ||
| drivers/block/xen-blkfront.c | 8 ++++++-- | ||
| 1 file changed, 6 insertions(+), 2 deletions(-) | ||
|
|
||
| diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c | ||
| index 8e56e69fb4c4..dfae08115450 100644 | ||
| --- a/drivers/block/xen-blkfront.c | ||
| +++ b/drivers/block/xen-blkfront.c | ||
| @@ -213,6 +213,9 @@ struct blkfront_info | ||
| unsigned int feature_fua:1; | ||
| unsigned int feature_discard:1; | ||
| unsigned int feature_secdiscard:1; | ||
| + /* Connect-time cached feature_persistent parameter */ | ||
| + unsigned int feature_persistent_parm:1; | ||
| + /* Persistent grants feature negotiation result */ | ||
| unsigned int feature_persistent:1; | ||
| unsigned int bounce:1; | ||
| unsigned int discard_granularity; | ||
| @@ -1848,7 +1851,7 @@ static int talk_to_blkback(struct xenbus_device *dev, | ||
| goto abort_transaction; | ||
| } | ||
| err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u", | ||
| - info->feature_persistent); | ||
| + info->feature_persistent_parm); | ||
| if (err) | ||
| dev_warn(&dev->dev, | ||
| "writing persistent grants feature to xenbus"); | ||
| @@ -2281,7 +2284,8 @@ static void blkfront_gather_backend_features(struct blkfront_info *info) | ||
| if (xenbus_read_unsigned(info->xbdev->otherend, "feature-discard", 0)) | ||
| blkfront_setup_discard(info); | ||
|
|
||
| - if (feature_persistent) | ||
| + info->feature_persistent_parm = feature_persistent; | ||
| + if (info->feature_persistent_parm) | ||
| info->feature_persistent = | ||
| !!xenbus_read_unsigned(info->xbdev->otherend, | ||
| "feature-persistent", 0); | ||
| -- | ||
| 2.35.3 | ||
|
|
84 changes: 84 additions & 0 deletions
84
0003-xen-blkfront-Cache-feature_persistent-value-before-a.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| From a6889ca1f9f39f18aacb469142b5aba22c672ecc Mon Sep 17 00:00:00 2001 | ||
| From: SeongJae Park <sj@kernel.org> | ||
| Date: Wed, 31 Aug 2022 16:58:24 +0000 | ||
| Subject: [PATCH 3/3] xen-blkfront: Cache feature_persistent value before | ||
| advertisement | ||
| MIME-Version: 1.0 | ||
| Content-Type: text/plain; charset=UTF-8 | ||
| Content-Transfer-Encoding: 8bit | ||
|
|
||
| Xen blkfront advertises its support of the persistent grants feature | ||
| when it first setting up and when resuming in 'talk_to_blkback()'. | ||
| Then, blkback reads the advertised value when it connects with blkfront | ||
| and decides if it will use the persistent grants feature or not, and | ||
| advertises its decision to blkfront. Blkfront reads the blkback's | ||
| decision and it also makes the decision for the use of the feature. | ||
|
|
||
| Commit 402c43ea6b34 ("xen-blkfront: Apply 'feature_persistent' parameter | ||
| when connect"), however, made the blkfront's read of the parameter for | ||
| disabling the advertisement, namely 'feature_persistent', to be done | ||
| when it negotiate, not when advertise. Therefore blkfront advertises | ||
| without reading the parameter. As the field for caching the parameter | ||
| value is zero-initialized, it always advertises as the feature is | ||
| disabled, so that the persistent grants feature becomes always disabled. | ||
|
|
||
| This commit fixes the issue by making the blkfront does parmeter caching | ||
| just before the advertisement. | ||
|
|
||
| Fixes: 402c43ea6b34 ("xen-blkfront: Apply 'feature_persistent' parameter when connect") | ||
| Cc: <stable@vger.kernel.org> # 5.10.x | ||
| Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> | ||
| Signed-off-by: SeongJae Park <sj@kernel.org> | ||
| --- | ||
| drivers/block/xen-blkfront.c | 14 +++++++------- | ||
| 1 file changed, 7 insertions(+), 7 deletions(-) | ||
|
|
||
| diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c | ||
| index dfae08115450..35b9bcad9db9 100644 | ||
| --- a/drivers/block/xen-blkfront.c | ||
| +++ b/drivers/block/xen-blkfront.c | ||
| @@ -1759,6 +1759,12 @@ static int write_per_ring_nodes(struct xenbus_transaction xbt, | ||
| return err; | ||
| } | ||
|
|
||
| +/* 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"); | ||
| + | ||
| /* Common code used when first setting up, and when resuming. */ | ||
| static int talk_to_blkback(struct xenbus_device *dev, | ||
| struct blkfront_info *info) | ||
| @@ -1850,6 +1856,7 @@ static int talk_to_blkback(struct xenbus_device *dev, | ||
| message = "writing protocol"; | ||
| goto abort_transaction; | ||
| } | ||
| + info->feature_persistent_parm = feature_persistent; | ||
| err = xenbus_printf(xbt, dev->nodename, "feature-persistent", "%u", | ||
| info->feature_persistent_parm); | ||
| if (err) | ||
| @@ -1919,12 +1926,6 @@ static int negotiate_mq(struct blkfront_info *info) | ||
| return 0; | ||
| } | ||
|
|
||
| -/* 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"); | ||
| - | ||
| /* | ||
| * Entry point to this code when a new device is created. Allocate the basic | ||
| * structures and the ring buffer for communication with the backend, and | ||
| @@ -2284,7 +2285,6 @@ static void blkfront_gather_backend_features(struct blkfront_info *info) | ||
| if (xenbus_read_unsigned(info->xbdev->otherend, "feature-discard", 0)) | ||
| blkfront_setup_discard(info); | ||
|
|
||
| - info->feature_persistent_parm = feature_persistent; | ||
| if (info->feature_persistent_parm) | ||
| info->feature_persistent = | ||
| !!xenbus_read_unsigned(info->xbdev->otherend, | ||
| -- | ||
| 2.35.3 | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters