All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>
Cc: Mike Christie <michael.christie@oracle.com>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	"ssudhakarp@gmail.com" <ssudhakarp@gmail.com>,
	"dm-crypt@saout.de" <dm-crypt@saout.de>,
	Eric Biggers <ebiggers@kernel.org>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Shirley Ma <shirley.ma@oracle.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	Milan Broz <gmazyland@gmail.com>,
	"agk@redhat.com" <agk@redhat.com>,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	mst@redhat.com
Subject: Re: [dm-crypt] [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices
Date: Fri, 25 Sep 2020 16:15:12 -0400	[thread overview]
Message-ID: <20200925201512.GA6025@redhat.com> (raw)
In-Reply-To: <MWHPR04MB37588DF8C3FFF4BD0C3CD543E7360@MWHPR04MB3758.namprd04.prod.outlook.com>

On Thu, Sep 24 2020 at  9:09pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> On 2020/09/25 4:14, Sudhakar Panneerselvam wrote:
> >>
> >> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote:
> >>
> >>>> By copying it to a temporary aligned buffer and issuing I/O on this
> >>>> buffer.
> >>>
> >>> I don't like this idea. Because, you need to allocate additional pages
> >>> for the entire I/O size(for the misaligned case, if you think through
> >>
> >> You can break the I/O to smaller pieces. You can use mempool for
> >> pre-allocation of the pages.
> > 
> > Assuming we do this, how is this code simpler(based on your
> > comment below) than the fix in dm-crypt? In fact, this approach 
> > would make the code change look bad in vhost, at the same time
> > having performance penalty. By doing this, we are just moving the 
> > responsibility to other unrelated component.
> 
> Because vhost is at the top of the block-io food chain. Fixing the unaligned
> segments there will ensure that it does not matter what device is under it. It
> will work.

Right, I agree. This should be addressed in vhost-scsi.  And vhost-scsi
probably needs to be interfacing through block core to submit IO that
respects the limits of its underlying block device.

So please lift your proposed dm-crypt changes to vhost-scsi:
https://patchwork.kernel.org/patch/11781207/
https://patchwork.kernel.org/patch/11781053/

Maybe work with vhost-scsi maintainers to see about making the code
reusable in block core; so that any future unaligned application IO is
dealt in other drivers using the same common code.

But I'm not interested in taking these changes into dm-crypt:

NAK

> I am still baffled that the unaligned segments go through in the first place...
> Do we have something missing in the BIO code ?

Cc'ing linux-block, could be.

Thanks,
Mike

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>
Cc: linux-block@vger.kernel.org,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	"ssudhakarp@gmail.com" <ssudhakarp@gmail.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	mst@redhat.com, "dm-crypt@saout.de" <dm-crypt@saout.de>,
	Eric Biggers <ebiggers@kernel.org>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	"agk@redhat.com" <agk@redhat.com>,
	linux-scsi@vger.kernel.org, Shirley Ma <shirley.ma@oracle.com>,
	Milan Broz <gmazyland@gmail.com>,
	Mike Christie <michael.christie@oracle.com>
Subject: Re: [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices
Date: Fri, 25 Sep 2020 16:15:12 -0400	[thread overview]
Message-ID: <20200925201512.GA6025@redhat.com> (raw)
In-Reply-To: <MWHPR04MB37588DF8C3FFF4BD0C3CD543E7360@MWHPR04MB3758.namprd04.prod.outlook.com>

On Thu, Sep 24 2020 at  9:09pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> On 2020/09/25 4:14, Sudhakar Panneerselvam wrote:
> >>
> >> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote:
> >>
> >>>> By copying it to a temporary aligned buffer and issuing I/O on this
> >>>> buffer.
> >>>
> >>> I don't like this idea. Because, you need to allocate additional pages
> >>> for the entire I/O size(for the misaligned case, if you think through
> >>
> >> You can break the I/O to smaller pieces. You can use mempool for
> >> pre-allocation of the pages.
> > 
> > Assuming we do this, how is this code simpler(based on your
> > comment below) than the fix in dm-crypt? In fact, this approach 
> > would make the code change look bad in vhost, at the same time
> > having performance penalty. By doing this, we are just moving the 
> > responsibility to other unrelated component.
> 
> Because vhost is at the top of the block-io food chain. Fixing the unaligned
> segments there will ensure that it does not matter what device is under it. It
> will work.

Right, I agree. This should be addressed in vhost-scsi.  And vhost-scsi
probably needs to be interfacing through block core to submit IO that
respects the limits of its underlying block device.

So please lift your proposed dm-crypt changes to vhost-scsi:
https://patchwork.kernel.org/patch/11781207/
https://patchwork.kernel.org/patch/11781053/

Maybe work with vhost-scsi maintainers to see about making the code
reusable in block core; so that any future unaligned application IO is
dealt in other drivers using the same common code.

But I'm not interested in taking these changes into dm-crypt:

NAK

> I am still baffled that the unaligned segments go through in the first place...
> Do we have something missing in the BIO code ?

Cc'ing linux-block, could be.

Thanks,
Mike

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>
Cc: Mike Christie <michael.christie@oracle.com>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	"ssudhakarp@gmail.com" <ssudhakarp@gmail.com>,
	"dm-crypt@saout.de" <dm-crypt@saout.de>,
	Eric Biggers <ebiggers@kernel.org>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Shirley Ma <shirley.ma@oracle.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	Milan Broz <gmazyland@gmail.com>,
	"agk@redhat.com" <agk@redhat.com>,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	mst@redhat.com
Subject: Re: [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices
Date: Fri, 25 Sep 2020 16:15:12 -0400	[thread overview]
Message-ID: <20200925201512.GA6025@redhat.com> (raw)
In-Reply-To: <MWHPR04MB37588DF8C3FFF4BD0C3CD543E7360@MWHPR04MB3758.namprd04.prod.outlook.com>

On Thu, Sep 24 2020 at  9:09pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> On 2020/09/25 4:14, Sudhakar Panneerselvam wrote:
> >>
> >> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote:
> >>
> >>>> By copying it to a temporary aligned buffer and issuing I/O on this
> >>>> buffer.
> >>>
> >>> I don't like this idea. Because, you need to allocate additional pages
> >>> for the entire I/O size(for the misaligned case, if you think through
> >>
> >> You can break the I/O to smaller pieces. You can use mempool for
> >> pre-allocation of the pages.
> > 
> > Assuming we do this, how is this code simpler(based on your
> > comment below) than the fix in dm-crypt? In fact, this approach 
> > would make the code change look bad in vhost, at the same time
> > having performance penalty. By doing this, we are just moving the 
> > responsibility to other unrelated component.
> 
> Because vhost is at the top of the block-io food chain. Fixing the unaligned
> segments there will ensure that it does not matter what device is under it. It
> will work.

Right, I agree. This should be addressed in vhost-scsi.  And vhost-scsi
probably needs to be interfacing through block core to submit IO that
respects the limits of its underlying block device.

So please lift your proposed dm-crypt changes to vhost-scsi:
https://patchwork.kernel.org/patch/11781207/
https://patchwork.kernel.org/patch/11781053/

Maybe work with vhost-scsi maintainers to see about making the code
reusable in block core; so that any future unaligned application IO is
dealt in other drivers using the same common code.

But I'm not interested in taking these changes into dm-crypt:

NAK

> I am still baffled that the unaligned segments go through in the first place...
> Do we have something missing in the BIO code ?

Cc'ing linux-block, could be.

Thanks,
Mike


  reply	other threads:[~2020-09-25 20:21 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16 18:40 [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices Sudhakar Panneerselvam
2020-09-16 18:40 ` [RFC PATCH 1/2] dm crypt: Allow unaligned bio " Sudhakar Panneerselvam
2020-09-16 18:40 ` [RFC PATCH 2/2] dm crypt: Handle unaligned bio buffer lengths for lmk and tcw Sudhakar Panneerselvam
2020-09-23 17:01 ` [dm-crypt] [dm-devel] [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices Sudhakar Panneerselvam
2020-09-23 17:01   ` Sudhakar Panneerselvam
2020-09-24  1:27   ` [dm-crypt] " Mike Snitzer
2020-09-24  1:27     ` Mike Snitzer
2020-09-24  5:14     ` [dm-crypt] [dm-devel] " Eric Biggers
2020-09-24  5:14       ` Eric Biggers
2020-09-24  8:15       ` [dm-crypt] [dm-devel] " Milan Broz
2020-09-24  8:15         ` Milan Broz
2020-09-24 16:55         ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 16:55           ` Sudhakar Panneerselvam
2020-09-24 16:44       ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 16:44         ` Sudhakar Panneerselvam
2020-09-24 17:26         ` [dm-crypt] [dm-devel] " Mikulas Patocka
2020-09-24 17:26           ` Mikulas Patocka
2020-09-24 17:38           ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 17:38             ` Sudhakar Panneerselvam
2020-09-24 17:50             ` [dm-crypt] [dm-devel] " Mikulas Patocka
2020-09-24 17:50               ` Mikulas Patocka
2020-09-24 18:11               ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 18:11                 ` Sudhakar Panneerselvam
2020-09-24 18:44                 ` [dm-crypt] [dm-devel] " Mikulas Patocka
2020-09-24 18:44                   ` Mikulas Patocka
2020-09-24 19:13                   ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 19:13                     ` Sudhakar Panneerselvam
2020-09-25  1:09                     ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-09-25  1:09                       ` Damien Le Moal
2020-09-25 20:15                       ` Mike Snitzer [this message]
2020-09-25 20:15                         ` Mike Snitzer
2020-09-25 20:15                         ` Mike Snitzer
2020-09-24 12:47     ` [dm-crypt] " Mikulas Patocka
2020-09-24 12:47       ` Mikulas Patocka
2020-09-24 15:58     ` [dm-crypt] " Sudhakar Panneerselvam
2020-09-24 15:58       ` Sudhakar Panneerselvam
2020-09-24 12:40   ` [dm-crypt] [dm-devel] " Mikulas Patocka
2020-09-24 12:40     ` Mikulas Patocka
2020-09-24 17:12     ` [dm-crypt] [dm-devel] " Sudhakar Panneerselvam
2020-09-24 17:12       ` Sudhakar Panneerselvam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200925201512.GA6025@redhat.com \
    --to=snitzer@redhat.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=agk@redhat.com \
    --cc=dm-crypt@saout.de \
    --cc=dm-devel@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=gmazyland@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=michael.christie@oracle.com \
    --cc=mpatocka@redhat.com \
    --cc=mst@redhat.com \
    --cc=shirley.ma@oracle.com \
    --cc=ssudhakarp@gmail.com \
    --cc=sudhakar.panneerselvam@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.