Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251928 > unrolled thread

Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE

Started byGrant Grundler <grundler@chromium.org>
First post2015-10-20 20:00 +0200
Last post2015-10-21 19:40 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Grant Grundler <grundler@chromium.org> - 2015-10-20 20:00 +0200
    Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Jeff Moyer <jmoyer@redhat.com> - 2015-10-20 21:00 +0200
      Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Ulf Hansson <ulf.hansson@linaro.org> - 2015-10-21 11:10 +0200
        Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Grant Grundler <grundler@chromium.org> - 2015-10-21 19:10 +0200
        Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Jeff Moyer <jmoyer@redhat.com> - 2015-10-28 23:20 +0100
      Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE Grant Grundler <grundler@chromium.org> - 2015-10-21 19:40 +0200

#1251928 — Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE

FromGrant Grundler <grundler@chromium.org>
Date2015-10-20 20:00 +0200
SubjectRe: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE
Message-ID<qlJmy-2y3-15@gated-at.bofh.it>
Ping? Does no one care how long BLK_SECDISCARD takes?

ChromeOS has landed this change as a compromise between "fast" (<10
seconds) and "minimize risk" (~90 seconds) for a 23GB partition on
eMMC:
    https://chromium-review.googlesource.com/#/c/302413/

This is a generic problem if we care about data privacy since
consumers won't expect a "secure erase" operation to take 1/2h or more
and think the device is hung.

cheers,
grant

On Mon, Sep 28, 2015 at 2:45 PM, Grant Grundler <grundler@chromium.org> wrote:
> [resending...I forgot to switch gmail back to text-only mode. grrrh..]
>
> ---------- Forwarded message ----------
> From: Grant Grundler <grundler@chromium.org>
> Date: Mon, Sep 28, 2015 at 2:42 PM
> Subject: Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE
> To: Grant Grundler <grundler@chromium.org>
> Cc: Jens Axboe <axboe@kernel.dk>, Ulf Hansson
> <ulf.hansson@linaro.org>, LKML <linux-kernel@vger.kernel.org>,
> "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
>
>
> On Thu, Sep 24, 2015 at 10:39 AM, Grant Grundler <grundler@chromium.org> wrote:
>>
>> Some followup.
> ...
>>
>> 2) I've been able to test this hack on an eMMC device:
>> [   13.147747] mmc..._secdiscard_rq(mmc1) ERASE from 14116864 cnt
>> 0x2c00000 (size 22528 MiB)
>> [   13.155964] sdhci cmd: 35/0x1a arg 0xd76800
>> [   13.160266] sdhci cmd: 36/0x1a arg 0x39767ff
>> [   13.164593] sdhci cmd: 38/0x1b arg 0x80000000
>> [   13.803360] random: nonblocking pool is initialized
>> [   14.567735] sdhci cmd: 13/0x1a arg 0x10000
>> [   14.573324] mmc..._secdiscard_rq(mmc1) err 0
>>
>> This was with ~15K files and about 5GB written to the device. 1.4
>> seconds compared to about 20 minutes to secure erase the same region
>> with original v3.18 code.
>
>
> To put a few more numbers on the "chunk size vs perf":
>  1EG (512KB) -> 44K commands -> ~20 minutes
> 32EG (16MB) -> 1375 commands -> ~1 minute
> 128EG (64MB) -> 344 commands -> ~30 seconds
> 8191EG (~4GB) -> 6 commands -> 2 seconds + ~8 seconds mkfs
> (I'm assuming times above include about 6-10 seconds of mkfs as part
> of writing a new file system)
>
> This is with only ~300MB of data written to the partition. I'm fully
> aware that times will vary depending on how much data needs to be
> migrated (and in this case very little or none). I'm certain the
> difference will only get worse for the smaller the "chunk size" used
> to Secure Erase due to repeated data migration.
>
> Given the different use model for secure erase (legal/contractually
> required behavior), is using 4GB chunk size acceptable?
>
> Would anyone be terribly offended if I used the recently added
> "MMC_IOC_MULTI_CMD" to send the cmd 35/36/38 sequence to the eMMC
> device to securely erase the offending partition?
>
> thanks,
> grant
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1251960

FromJeff Moyer <jmoyer@redhat.com>
Date2015-10-20 21:00 +0200
Message-ID<qlKiC-3V5-3@gated-at.bofh.it>
In reply to#1251928
Hi Grant,

