Patchwork dm-crypt: never use write same (was Re: [v3.7 Regression] [SCSI] sd: Implement support for WRITE SAME)

login
register
mail settings
Submitter Milan Broz
Date Dec. 19, 2012, 9:07 p.m.
Message ID <50D22C7F.2020201@redhat.com>
Download mbox | patch
Permalink /patch/207803/
State New
Headers show

Comments

Milan Broz - Dec. 19, 2012, 9:07 p.m.
Does this help?

dm-crypt: never use write same

Ciphertext device is not compatible with WRITE SAME,
disable it for all dmcrypt devices.

Signed-off-by: Milan Broz <mbroz@redhat.com>
Milan Broz - Dec. 19, 2012, 10:23 p.m.
On 12/19/2012 11:20 PM, Joseph Salisbury wrote:
> Great work, Milan.  Your patch fixes the bug, stops the panic and allows 
> dm-crypt to function properly.

Thanks.

> 
> Will you be requesting this in v3.8 ?

This should go into 3.7 stable as well, I am talking with Alasdair already
how to handle it.

Milan
Martin K. Petersen - Dec. 20, 2012, 12:11 a.m.
>>>>> "Milan" == Milan Broz <mbroz@redhat.com> writes:

Milan> dm-crypt: never use write same

Milan> Ciphertext device is not compatible with WRITE SAME, disable it
Milan> for all dmcrypt devices.

Milan> Signed-off-by: Milan Broz <mbroz@redhat.com>

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Mike Snitzer - Dec. 20, 2012, 5:47 a.m.
On Wed, Dec 19 2012 at  7:11pm -0500,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> >>>>> "Milan" == Milan Broz <mbroz@redhat.com> writes:
> 
> Milan> dm-crypt: never use write same
> 
> Milan> Ciphertext device is not compatible with WRITE SAME, disable it
> Milan> for all dmcrypt devices.
> 
> Milan> Signed-off-by: Milan Broz <mbroz@redhat.com>
> 
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

I've developed more comprehensive WRITE SAME support for DM.  It can be
used in conjunction with Milan's patch (which enables Milan's patch to
go in too and be more easily tagged for v3.7 stable).

Anyway, my changes are available in the 'dm-write-same' branch of my
github tree: git://github.com/snitm/linux.git

See the top 5 commits: https://github.com/snitm/linux/commits/dm-write-same
(topmost needs to be folded, I'll do that when posting to dm-devel in
reply to this mail)
Joseph Salisbury - July 13, 2015, 4:33 p.m.
On 12/19/2012 05:20 PM, Joseph Salisbury wrote:
> On 12/19/2012 04:07 PM, Milan Broz wrote:
>> Does this help?
>>
>> dm-crypt: never use write same
>>
>> Ciphertext device is not compatible with WRITE SAME,
>> disable it for all dmcrypt devices.
>>
>> Signed-off-by: Milan Broz <mbroz@redhat.com>
>>
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -1844,6 +1844,12 @@ static int crypt_iterate_devices(struct
>> dm_target *ti,
>>       return fn(ti, cc->dev, cc->start, ti->len, data);
>>   }
>>   +static void crypt_io_hints(struct dm_target *ti,
>> +                struct queue_limits *limits)
>> +{
>> +    limits->max_write_same_sectors = 0;
>> +}
>> +
>>   static struct target_type crypt_target = {
>>       .name   = "crypt",
>>       .version = {1, 11, 0},
>> @@ -1858,6 +1864,7 @@ static struct target_type crypt_target = {
>>       .message = crypt_message,
>>       .merge  = crypt_merge,
>>       .iterate_devices = crypt_iterate_devices,
>> +    .io_hints = crypt_io_hints,
>>   };
>>     static int __init dm_crypt_init(void)
>>
> Great work, Milan.  Your patch fixes the bug, stops the panic and
> allows dm-crypt to function properly.
>
> Will you be requesting this in v3.8 ?
>
> Thanks again,
>
> Joe
>
Hi Milan,

The Ubuntu kernel has been carrying this patch since the discussion[0]
we were having about the bug.  I don't see that patch was ever included
in mainline.  Do you happen to know if this patch is still needed or was
the bug we were seeing fixed in some other way?

Thanks,

Joe 


[0] https://lists.ubuntu.com/archives/kernel-team/2012-December/023524.html
Milan Broz - July 13, 2015, 4:59 p.m.
On 07/13/2015 06:33 PM, Joseph Salisbury wrote:
> Hi Milan,
> 
> The Ubuntu kernel has been carrying this patch since the discussion[0]
> we were having about the bug.  I don't see that patch was ever included
> in mainline.  Do you happen to know if this patch is still needed or was
> the bug we were seeing fixed in some other way?

I think it was superseded by later Mike's approach to reverse the logic
- disable write same for all targets and require to explicitly allow it.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-table.c?id=c1a94672a830e01d58c7c7e8de530c3f136d6ff2
(+later patches)

So I think we do no need this patch upstream anymore.

Mike, am I right here?

Milan
Milan Broz - July 13, 2015, 5:01 p.m.
(sorry, resending again, not sure if it was sent correctly)

On 07/13/2015 06:33 PM, Joseph Salisbury wrote:
> Hi Milan,
> 
> The Ubuntu kernel has been carrying this patch since the discussion[0]
> we were having about the bug.  I don't see that patch was ever included
> in mainline.  Do you happen to know if this patch is still needed or was
> the bug we were seeing fixed in some other way?

I think it was superseded by later Mike's approach to reverse the logic
- disable write same for all targets and require to explicitly allow it.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-table.c?id=c1a94672a830e01d58c7c7e8de530c3f136d6ff2
(+later patches)

So I think we do no need this patch upstream anymore.

Mike, am I right here?

Milan
Mike Snitzer - July 13, 2015, 6:01 p.m.
On Mon, Jul 13 2015 at  1:01pm -0400,
Milan Broz <mbroz@redhat.com> wrote:

> (sorry, resending again, not sure if it was sent correctly)
> 
> On 07/13/2015 06:33 PM, Joseph Salisbury wrote:
> > Hi Milan,
> > 
> > The Ubuntu kernel has been carrying this patch since the discussion[0]
> > we were having about the bug.  I don't see that patch was ever included
> > in mainline.  Do you happen to know if this patch is still needed or was
> > the bug we were seeing fixed in some other way?
> 
> I think it was superseded by later Mike's approach to reverse the logic
> - disable write same for all targets and require to explicitly allow it.
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-table.c?id=c1a94672a830e01d58c7c7e8de530c3f136d6ff2
> (+later patches)
> 
> So I think we do no need this patch upstream anymore.
> 
> Mike, am I right here?

Yeah, a DM target needs to opt-in by setting ti->num_write_same_bios
(only linear, stripe and mpath do at this time).

Patch

--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1844,6 +1844,12 @@  static int crypt_iterate_devices(struct dm_target *ti,
 	return fn(ti, cc->dev, cc->start, ti->len, data);
 }
 
+static void crypt_io_hints(struct dm_target *ti,
+			    struct queue_limits *limits)
+{
+	limits->max_write_same_sectors = 0;
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
 	.version = {1, 11, 0},
@@ -1858,6 +1864,7 @@  static struct target_type crypt_target = {
 	.message = crypt_message,
 	.merge  = crypt_merge,
 	.iterate_devices = crypt_iterate_devices,
+	.io_hints = crypt_io_hints,
 };
 
 static int __init dm_crypt_init(void)