All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Do not flush cache when it is disabled
@ 2021-04-20  5:53 Avri Altman
  2021-04-20  5:53 ` [PATCH v3 1/2] mmc: block: Issue flush only if allowed Avri Altman
  2021-04-20  5:53 ` [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written Avri Altman
  0 siblings, 2 replies; 6+ messages in thread
From: Avri Altman @ 2021-04-20  5:53 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, Adrian Hunter; +Cc: linux-kernel, Avri Altman

v2 -> v3:
 - rebase onto recent cache changes

v1 -> v2:
 - Attend Adrian's comments

Cache is a temporary storage space in an eMMC device. Volatile by
nature, the cache should in typical case reduce the access time compared
to an access to the main nonvolatile storage.

The cache function can be turned ON and OFF. Once OFF, the host is not
expected to issue a flush-cache command to the device.

Avri Altman (2):
  mmc: block: Issue flush only if allowed
  mmc: block: Update ext_csd.cache_ctrl if it was written

 drivers/mmc/core/block.c   | 19 +++++++++++++++++++
 drivers/mmc/core/mmc.c     |  2 +-
 drivers/mmc/core/mmc_ops.h |  5 +++++
 3 files changed, 25 insertions(+), 1 deletion(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/2] mmc: block: Issue flush only if allowed
  2021-04-20  5:53 [PATCH v3 0/2] Do not flush cache when it is disabled Avri Altman
@ 2021-04-20  5:53 ` Avri Altman
  2021-04-20 11:20   ` Adrian Hunter
  2021-04-20  5:53 ` [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written Avri Altman
  1 sibling, 1 reply; 6+ messages in thread
From: Avri Altman @ 2021-04-20  5:53 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, Adrian Hunter
  Cc: linux-kernel, Avri Altman, Brendan Peter

The cache may be flushed to the nonvolatile storage by writing to
FLUSH_CACHE byte (EXT_CSD byte [32]). When in command queueing mode, the
cache may be flushed by issuing a CMDQ_TASK_ DEV_MGMT (CMD48) with a
FLUSH_CACHE op-code.  Either way, verify that The cache function is
turned ON before doing so.

fixes: 1e8e55b67030 (mmc: block: Add CQE support)

Reported-by: Brendan Peter <bpeter@lytx.com>
Tested-by: Brendan Peter <bpeter@lytx.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c   | 7 +++++++
 drivers/mmc/core/mmc.c     | 2 +-
 drivers/mmc/core/mmc_ops.h | 5 +++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 8bfd4d95b386..5b6501fc9fb7 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1476,6 +1476,11 @@ static int mmc_blk_cqe_issue_flush(struct mmc_queue *mq, struct request *req)
 	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
 	struct mmc_request *mrq = mmc_blk_cqe_prep_dcmd(mqrq, req);
 
+	if (mmc_card_mmc(mq->card) && !mmc_flush_allowed(mq->card)) {
+		blk_mq_end_request(req, BLK_STS_OK);
+		return -EPERM;
+	}
+
 	mrq->cmd->opcode = MMC_SWITCH;
 	mrq->cmd->arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
 			(EXT_CSD_FLUSH_CACHE << 16) |
@@ -2226,6 +2231,8 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
 		switch (req_op(req)) {
 		case REQ_OP_FLUSH:
 			ret = mmc_blk_cqe_issue_flush(mq, req);
+			if (ret == -EPERM)
+				return MMC_REQ_FINISHED;
 			break;
 		case REQ_OP_READ:
 		case REQ_OP_WRITE:
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 9ad4aa537867..e3da62ffcb5e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -2037,7 +2037,7 @@ static int _mmc_flush_cache(struct mmc_card *card)
 {
 	int err = 0;
 
-	if (card->ext_csd.cache_size > 0 && card->ext_csd.cache_ctrl & 1) {
+	if (mmc_flush_allowed(card)) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_FLUSH_CACHE, 1,
 				 CACHE_FLUSH_TIMEOUT_MS);
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 5782fdf4e8e9..2682bf66708a 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -19,6 +19,11 @@ enum mmc_busy_cmd {
 struct mmc_host;
 struct mmc_card;
 
+static inline bool mmc_flush_allowed(struct mmc_card *card)
+{
+	return card->ext_csd.cache_size > 0 && card->ext_csd.cache_ctrl & 1;
+}
+
 int mmc_select_card(struct mmc_card *card);
 int mmc_deselect_cards(struct mmc_host *host);
 int mmc_set_dsr(struct mmc_host *host);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written
  2021-04-20  5:53 [PATCH v3 0/2] Do not flush cache when it is disabled Avri Altman
  2021-04-20  5:53 ` [PATCH v3 1/2] mmc: block: Issue flush only if allowed Avri Altman
@ 2021-04-20  5:53 ` Avri Altman
  2021-04-20 11:22   ` Adrian Hunter
  1 sibling, 1 reply; 6+ messages in thread
From: Avri Altman @ 2021-04-20  5:53 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, Adrian Hunter; +Cc: linux-kernel, Avri Altman

The cache function can be turned ON and OFF by writing to the CACHE_CTRL
byte (EXT_CSD byte [33]).  However,  card->ext_csd.cache_ctrl is only
set on init if cache size > 0.

Fix that by explicitly setting ext_csd.cache_ctrl on ext-csd write.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 5b6501fc9fb7..8b07ed5e08de 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -572,6 +572,18 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 		main_md->part_curr = value & EXT_CSD_PART_CONFIG_ACC_MASK;
 	}
 