Grant Grundler <grundler@chromium.org> writes:

> Ping? Does no one care how long BLK_SECDISCARD takes?
>
> ChromeOS has landed this change as a compromise between "fast" (<10
> seconds) and "minimize risk" (~90 seconds) for a 23GB partition on
> eMMC:
>     https://chromium-review.googlesource.com/#/c/302413/

Including the patch would be helpful.  I believe this is it.  My
comments are inline.

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 8411be3..43943c7 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c

@@ -60,21 +60,37 @@
 	granularity = max(q->limits.discard_granularity >> 9, 1U);
 	alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
 
-	/*
-	 * Ensure that max_discard_sectors is of the proper
-	 * granularity, so that requests stay aligned after a split.
-	 */
-	max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
-	max_discard_sectors -= max_discard_sectors % granularity;
-	if (unlikely(!max_discard_sectors)) {
-		/* Avoid infinite loop below. Being cautious never hurts. */
-		return -EOPNOTSUPP;
-	}
+	max_discard_sectors = min(q->limits.max_discard_sectors,
+						UINT_MAX >> 9);

Unnecessary reformatting.
 
 	if (flags & BLKDEV_DISCARD_SECURE) {
 		if (!blk_queue_secdiscard(q))
 			return -EOPNOTSUPP;
 		type |= REQ_SECURE;
+		/*
+		 * Secure erase performs better by telling the device
+		 * about the largest range possible.  Secure erase
+		 * piecemeal will likely result in mapped sectors
+		 * getting evacuated from one range and parked in
+		 * another range that will get erased by a future
+		 * erase command.  This does NOT happen for normal
+		 * TRIM or DISCARD operations.
+		 *
+		 * 32GB was a compromise to avoid blocking the device
+		 * for potentially minute(s) at a time.
+		 */
+		if (max_discard_sectors < (1 << (25-9)))	/* 32GiB */
+			max_discard_sectors = 1 << (25-9);

And here you're ignoring q->limits.max_discard_sectors.  I'm surprised
this worked!

+	}
+
+	/*
+	 * Ensure that max_discard_sectors is of the proper
+	 * granularity, so that requests stay aligned after a split.
+	 */
+	max_discard_sectors -= max_discard_sectors % granularity;
+	if (unlikely(!max_discard_sectors)) {
+		/* Avoid infinite loop below. Being cautious never hurts. */
+		return -EOPNOTSUPP;
 	}
 
 	atomic_set(&bb.done, 1);

Grant, can we start over with the problem description? (Sorry, I didn't
see the previous posts.)  I'd like to know the values of discard_granularity
and discard_max_bytes for your device.  Additionally, it would be
interesting to know how the discards are being initiatied.  Is it via a
userspace utility such as mkfs, online discard via some file system
mounted with -o discard, or something else?  Finally, can you post
binary blktrace data somewhere for the slow case?

Thanks!
Jeff




