Message ID | 20221020024529.25227-1-wenchao.chen666@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | mmc: block: Support Host to control FUA. | expand |
On 10/19/22 19:45, Wenchao Chen wrote: > From: Wenchao Chen <wenchao.chen@unisoc.com> > > This patch introduces host->fua_disable for MMC host controller. > The host can turn off FUA to improve performance. > > 1. fua_disable = 1 > /sys/block/mmcblk0/queue # cat fua 0 > I tested 5 times for each case and output a average speed. > > 1) Sequential read: > Speed: 266.8MiB/s, 265.1MiB/s, 262.9MiB/s, 268.7MiB/s, 265.2MiB/s > Average speed: 265.74MiB/s > > 2) Random read: > Speed: 98.75MiB/s, 98.7MiB/s, 98.5MiB/s, 99.4MiB/s, 98.7MiB/s > Average speed: 98.81MiB/s > > 3) Sequential write: > Speed: 199.94MiB/s, 199.1MiB/s, 205.5MiB/s, 206.5MiB/s, 191.5MiB/s > Average speed: 200.5MiB/s > > 4) Random write: > Speed: 68.6MiB/s, 71.8MiB/s, 77.1MiB/s, 64.8MiB/s, 69.3MiB/s > Average speed: 70.32MiB/s > > 2. fua_disable = 0 (default 0) > /sys/block/mmcblk0/queue # cat fua 1 > I tested 5 times for each case and output a average speed. > > 1) Sequential read: > Speed: 259.3MiB/s, 258.8MiB/s, 258.2MiB/s, 259.5MiB/s, 253.5MiB/s > Average speed: 257.86MiB/s > > 2) Random read: > Speed: 98.9MiB/s, 101MiB/s, 101MiB/s, 99MiB/s, 101.1MiB/s > Average speed: 100.2MiB/s > > 3) Sequential write: > Speed: 153.7MiB/s, 146.2MiB/s, 151.2MiB/s, 148.8MiB/s, 147.5MiB/s > Average speed: 149.48MiB/s > > 4) Random write: > Speed: 12.9MiB/s, 12.3MiB/s, 12.6MiB/s, 12.8MiB/s, 12.8MiB/s > Average speed: 12.68MiB/s > > According to the above data, disable FUA (fua_disable = 1) improves the > performance. Therefore, it is recommended to support the host to control > FUA. > > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com> Based on the data provided seems it does increase the performance. Perhaps it is worth mentioning the reason behind the performance increase and other side effects of doing so ? Also, it is usually helpful to mention performance increase in the % in the commit log so you can move verbose log (like above) into the cover-letter for documentation purpose, this is based on the feedback I got, unless maintainer(s) prefers is this way :). -ck
> According to the above data, disable FUA (fua_disable = 1) improves the > performance. Therefore, it is recommended to support the host to control > FUA. > > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com> > --- > drivers/mmc/core/block.c | 3 ++- > include/linux/mmc/host.h | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index > 54cd009aee50..333e819e077a 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2490,7 +2490,8 @@ static struct mmc_blk_data > *mmc_blk_alloc_req(struct mmc_card *card, > ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) || > card->ext_csd.rel_sectors)) { > md->flags |= MMC_BLK_REL_WR; > - fua_enabled = true; > + if (!card->host->fua_disable) > + fua_enabled = true; Where are you allowing to set fua_disable? And then I would expect you to call blk_queue_flag_set to set QUEUE_FLAG_FUA. Thanks, Avri > cache_enabled = true; > } > if (mmc_cache_enabled(card->host)) diff --git > a/include/linux/mmc/host.h b/include/linux/mmc/host.h index > 8fdd3cf971a3..16a5bee3eeae 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -517,6 +517,9 @@ struct mmc_host { > struct blk_crypto_profile crypto_profile; #endif > > + /* Host FUA support */ > + bool fua_disable; > + > /* Host Software Queue support */ > bool hsq_enabled; > > -- > 2.17.1
On Thu, Oct 20, 2022 at 11:07 AM Chaitanya Kulkarni <chaitanyak@nvidia.com> wrote: > > On 10/19/22 19:45, Wenchao Chen wrote: > > From: Wenchao Chen <wenchao.chen@unisoc.com> > > > > This patch introduces host->fua_disable for MMC host controller. > > The host can turn off FUA to improve performance. > > > > 1. fua_disable = 1 > > /sys/block/mmcblk0/queue # cat fua 0 > > I tested 5 times for each case and output a average speed. > > > > 1) Sequential read: > > Speed: 266.8MiB/s, 265.1MiB/s, 262.9MiB/s, 268.7MiB/s, 265.2MiB/s > > Average speed: 265.74MiB/s > > > > 2) Random read: > > Speed: 98.75MiB/s, 98.7MiB/s, 98.5MiB/s, 99.4MiB/s, 98.7MiB/s > > Average speed: 98.81MiB/s > > > > 3) Sequential write: > > Speed: 199.94MiB/s, 199.1MiB/s, 205.5MiB/s, 206.5MiB/s, 191.5MiB/s > > Average speed: 200.5MiB/s > > > > 4) Random write: > > Speed: 68.6MiB/s, 71.8MiB/s, 77.1MiB/s, 64.8MiB/s, 69.3MiB/s > > Average speed: 70.32MiB/s > > > > 2. fua_disable = 0 (default 0) > > /sys/block/mmcblk0/queue # cat fua 1 > > I tested 5 times for each case and output a average speed. > > > > 1) Sequential read: > > Speed: 259.3MiB/s, 258.8MiB/s, 258.2MiB/s, 259.5MiB/s, 253.5MiB/s > > Average speed: 257.86MiB/s > > > > 2) Random read: > > Speed: 98.9MiB/s, 101MiB/s, 101MiB/s, 99MiB/s, 101.1MiB/s > > Average speed: 100.2MiB/s > > > > 3) Sequential write: > > Speed: 153.7MiB/s, 146.2MiB/s, 151.2MiB/s, 148.8MiB/s, 147.5MiB/s > > Average speed: 149.48MiB/s > > > > 4) Random write: > > Speed: 12.9MiB/s, 12.3MiB/s, 12.6MiB/s, 12.8MiB/s, 12.8MiB/s > > Average speed: 12.68MiB/s > > > > According to the above data, disable FUA (fua_disable = 1) improves the > > performance. Therefore, it is recommended to support the host to control > > FUA. > > > > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com> > > Based on the data provided seems it does increase the performance. > Perhaps it is worth mentioning the reason behind the performance > increase and other side effects of doing so ? We suspect it will affect the progressive power-down test, but after a week of testing, turning off FUA did not affect this test. > > Also, it is usually helpful to mention performance increase in the > % in the commit log so you can move verbose log (like above) into > the cover-letter for documentation purpose, this is based on > the feedback I got, unless maintainer(s) prefers is this way :). > > -ck > Thank you very much for your suggestion, I will update it in the next version.
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 54cd009aee50..333e819e077a 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -2490,7 +2490,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) || card->ext_csd.rel_sectors)) { md->flags |= MMC_BLK_REL_WR; - fua_enabled = true; + if (!card->host->fua_disable) + fua_enabled = true; cache_enabled = true; } if (mmc_cache_enabled(card->host)) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 8fdd3cf971a3..16a5bee3eeae 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -517,6 +517,9 @@ struct mmc_host { struct blk_crypto_profile crypto_profile; #endif + /* Host FUA support */ + bool fua_disable; + /* Host Software Queue support */ bool hsq_enabled;