NACK/Cmnt: [SRU][B][F][G][PATCH 0/1] dm crypt: add flags to optionally

Tue Jun 29 10:01:51 UTC 2021

Understood, thank you Stefan

On Tue, Jun 29, 2021 at 3:26 PM Stefan Bader <stefan.bader at canonical.com>
wrote:

> On 28.06.21 11:37, Gerald Yang wrote:
> > Hi Stefan,
> >
> > I checked and also tried to backport these commits:
> > git log --oneline --grep "Fixes: 39d42fa96ba1"
> > c87a95dc28b1 dm crypt: defer decryption to a tasklet if interrupts
> disabled
> > 8e14f610159d dm crypt: do not call bio_endio() from the dm-crypt tasklet
> > d68b29584c25 dm crypt: use GFP_ATOMIC when allocating crypto requests
> from softirq
> > 8abec36d1274 dm crypt: do not wait for backlogged crypto request
> completion in
> > softirq
> > 4a5caa4af0df dm crypt: document new no_workqueue flags
> >
> > But it failed to build the kernel, I found there are more dependencies
> need to
> > be backported
> > Take focal kernel (5.4) for instance, the build error is:
> > /home/ubuntu/focal/drivers/md/dm-crypt.c:2768:8: error:
> ‘dm_report_zones_cb’
> > undeclared (first use in this function); did you mean
> ‘dm_report_zones_fn’?
> > 2768 | dm_report_zones_cb, args);
> >
> > This function "dm_report_zones_cb" was introduced in this commit in 5.5
> kernel:
> > git log -L :dm_report_zones_cb:drivers/md/dm.c
> > commit d41003513e61dd9d4974cb441d30b63650b85654
> > git describe --contains d41003513e61dd9d4974cb441d30b63650b85654
> > v5.5-rc1~199^2~1
> >
> > It reworked "zone reporting"
> > d41003513e61 block: rework zone reporting
> >
> > And modify some other files in block layer
> > drivers/md/dm-flakey.c
> > drivers/md/dm-linear.c
> > drivers/scsi/sd.h
> > fs/f2fs/super.c
> > include/linux/device-mapper.h
> > block/blk-zoned.c
> > drivers/block/null_blk.h
> > drivers/block/null_blk_zoned.c
> > drivers/md/dm-zoned-metadata.c
> > drivers/md/dm.c
> > drivers/scsi/sd_zbc.c
> > include/linux/blkdev.h
> >
> > I think it could get more dependencies involved for backporting this
> commit
> >
> > And the original commit I wanted to backport
> > (39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877)
> > changes dm-crypt version from 1.19.0 to 1.22.0 for focal kernel (5.4)
> >
> > Because this is a request from our customer, I would like to ask for
> advice if
> > it's still proper to backport this one and all related commits?
>
> The amount of follow-up changes which were done upstream to a seemingly
> simple
> functional tweak suggest this change had more complications than even
> upstream
> anticipated. At some point we have to make a decision and for LTS release
> kernels this would be rather in favour of stability over feature
> backports. Even
> more so the older a release is (I would acutally never consider feature
> changes
> to Bionic anymore). Groovy nears its end of life and I assume Hirsute
> already
> has this. And somehow the errors of the Focal 5.4 backport sound like a
> lot of
> generic device-mapper infrastructure would have to be backported and that
> would
> put the stability of the kernel at risk.
> So I would rather point that customer at the hwe-5.11 kernel which is
> about to
> become the default hwe kernel for Focal.
>
> -Stefan
>
> > or it's better to wait for a newer hwe kernel that has this commit
> included?
> >
> > Thanks,
> > Gerald
> >
> > On Wed, May 19, 2021 at 3:26 PM Gerald Yang <gerald.yang at canonical.com
> > <mailto:gerald.yang at canonical.com>> wrote:
> >
> >     Thanks Stefan, I will update the SRU with all related commits
> >
> >     On Tue, May 18, 2021 at 2:42 PM Stefan Bader <
> stefan.bader at canonical.com
> >     <mailto:stefan.bader at canonical.com>> wrote:
> >
> >         On 18.05.21 04:56, Gerald Yang wrote:
> >          > BugLink:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976
> >         <https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976>
> >
> >         Above should be https://bugs.launchpad.net/bugs/1910976
> >         <https://bugs.launchpad.net/bugs/1910976> but that could be done
> >         when applying. However there seem to be multiple references to
> this patch
> >         following its introduction upstream (see below). That does not
> sound
> >         like just
> >         picking this one patch is a good idea.
> >
> >         -Stefan
> >
> >          >
> >          > === SRU Justification ===
> >          > [Impact]
> >          > To get better performance for dm-crypt in some cases, bypass
> kcryptd
> >          > workqueue can reduce the overhead in context switch between
> kworkers,
> >          > cherry-pick commit 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877
> from mainline
> >          > kernel, and bypass kcryptd workqueue is not enabled by default
> >          >
> >          > [Fix]
> >          > Add flags to bypass kcryptd workqueue
> >          >
> >          > [Test]
> >          > create dm-crypt and setup DM_CRYPT_NO_READ_WORKQUEUE and
> >          > DM_CRYPT_NO_WRITE_WORKQUEUE, read/write data and run perf
> record to see
> >          > if read/write to the encrypted device will bypass kcryptd
> workqueue
> >          >
> >          > [Regression Potential]
> >          > Low, this feature is disabled by default, need to enable
> manually
> >          >
> >          > Ignat Korchagin (1):
> >          >    dm crypt: add flags to optionally bypass kcryptd workqueues
> >          >
> >          >   drivers/md/dm-crypt.c | 50
> ++++++++++++++++++++++++++++++++++++-------
> >          >   1 file changed, 42 insertions(+), 8 deletions(-)
> >          >
> >
> >           > git log --oneline --grep "Fixes: 39d42fa96ba1"
> >         c87a95dc28b1 dm crypt: defer decryption to a tasklet if
> interrupts disabled
> >         8e14f610159d dm crypt: do not call bio_endio() from the dm-crypt
> tasklet
> >         d68b29584c25 dm crypt: use GFP_ATOMIC when allocating crypto
> requests
> >         from softirq
> >         8abec36d1274 dm crypt: do not wait for backlogged crypto request
> >         completion in
> >         softirq
> >         4a5caa4af0df dm crypt: document new no_workqueue flags
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210629/a52858de/attachment-0001.html>