> On Mon, Sep 28, 2015 at 2:45 PM, Grant Grundler <grundler@chromium.org> wrote:
>> [resending...I forgot to switch gmail back to text-only mode. grrrh..]
>>
>> ---------- Forwarded message ----------
>> From: Grant Grundler <grundler@chromium.org>
>> Date: Mon, Sep 28, 2015 at 2:42 PM
>> Subject: Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE
>> To: Grant Grundler <grundler@chromium.org>
>> Cc: Jens Axboe <axboe@kernel.dk>, Ulf Hansson
>> <ulf.hansson@linaro.org>, LKML <linux-kernel@vger.kernel.org>,
>> "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
>>
>>
>> On Thu, Sep 24, 2015 at 10:39 AM, Grant Grundler <grundler@chromium.org> wrote:
>>>
>>> Some followup.
>> ...
>>>
>>> 2) I've been able to test this hack on an eMMC device:
>>> [   13.147747] mmc..._secdiscard_rq(mmc1) ERASE from 14116864 cnt
>>> 0x2c00000 (size 22528 MiB)
>>> [   13.155964] sdhci cmd: 35/0x1a arg 0xd76800
>>> [   13.160266] sdhci cmd: 36/0x1a arg 0x39767ff
>>> [   13.164593] sdhci cmd: 38/0x1b arg 0x80000000
>>> [   13.803360] random: nonblocking pool is initialized
>>> [   14.567735] sdhci cmd: 13/0x1a arg 0x10000
>>> [   14.573324] mmc..._secdiscard_rq(mmc1) err 0
>>>
>>> This was with ~15K files and about 5GB written to the device. 1.4
>>> seconds compared to about 20 minutes to secure erase the same region
>>> with original v3.18 code.
>>
>>
>> To put a few more numbers on the "chunk size vs perf":
>>  1EG (512KB) -> 44K commands -> ~20 minutes
>> 32EG (16MB) -> 1375 commands -> ~1 minute
>> 128EG (64MB) -> 344 commands -> ~30 seconds
>> 8191EG (~4GB) -> 6 commands -> 2 seconds + ~8 seconds mkfs
>> (I'm assuming times above include about 6-10 seconds of mkfs as part
>> of writing a new file system)
>>
>> This is with only ~300MB of data written to the partition. I'm fully
>> aware that times will vary depending on how much data needs to be
>> migrated (and in this case very little or none). I'm certain the
>> difference will only get worse for the smaller the "chunk size" used
>> to Secure Erase due to repeated data migration.
>>
>> Given the different use model for secure erase (legal/contractually
>> required behavior), is using 4GB chunk size acceptable?
>>
>> Would anyone be terribly offended if I used the recently added
>> "MMC_IOC_MULTI_CMD" to send the cmd 35/36/38 sequence to the eMMC
>> device to securely erase the offending partition?
>>
>> thanks,
>> grant
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252596

