Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Vincent Whitchurch <vincent.whitchurch@axis.com>,
        Ulf Hansson <ulf.hansson@linaro.org>
CC:     "kernel@axis.com" <kernel@axis.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "tytso@mit.edu" <tytso@mit.edu>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] mmc: core: support zeroout using TRIM
Thread-Topic: [PATCH] mmc: core: support zeroout using TRIM
Thread-Index: AQHYW9zpo90Gc6qNZU+y8mD6XK0FLq0HR1bA
Date:   Fri, 29 Apr 2022 19:34:10 +0000
Message-ID: <DM6PR04MB657540F6BB9D02A5BA8A2AE6FCFC9@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20220429152118.3617303-1-vincent.whitchurch@axis.com>
In-Reply-To: <20220429152118.3617303-1-vincent.whitchurch@axis.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?8+Sk0MYVF841ohxD/FoUGDUihEGHwt/coR0oGbJi/5HyfNha/QP1n16E0rH2?=
 =?us-ascii?Q?wnhO+RiabaubGIjo3uUmw6gT/6K3g4RwS3OyOCWjxtzFteuVNWYvmSi4oxpI?=
 =?us-ascii?Q?uw7m6TMlJaVd14twmA0zge/JK4UKnevQ2OPeu/AxE7v3mDxXI26iyHGhbteF?=
 =?us-ascii?Q?h3sDa1h2vz+QPuE0TSakDvIq1AW7IScHFjCQi8DVIQzMA8Xm7QUQmMAzthDz?=
 =?us-ascii?Q?tmJqKJ94b2slM8HRvEhCtg5TcsDUP22inUN24Dpz06p9fUtF1RSkSjfLM5IZ?=
 =?us-ascii?Q?NHT8sTjSPss0rOlFpPVss9l8zpvuw7+nsszAR/4tSoCKiGANCNwEC7rgqIZH?=
 =?us-ascii?Q?bVyVSgP+I/4Cxn7oVpN3SLuQSWaWJxGrcm3+3PxF9SGhO6INRCi/VpJgUGVR?=
 =?us-ascii?Q?WBNFblqv9gc8UQVQFOg7wO4J4q6XorhP4EA3+eXrFERA5ZcqtKzuvmbw2e66?=
 =?us-ascii?Q?MUMArENP8NYaIoQEWqE8VKH1tAINwBo6f7SO7kOGDNeO3WbpG8gDmyd4O/Lx?=
 =?us-ascii?Q?n7B9bkR8eK5pBWPXeMwfIlb8EhltqbqXGx3dXfDWI4UbURhFvKWL1+kQ98cz?=
 =?us-ascii?Q?//owiOTc2A3dl8oQODvEIbi8qCN29xwTGZJkhFLcwlAjPzFKnOH8dn4ZLb69?=
 =?us-ascii?Q?eKoTzJuckbI9ex6cp3PNvqp7PED/Jl0KHksUxTw5TDhU1MOQGHepPROQ3914?=
 =?us-ascii?Q?8WRdI1UvS/aW+u4SakY+iHdSzcHuzaXtcQGzMu2p7+zG25YUlkQWnRrdmevC?=
 =?us-ascii?Q?uKIU3jfcw+XDPml/xmun0uAHqspCEhzy42rzXYqkLKlbfJmmOHSccltMInBo?=
 =?us-ascii?Q?WAx58e8BrmXygshOh5OHXP4Rc4/BMutOUkFDod5VicMI3jwY9J1Jjpx3GeE+?=
 =?us-ascii?Q?S/iuftu+JmWzXs2TZanhVBDCic6BR2n4FzhTzT6ax8s2eekEIxcQan5HGAF1?=
 =?us-ascii?Q?C591aVP6xnJWAG4/LkYIx++4TpqGCezPDjjrxu8K9LcWki0H4/Ep+UdM9Ktp?=
 =?us-ascii?Q?f5pyH7/LT7NrG5bDhkVSD/uKzqJWuQzOSXEJcQklPZiHBR54tg/RClyuzfDJ?=
 =?us-ascii?Q?/pLm5uCXl80tznWErfmk9nNLh/DD2FynYtFoqRJ5Q4L9X9s6JYOnGWAyQVY3?=
 =?us-ascii?Q?uTYecsJLpMC9awU0oOE190bACcba736bK8pv5quTXOQrSG+pH++fVWbk1dqi?=
 =?us-ascii?Q?1RRve9qUo5jwFmIIUZKfaTXi4RXny41ht1EGLtIVNU8WEg/QogDzhP0cqZNB?=
 =?us-ascii?Q?P6VXAt1MBbcV8HOnai+0GNGsCWFTQbURV3CUlW5pMfto+mU1DBIA2faUObf5?=
 =?us-ascii?Q?2dRb1rWZFSeuz/OWa4uhQZwgxG2F/rp4csrUWniWkQyEj8T+93DC+pKpbdf3?=
 =?us-ascii?Q?uqIgfZ1k9nkG51FAKiwxYD8YkD3V0znittuvBJ/Fj/95sj8VyjvI9oH2gz4a?=
 =?us-ascii?Q?KCjUrLSA1/JkgpHQ0juGWu4FzF9iFZUdGG0wSb0OaFq21RQFWBvo4PxEjQRv?=
 =?us-ascii?Q?zILNUqwZ+FUUpCbY+ia9FQ8WmxybkSOdX4pvkHUJuVk1j02PwKLrvgJIAY7/?=
 =?us-ascii?Q?EJHJOee5edeCciHE57uTEuvRxdmwbX+l+yanCM9lgFiBQEZ6UzXB1cHK/Dub?=
 =?us-ascii?Q?+5GwOeF9QCsCNeXECqF0fcSTQOAEU19LQUav7C1VxPT3XeaMpHUnZGFPGpzs?=
 =?us-ascii?Q?uw+HpEcXsYQBzN4L47nZJhyF065Ys40jDMTzuicUR8YJ26S1b5zIKM5urhGt?=
 =?us-ascii?Q?Wu/eCnpuxw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR04MB6575.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c802fc1-6c72-4941-bda7-08da2a173c1e
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2022 19:34:10.7452
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: neHgGG8lO1tnNbYZ8JHpB4xJvqIb7eTN/5G1Hi+sSJHgS7Pzq3H90C4UwVATRWn8ZfkrE1BS8bC1uLDoj8JqGw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR04MB0238
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RDNS_NONE,
	SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> If the device supports TRIM and indicates that it erases to zeros, we can=
 use
