[PATCH v3 0/2] Fix persistent grants negotiation with a behavior change

SeongJae Park posted 2 patches 1 year, 9 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
Documentation/ABI/testing/sysfs-driver-xen-blkback  | 2 +-
Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +-
drivers/block/xen-blkback/xenbus.c                  | 9 +++------
drivers/block/xen-blkfront.c                        | 4 +---
4 files changed, 6 insertions(+), 11 deletions(-)
[PATCH v3 0/2] Fix persistent grants negotiation with a behavior change
Posted by SeongJae Park 1 year, 9 months ago
The first patch of this patchset fixes 'feature_persistent' parameter
handling in 'blkback' to respect the frontend's persistent grants
support always.  The fix makes a behavioral change, so the second patch
makes the counterpart of 'blkfront' to consistently follow the behavior
change.

Changes from v2
(https://lore.kernel.org/xen-devel/20220714224410.51147-1-sj@kernel.org/)
- Keep the behavioral change of v1
- Update blkfront's counterpart to follow the changed behavior
- Update documents for the changed behavior

Changes from v1
(https://lore.kernel.org/xen-devel/20220106091013.126076-1-mheyne@amazon.de/)
- Avoid the behavioral change
  (https://lore.kernel.org/xen-devel/20220121102309.27802-1-sj@kernel.org/)
- Rebase on latest xen/tip/linux-next
- Re-work by SeongJae Park <sj@kernel.org>
- Cc stable@



Maximilian Heyne (1):
  xen, blkback: fix persistent grants negotiation

SeongJae Park (1):
  xen-blkfront: Apply 'feature_persistent' parameter when connect

 Documentation/ABI/testing/sysfs-driver-xen-blkback  | 2 +-
 Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +-
 drivers/block/xen-blkback/xenbus.c                  | 9 +++------
 drivers/block/xen-blkfront.c                        | 4 +---
 4 files changed, 6 insertions(+), 11 deletions(-)

-- 
2.25.1
Re: [PATCH v3 0/2] Fix persistent grants negotiation with a behavior change
Posted by SeongJae Park 1 year, 9 months ago
Hi all,

On Fri, 15 Jul 2022 17:55:19 +0000 SeongJae Park <sj@kernel.org> wrote:

> The first patch of this patchset fixes 'feature_persistent' parameter
> handling in 'blkback' to respect the frontend's persistent grants
> support always.  The fix makes a behavioral change, so the second patch
> makes the counterpart of 'blkfront' to consistently follow the behavior
> change.

I made the behavior change as requested by Andrii[1].  I therefore made similar
behavior change to blkfront and Cc-ed stable for the second change, too.

To make the change history clear and reduce the stable side overhead, however,
it might be better to apply the v2, which don't make behavior change but only
fix the issue, Cc stable@ for it, make the behavior change commits for both
blkback and blkfront, update the documents, and don't Cc stable@ for the
behavior change and documents update commits.

One downside of that would be that it will make a behavioral difference in
pre-5.19.x and post-5.19.x.

I think both downsides are not critical, so posted this patchset in this shape.
If anyone prefer some changes, please let me know, though.

[1] https://lore.kernel.org/xen-devel/CAJwUmVB6H3iTs-C+U=v-pwJB7-_ZRHPxHzKRJZ22xEPW7z8a=g@mail.gmail.com/


Thanks,
SJ

> 
> Changes from v2
> (https://lore.kernel.org/xen-devel/20220714224410.51147-1-sj@kernel.org/)
> - Keep the behavioral change of v1
> - Update blkfront's counterpart to follow the changed behavior
> - Update documents for the changed behavior
> 
> Changes from v1
> (https://lore.kernel.org/xen-devel/20220106091013.126076-1-mheyne@amazon.de/)
> - Avoid the behavioral change
>   (https://lore.kernel.org/xen-devel/20220121102309.27802-1-sj@kernel.org/)
> - Rebase on latest xen/tip/linux-next
> - Re-work by SeongJae Park <sj@kernel.org>
> - Cc stable@
> 
> 
> 
> Maximilian Heyne (1):
>   xen, blkback: fix persistent grants negotiation
> 
> SeongJae Park (1):
>   xen-blkfront: Apply 'feature_persistent' parameter when connect
> 
>  Documentation/ABI/testing/sysfs-driver-xen-blkback  | 2 +-
>  Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +-
>  drivers/block/xen-blkback/xenbus.c                  | 9 +++------
>  drivers/block/xen-blkfront.c                        | 4 +---
>  4 files changed, 6 insertions(+), 11 deletions(-)
> 
> -- 
> 2.25.1
Re: [PATCH v3 0/2] Fix persistent grants negotiation with a behavior change
Posted by SeongJae Park 1 year, 9 months ago
Hi all,

On Fri, 15 Jul 2022 18:12:26 +0000 SeongJae Park <sj@kernel.org> wrote:

> Hi all,
> 
> On Fri, 15 Jul 2022 17:55:19 +0000 SeongJae Park <sj@kernel.org> wrote:
> 
> > The first patch of this patchset fixes 'feature_persistent' parameter
> > handling in 'blkback' to respect the frontend's persistent grants
> > support always.  The fix makes a behavioral change, so the second patch
> > makes the counterpart of 'blkfront' to consistently follow the behavior
> > change.
> 
> I made the behavior change as requested by Andrii[1].  I therefore made similar
> behavior change to blkfront and Cc-ed stable for the second change, too.

Now I realize that commit aac8a70db24b ("xen-blkback: add a parameter for
disabling of persistent grants") introduced two issues.  One is what Max
reported with his patch, and the second one is unintended behavioral change
that broke Andrii's use case.

That is, Andrii's use case should had no problem at all before the introduction
of 'feature_persistent', as at that time 'blkback' checked if the frontend
support the persistent grants for every 'reconnect()' and enables it if so.
However, introduction of the parameter made it behaves differently.

Yes, we intended to make the prameter to make effects to newly created devices.
But, as it breaks user workflows, this should be fixed.  Same for the
'blkfront' side 'feature_persistent'.

> 
> To make the change history clear and reduce the stable side overhead, however,
> it might be better to apply the v2, which don't make behavior change but only
> fix the issue, Cc stable@ for it, make the behavior change commits for both
> blkback and blkfront, update the documents, and don't Cc stable@ for the
> behavior change and documents update commits.

I'd say having one patch for each issue would be the right way to go, and all
fixes should Cc stable@.

> 
> One downside of that would be that it will make a behavioral difference in
> pre-5.19.x and post-5.19.x.

The unintended behavioral fix should also be considered fix and therefore
should be merged into stable@, so above concern is not valid.

I will send the next spin soon.


Thanks,
SJ

[...]