FromUlf Hansson <ulf.hansson@linaro.org>
Date2015-10-21 11:10 +0200
Message-ID<qlXzc-6P0-23@gated-at.bofh.it>
In reply to#1251960
On 20 October 2015 at 20:57, Jeff Moyer <jmoyer@redhat.com> wrote:
> Hi Grant,
>
> Grant Grundler <grundler@chromium.org> writes:
>
>> Ping? Does no one care how long BLK_SECDISCARD takes?
>>
>> ChromeOS has landed this change as a compromise between "fast" (<10
>> seconds) and "minimize risk" (~90 seconds) for a 23GB partition on
>> eMMC:
>>     https://chromium-review.googlesource.com/#/c/302413/
>
> Including the patch would be helpful.  I believe this is it.  My
> comments are inline.
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 8411be3..43943c7 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
>
> @@ -60,21 +60,37 @@
>         granularity = max(q->limits.discard_granularity >> 9, 1U);
>         alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
>
> -       /*
> -        * Ensure that max_discard_sectors is of the proper
> -        * granularity, so that requests stay aligned after a split.
> -        */
> -       max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> -       max_discard_sectors -= max_discard_sectors % granularity;
> -       if (unlikely(!max_discard_sectors)) {
> -               /* Avoid infinite loop below. Being cautious never hurts. */
> -               return -EOPNOTSUPP;
> -       }
> +       max_discard_sectors = min(q->limits.max_discard_sectors,
> +                                               UINT_MAX >> 9);
>
> Unnecessary reformatting.
>
>         if (flags & BLKDEV_DISCARD_SECURE) {
>                 if (!blk_queue_secdiscard(q))
>                         return -EOPNOTSUPP;
>                 type |= REQ_SECURE;
> +               /*
> +                * Secure erase performs better by telling the device
> +                * about the largest range possible.  Secure erase
> +                * piecemeal will likely result in mapped sectors
> +                * getting evacuated from one range and parked in
> +                * another range that will get erased by a future
> +                * erase command.  This does NOT happen for normal
> +                * TRIM or DISCARD operations.
> +                *
> +                * 32GB was a compromise to avoid blocking the device
> +                * for potentially minute(s) at a time.
> +                */
> +               if (max_discard_sectors < (1 << (25-9)))        /* 32GiB */
> +                       max_discard_sectors = 1 << (25-9);
>
> And here you're ignoring q->limits.max_discard_sectors.  I'm surprised
> this worked!
>
> +       }
> +
> +       /*
> +        * Ensure that max_discard_sectors is of the proper
> +        * granularity, so that requests stay aligned after a split.
> +        */
> +       max_discard_sectors -= max_discard_sectors % granularity;
> +       if (unlikely(!max_discard_sectors)) {
> +               /* Avoid infinite loop below. Being cautious never hurts. */
> +               return -EOPNOTSUPP;
>         }
>
>         atomic_set(&bb.done, 1);
>
> Grant, can we start over with the problem description? (Sorry, I didn't
> see the previous posts.)  I'd like to know the values of discard_granularity
> and discard_max_bytes for your device.  Additionally, it would be
> interesting to know how the discards are being initiatied.  Is it via a
> userspace utility such as mkfs, online discard via some file system
> mounted with -o discard, or something else?  Finally, can you post
> binary blktrace data somewhere for the slow case?
>
> Thanks!
> Jeff
>
>
>
>
>> On Mon, Sep 28, 2015 at 2:45 PM, Grant Grundler <grundler@chromium.org> wrote:
>>> [resending...I forgot to switch gmail back to text-only mode. grrrh..]
>>>
>>> ---------- Forwarded message ----------
>>> From: Grant Grundler <grundler@chromium.org>
>>> Date: Mon, Sep 28, 2015 at 2:42 PM
>>> Subject: Re: RFC: 32-bit __data_len and REQ_DISCARD+REQ_SECURE
>>> To: Grant Grundler <grundler@chromium.org>
>>> Cc: Jens Axboe <axboe@kernel.dk>, Ulf Hansson
>>> <ulf.hansson@linaro.org>, LKML <linux-kernel@vger.kernel.org>,
>>> "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
>>>
>>>
>>> On Thu, Sep 24, 2015 at 10:39 AM, Grant Grundler <grundler@chromium.org> wrote:
>>>>
>>>> Some followup.
>>> ...
>>>>
>>>> 2) I've been able to test this hack on an eMMC device:
>>>> [   13.147747] mmc..._secdiscard_rq(mmc1) ERASE from 14116864 cnt
>>>> 0x2c00000 (size 22528 MiB)
>>>> [   13.155964] sdhci cmd: 35/0x1a arg 0xd76800
>>>> [   13.160266] sdhci cmd: 36/0x1a arg 0x39767ff
>>>> [   13.164593] sdhci cmd: 38/0x1b arg 0x80000000
>>>> [   13.803360] random: nonblocking pool is initialized
>>>> [   14.567735] sdhci cmd: 13/0x1a arg 0x10000
>>>> [   14.573324] mmc..._secdiscard_rq(mmc1) err 0
>>>>
>>>> This was with ~15K files and about 5GB written to the device. 1.4
>>>> seconds compared to about 20 minutes to secure erase the same region
>>>> with original v3.18 code.
>>>
>>>
>>> To put a few more numbers on the "chunk size vs perf":
>>>  1EG (512KB) -> 44K commands -> ~20 minutes
>>> 32EG (16MB) -> 1375 commands -> ~1 minute
>>> 128EG (64MB) -> 344 commands -> ~30 seconds
>>> 8191EG (~4GB) -> 6 commands -> 2 seconds + ~8 seconds mkfs
>>> (I'm assuming times above include about 6-10 seconds of mkfs as part
>>> of writing a new file system)
>>>
>>> This is with only ~300MB of data written to the partition. I'm fully
>>> aware that times will vary depending on how much data needs to be
>>> migrated (and in this case very little or none). I'm certain the
>>> difference will only get worse for the smaller the "chunk size" used
>>> to Secure Erase due to repeated data migration.
>>>
>>> Given the different use model for secure erase (legal/contractually
>>> required behavior), is using 4GB chunk size acceptable?
>>>
>>> Would anyone be terribly offended if I used the recently added
>>> "MMC_IOC_MULTI_CMD" to send the cmd 35/36/38 sequence to the eMMC
>>> device to securely erase the offending partition?
>>>
>>> thanks,
>>> grant
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

I am not sure if this issue is the same as been discussed earlier on
the mmc list regarding "discard/erase".

Anyway, there have been several attempts to fix bugs related to this.
One of these discussion kind of pointed out a viable solution, but
unfortunate no patches that adopts that solution have been posted yet.

You might want to read up on this.
https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg23643.html
http://linux-mmc.vger.kernel.narkive.com/Wp31G953/patch-mmc-core-don-t-return-1-for-max-discard

So this is an old issue, which should have been fixed long long long time ago...

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253075