> it to support hardware offloading of REQ_OP_WRITE_ZEROES.
>=20
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
I agree that zero out is indeed a privet case of trim, as indicated by ERAS=
ED_MEM_CONT.

Looks good to me.

Thanks,
Avri

> ---
>=20
> Notes:
>     https://lore.kernel.org/lkml/20160303182146.GG9772@thunk.org/ seems
> to agree
>     that BLKZEROOUT can use TRIM on eMMC.
>=20
>     BLKDISCARD uses DISCARD when available so it can't be used to send
> TRIM.
>=20
>     If TRIM should not be used for BLKZEROOUT for some reason I guess the
> only way
>     is to use MMC_IOC_MULTI_CMD like in this commit in mmc-utils but
> that's a
>     rather low-level interface:
>=20
>      https://git.kernel.org/pub/scm/utils/mmc/mmc-
> utils.git/commit/?id=3D43282e80e174cc73b09b81a4d17cb3a7b4dc5cfc
>=20
>  drivers/mmc/core/block.c | 26 ++++++++++++++++++++++----
> drivers/mmc/core/queue.c |  2 ++
>  2 files changed, 24 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> 506dc900f5c7..0398b205a285 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -126,6 +126,7 @@ struct mmc_blk_data {
>  #define MMC_BLK_DISCARD                BIT(2)
>  #define MMC_BLK_SECDISCARD     BIT(3)
>  #define MMC_BLK_CQE_RECOVERY   BIT(4)
> +#define MMC_BLK_TRIM           BIT(5)
>=20
>         /*
>          * Only set in main mmc_blk_data associated @@ -1090,12 +1091,13
> @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct
> request *req)
>         blk_mq_end_request(req, ret ? BLK_STS_IOERR : BLK_STS_OK);  }
>=20
> -static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct
> request *req)
> +static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct
> request *req,
> +                                  int type, unsigned int erase_arg)
>  {
>         struct mmc_blk_data *md =3D mq->blkdata;
>         struct mmc_card *card =3D md->queue.card;
>         unsigned int from, nr;
> -       int err =3D 0, type =3D MMC_BLK_DISCARD;
> +       int err =3D 0;
>         blk_status_t status =3D BLK_STS_OK;
>=20
>         if (!mmc_can_erase(card)) {
> @@ -1111,13 +1113,13 @@ static void mmc_blk_issue_discard_rq(struct
> mmc_queue *mq, struct request *req)
>                 if (card->quirks & MMC_QUIRK_INAND_CMD38) {
>                         err =3D mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>                                          INAND_CMD38_ARG_EXT_CSD,
> -                                        card->erase_arg =3D=3D MMC_TRIM_=
ARG ?
> +                                        erase_arg =3D=3D MMC_TRIM_ARG ?
>                                          INAND_CMD38_ARG_TRIM :
>                                          INAND_CMD38_ARG_ERASE,
>                                          card->ext_csd.generic_cmd6_time)=
;
>                 }
>                 if (!err)
> -                       err =3D mmc_erase(card, from, nr, card->erase_arg=
);
> +                       err =3D mmc_erase(card, from, nr, erase_arg);
>         } while (err =3D=3D -EIO && !mmc_blk_reset(md, card->host, type))=
;
>         if (err)
>                 status =3D BLK_STS_IOERR;
> @@ -1127,6 +1129,19 @@ static void mmc_blk_issue_discard_rq(struct
> mmc_queue *mq, struct request *req)
>         blk_mq_end_request(req, status);  }
>=20
> +static void mmc_blk_issue_trim_rq(struct mmc_queue *mq, struct request
> +*req) {
> +       mmc_blk_issue_erase_rq(mq, req, MMC_BLK_TRIM, MMC_TRIM_ARG);
> }
> +
> +static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct
> +request *req) {
> +       struct mmc_blk_data *md =3D mq->blkdata;
> +       struct mmc_card *card =3D md->queue.card;
> +
> +       mmc_blk_issue_erase_rq(mq, req, MMC_BLK_DISCARD,
> +card->erase_arg); }
> +
>  static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
>                                        struct request *req)  { @@ -2327,6=
 +2342,9 @@ enum
> mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request
> *req)
>                 case REQ_OP_SECURE_ERASE:
>                         mmc_blk_issue_secdiscard_rq(mq, req);
>                         break;
> +               case REQ_OP_WRITE_ZEROES:
> +                       mmc_blk_issue_trim_rq(mq, req);
> +                       break;
>                 case REQ_OP_FLUSH:
>                         mmc_blk_issue_flush(mq, req);
>                         break;
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index
> c69b2d9df6f1..bbe2ea829ea7 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -191,6 +191,8 @@ static void mmc_queue_setup_discard(struct
> request_queue *q,
>                 q->limits.discard_granularity =3D SECTOR_SIZE;
>         if (mmc_can_secure_erase_trim(card))
>                 blk_queue_flag_set(QUEUE_FLAG_SECERASE, q);
> +       if (mmc_can_trim(card) && card->erased_byte =3D=3D 0)
> +               blk_queue_max_write_zeroes_sectors(q, max_discard);
>  }
>=20
>  static unsigned short mmc_get_max_segments(struct mmc_host *host)
> --
> 2.34.1