+	/*
+	 * Make sure to update CACHE_CTRL in case it was changed. The cache
+	 * will get turned back on if the card is re-initialized, e.g.
+	 * suspend/resume or hw reset in recovery.
+	 */
+	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_CACHE_CTRL) &&
+	    (cmd.opcode == MMC_SWITCH)) {
+		u8 value = MMC_EXTRACT_VALUE_FROM_ARG(cmd.arg) & 1;
+
+		card->ext_csd.cache_ctrl = value;
+	}
+
 	/*
 	 * According to the SD specs, some commands require a delay after
 	 * issuing the command.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] mmc: block: Issue flush only if allowed
  2021-04-20  5:53 ` [PATCH v3 1/2] mmc: block: Issue flush only if allowed Avri Altman
@ 2021-04-20 11:20   ` Adrian Hunter
  2021-04-20 13:22     ` Avri Altman
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2021-04-20 11:20 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: linux-kernel, Brendan Peter

On 20/04/21 8:53 am, Avri Altman wrote:
> The cache may be flushed to the nonvolatile storage by writing to
> FLUSH_CACHE byte (EXT_CSD byte [32]). When in command queueing mode, the
> cache may be flushed by issuing a CMDQ_TASK_ DEV_MGMT (CMD48) with a
> FLUSH_CACHE op-code.  Either way, verify that The cache function is
> turned ON before doing so.
> 
> fixes: 1e8e55b67030 (mmc: block: Add CQE support)
> 
> Reported-by: Brendan Peter <bpeter@lytx.com>
> Tested-by: Brendan Peter <bpeter@lytx.com>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c   | 7 +++++++
>  drivers/mmc/core/mmc.c     | 2 +-
>  drivers/mmc/core/mmc_ops.h | 5 +++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 8bfd4d95b386..5b6501fc9fb7 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1476,6 +1476,11 @@ static int mmc_blk_cqe_issue_flush(struct mmc_queue *mq, struct request *req)
>  	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
>  	struct mmc_request *mrq = mmc_blk_cqe_prep_dcmd(mqrq, req);
>  
> +	if (mmc_card_mmc(mq->card) && !mmc_flush_allowed(mq->card)) {
> +		blk_mq_end_request(req, BLK_STS_OK);
> +		return -EPERM;
> +	}
> +
>  	mrq->cmd->opcode = MMC_SWITCH;
>  	mrq->cmd->arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
>  			(EXT_CSD_FLUSH_CACHE << 16) |
> @@ -2226,6 +2231,8 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
>  		switch (req_op(req)) {
>  		case REQ_OP_FLUSH:
>  			ret = mmc_blk_cqe_issue_flush(mq, req);
> +			if (ret == -EPERM)
> +				return MMC_REQ_FINISHED;

Using an error code for this case seems a little fragile.

How about something like:

 		case REQ_OP_FLUSH:
			if (mmc_blk_cache_disabled(mq->card)) {
				blk_mq_end_request(req, BLK_STS_OK);
				return MMC_REQ_FINISHED;
			}
 			ret = mmc_blk_cqe_issue_flush(mq, req);


static bool mmc_blk_cache_disabled(struct mmc_card *card)
{
	return mmc_card_mmc(mq->card) && !mmc_flush_allowed(card);
}

>  			break;
>  		case REQ_OP_READ:
>  		case REQ_OP_WRITE:
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 9ad4aa537867..e3da62ffcb5e 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -2037,7 +2037,7 @@ static int _mmc_flush_cache(struct mmc_card *card)
>  {
>  	int err = 0;
>  
> -	if (card->ext_csd.cache_size > 0 && card->ext_csd.cache_ctrl & 1) {
> +	if (mmc_flush_allowed(card)) {
>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  				 EXT_CSD_FLUSH_CACHE, 1,
>  				 CACHE_FLUSH_TIMEOUT_MS);
> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
> index 5782fdf4e8e9..2682bf66708a 100644
> --- a/drivers/mmc/core/mmc_ops.h
> +++ b/drivers/mmc/core/mmc_ops.h
> @@ -19,6 +19,11 @@ enum mmc_busy_cmd {
>  struct mmc_host;
>  struct mmc_card;
>  
> +static inline bool mmc_flush_allowed(struct mmc_card *card)
> +{
> +	return card->ext_csd.cache_size > 0 && card->ext_csd.cache_ctrl & 1;
> +}
> +
>  int mmc_select_card(struct mmc_card *card);
>  int mmc_deselect_cards(struct mmc_host *host);
>  int mmc_set_dsr(struct mmc_host *host);
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written
  2021-04-20  5:53 ` [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written Avri Altman
@ 2021-04-20 11:22   ` Adrian Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2021-04-20 11:22 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: linux-kernel

On 20/04/21 8:53 am, Avri Altman wrote:
> The cache function can be turned ON and OFF by writing to the CACHE_CTRL
> byte (EXT_CSD byte [33]).  However,  card->ext_csd.cache_ctrl is only
> set on init if cache size > 0.
> 
> Fix that by explicitly setting ext_csd.cache_ctrl on ext-csd write.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/core/block.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 5b6501fc9fb7..8b07ed5e08de 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -572,6 +572,18 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>  		main_md->part_curr = value & EXT_CSD_PART_CONFIG_ACC_MASK;
>  	}
>  
> +	/*
> +	 * Make sure to update CACHE_CTRL in case it was changed. The cache
> +	 * will get turned back on if the card is re-initialized, e.g.
> +	 * suspend/resume or hw reset in recovery.
> +	 */
> +	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_CACHE_CTRL) &&
> +	    (cmd.opcode == MMC_SWITCH)) {
> +		u8 value = MMC_EXTRACT_VALUE_FROM_ARG(cmd.arg) & 1;
> +
> +		card->ext_csd.cache_ctrl = value;
> +	}
> +
>  	/*
>  	 * According to the SD specs, some commands require a delay after
>  	 * issuing the command.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH v3 1/2] mmc: block: Issue flush only if allowed
  2021-04-20 11:20   ` Adrian Hunter
@ 2021-04-20 13:22     ` Avri Altman
  0 siblings, 0 replies; 6+ messages in thread
From: Avri Altman @ 2021-04-20 13:22 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc; +Cc: linux-kernel, Brendan Peter

 
> On 20/04/21 8:53 am, Avri Altman wrote:
> > The cache may be flushed to the nonvolatile storage by writing to
> > FLUSH_CACHE byte (EXT_CSD byte [32]). When in command queueing mode,
> the
> > cache may be flushed by issuing a CMDQ_TASK_ DEV_MGMT (CMD48) with a
> > FLUSH_CACHE op-code.  Either way, verify that The cache function is
> > turned ON before doing so.
> >
> > fixes: 1e8e55b67030 (mmc: block: Add CQE support)
> >
> > Reported-by: Brendan Peter <bpeter@lytx.com>
> > Tested-by: Brendan Peter <bpeter@lytx.com>
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/block.c   | 7 +++++++
> >  drivers/mmc/core/mmc.c     | 2 +-
> >  drivers/mmc/core/mmc_ops.h | 5 +++++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 8bfd4d95b386..5b6501fc9fb7 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -1476,6 +1476,11 @@ static int mmc_blk_cqe_issue_flush(struct
> mmc_queue *mq, struct request *req)
> >       struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> >       struct mmc_request *mrq = mmc_blk_cqe_prep_dcmd(mqrq, req);
> >
> > +     if (mmc_card_mmc(mq->card) && !mmc_flush_allowed(mq->card)) {
> > +             blk_mq_end_request(req, BLK_STS_OK);
> > +             return -EPERM;
> > +     }
> > +
> >       mrq->cmd->opcode = MMC_SWITCH;
> >       mrq->cmd->arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
> >                       (EXT_CSD_FLUSH_CACHE << 16) |
> > @@ -2226,6 +2231,8 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct
> mmc_queue *mq, struct request *req)
> >               switch (req_op(req)) {
> >               case REQ_OP_FLUSH:
> >                       ret = mmc_blk_cqe_issue_flush(mq, req);
> > +                     if (ret == -EPERM)
> > +                             return MMC_REQ_FINISHED;
> 
> Using an error code for this case seems a little fragile.
> 
> How about something like:
> 
>                 case REQ_OP_FLUSH:
>                         if (mmc_blk_cache_disabled(mq->card)) {
>                                 blk_mq_end_request(req, BLK_STS_OK);
>                                 return MMC_REQ_FINISHED;
>                         }
>                         ret = mmc_blk_cqe_issue_flush(mq, req);
> 
> 
> static bool mmc_blk_cache_disabled(struct mmc_card *card)
> {
>         return mmc_card_mmc(mq->card) && !mmc_flush_allowed(card);
> }
Done.

Thanks,
Avri

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-20 13:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20  5:53 [PATCH v3 0/2] Do not flush cache when it is disabled Avri Altman
2021-04-20  5:53 ` [PATCH v3 1/2] mmc: block: Issue flush only if allowed Avri Altman
2021-04-20 11:20   ` Adrian Hunter
2021-04-20 13:22     ` Avri Altman
2021-04-20  5:53 ` [PATCH v3 2/2] mmc: block: Update ext_csd.cache_ctrl if it was written Avri Altman
2021-04-20 11:22   ` Adrian Hunter

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.