FromGrant Grundler <grundler@chromium.org>
Date2015-10-21 19:10 +0200
Message-ID<qm53I-ZN-15@gated-at.bofh.it>
In reply to#1252596
On Wed, Oct 21, 2015 at 2:00 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
....
>>>> To put a few more numbers on the "chunk size vs perf":
>>>>  1EG (512KB) -> 44K commands -> ~20 minutes
>>>> 32EG (16MB) -> 1375 commands -> ~1 minute
>>>> 128EG (64MB) -> 344 commands -> ~30 seconds
>>>> 8191EG (~4GB) -> 6 commands -> 2 seconds + ~8 seconds mkfs
>>>> (I'm assuming times above include about 6-10 seconds of mkfs as part
>>>> of writing a new file system)
>>>>
>>>> This is with only ~300MB of data written to the partition. I'm fully
>>>> aware that times will vary depending on how much data needs to be
>>>> migrated (and in this case very little or none). I'm certain the
>>>> difference will only get worse for the smaller the "chunk size" used
>>>> to Secure Erase due to repeated data migration.
....
> I am not sure if this issue is the same as been discussed earlier on
> the mmc list regarding "discard/erase".
>
> Anyway, there have been several attempts to fix bugs related to this.
> One of these discussion kind of pointed out a viable solution, but
> unfortunate no patches that adopts that solution have been posted yet.
>
> You might want to read up on this.
> https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg23643.html
> http://linux-mmc.vger.kernel.narkive.com/Wp31G953/patch-mmc-core-don-t-return-1-for-max-discard
>
> So this is an old issue, which should have been fixed long long long time ago...

Agreed. :)  I'll read the references but hope that Gwendal (or someone
on Android Team?) can follow up on this shorter term. I've moved to a
different team (Google Onhub) and currently have a whole new set of
(wireless) issues to deal with. :(

At some point I expect I'll be circling back to mmc issues - storage
keeps following me like a hungry puppy where ever I go. /o\

thank you!
grant
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258453

FromJeff Moyer <jmoyer@redhat.com>
Date2015-10-28 23:20 +0100
Message-ID<qoHex-5t-13@gated-at.bofh.it>
In reply to#1252596
Ulf Hansson <ulf.hansson@linaro.org> writes:

> I am not sure if this issue is the same as been discussed earlier on
> the mmc list regarding "discard/erase".
>
> Anyway, there have been several attempts to fix bugs related to this.
> One of these discussion kind of pointed out a viable solution, but
> unfortunate no patches that adopts that solution have been posted yet.
>
> You might want to read up on this.
> https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg23643.html
> http://linux-mmc.vger.kernel.narkive.com/Wp31G953/patch-mmc-core-don-t-return-1-for-max-discard
>
> So this is an old issue, which should have been fixed long long long time ago...

Thanks Ulf.  After reading all of the linked discussions, it's my
understanding that this is an emmc-specific issue that doesn't require
any block layer changes.  If that's wrong, please let me know.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253085

FromGrant Grundler <grundler@chromium.org>
Date2015-10-21 19:40 +0200
Message-ID<qm5wL-1y2-39@gated-at.bofh.it>
In reply to#1251960
On Tue, Oct 20, 2015 at 11:57 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
> Hi Grant,
>
> Grant Grundler <grundler@chromium.org> writes:
>
>> Ping? Does no one care how long BLK_SECDISCARD takes?
>>
>> ChromeOS has landed this change as a compromise between "fast" (<10
>> seconds) and "minimize risk" (~90 seconds) for a 23GB partition on
>> eMMC:
>>     https://chromium-review.googlesource.com/#/c/302413/
>
> Including the patch would be helpful.  I believe this is it.

Thanks Jeff!  Gerrit does provide easy mechanisms to review or pull
the patch - easy to use - not easy to find though. :/

> My comments are inline.
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 8411be3..43943c7 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
>
> @@ -60,21 +60,37 @@
>         granularity = max(q->limits.discard_granularity >> 9, 1U);
>         alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
>
> -       /*
> -        * Ensure that max_discard_sectors is of the proper
> -        * granularity, so that requests stay aligned after a split.
> -        */
> -       max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> -       max_discard_sectors -= max_discard_sectors % granularity;
> -       if (unlikely(!max_discard_sectors)) {
> -               /* Avoid infinite loop below. Being cautious never hurts. */
> -               return -EOPNOTSUPP;
> -       }
> +       max_discard_sectors = min(q->limits.max_discard_sectors,
> +                                               UINT_MAX >> 9);
>
> Unnecessary reformatting.
>
>         if (flags & BLKDEV_DISCARD_SECURE) {
>                 if (!blk_queue_secdiscard(q))
>                         return -EOPNOTSUPP;
>                 type |= REQ_SECURE;
> +               /*
> +                * Secure erase performs better by telling the device
> +                * about the largest range possible.  Secure erase
> +                * piecemeal will likely result in mapped sectors
> +                * getting evacuated from one range and parked in
> +                * another range that will get erased by a future
> +                * erase command.  This does NOT happen for normal
> +                * TRIM or DISCARD operations.
> +                *
> +                * 32GB was a compromise to avoid blocking the device
> +                * for potentially minute(s) at a time.
> +                */
> +               if (max_discard_sectors < (1 << (25-9)))        /* 32GiB */
> +                       max_discard_sectors = 1 << (25-9);
>
> And here you're ignoring q->limits.max_discard_sectors.  I'm surprised
> this worked!

See Gwendal's earlier reply. Here is the entire thread:
   https://lkml.org/lkml/2015/9/22/1235

>
> +       }
> +
> +       /*
> +        * Ensure that max_discard_sectors is of the proper
> +        * granularity, so that requests stay aligned after a split.
> +        */
> +       max_discard_sectors -= max_discard_sectors % granularity;
> +       if (unlikely(!max_discard_sectors)) {
> +               /* Avoid infinite loop below. Being cautious never hurts. */
> +               return -EOPNOTSUPP;
>         }
>
>         atomic_set(&bb.done, 1);
>
> Grant, can we start over with the problem description? (Sorry, I didn't
> see the previous posts.)

First/second posting in https://lkml.org/lkml/2015/9/22/1235 should
provide this.

>  I'd like to know the values of discard_granularity
> and discard_max_bytes for your device.

Gwendal might be able to provide those. I no longer have possession the HW.

>  Additionally, it would be
> interesting to know how the discards are being initiatied.  Is it via a
> userspace utility such as mkfs, online discard via some file system
> mounted with -o discard, or something else?

BLK_SECDISCARD ioctl with parameters to describe the /data partition
on an android device.

> Finally, can you post
> binary blktrace data somewhere for the slow case?

Sorry,  -ENOHW.
 second
I only have a snippet of printk output from the original code with
slow performance:
[   13.409334] sdhci-cmd:  CMD 0x231a arg 0x3976800
[   13.414150] sdhci-cmd:  CMD 0x241a arg 0x3976bff
[   13.418790] sdhci-cmd:  CMD 0x261b arg 0x80000000
[   13.424488] sdhci-cmd:  CMD 0xd1a arg 0x10000
[   13.429622] sdhci-cmd:  CMD 0x231a arg 0x3976c00
[   13.434333] sdhci-cmd:  CMD 0x241a arg 0x3976fff
[   13.438968] sdhci-cmd:  CMD 0x261b arg 0x80000000
[   13.443717] sdhci-cmd:  CMD 0xd1a arg 0x10000
[   13.448113] sdhci-cmd:  CMD 0x231a arg 0x3977000
[   13.453087] sdhci-cmd:  CMD 0x241a arg 0x39773ff
[   13.457780] sdhci-cmd:  CMD 0x261b arg 0x80000000
[   13.462839] sdhci-cmd:  CMD 0xd1a arg 0x10000
[   13.468237] sdhci-cmd:  CMD 0x231a arg 0x3977400
[   13.472980] sdhci-cmd:  CMD 0x241a arg 0x39777ff
[   13.477619] sdhci-cmd:  CMD 0x261b arg 0x80000000
[   13.482352] sdhci-cmd:  CMD 0xd1a arg 0x10000

"CMD" is 35/36/38/13 (but in hex) + flags (IIRC)

Each command is taking ~20ms. But multiple that by 46k to erase the
entire 23GB partition == 15 minutes.

I will assert this is "best case" since I usually tested with very
little "live" data (< 300MB) that would need to be evacuated from any
given erase block.

> Thanks!

Thanks for the feedback! :)

cheers,
grant
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web