Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Lu Hongfei <luhongfei@vivo.com>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Bart Van Assche <bvanassche@acm.org>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Bean Huo <beanhuo@micron.com>,
        Jinyoung Choi <j-young.choi@samsung.com>,
        Daniil Lunev <dlunev@chromium.org>,
        Peter Wang <peter.wang@mediatek.com>,
        "open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER" 
        <linux-scsi@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
CC:     "opensource.kernel@vivo.com" <opensource.kernel@vivo.com>
Subject: RE: [PATCH v3] scsi: ufs: wb: Add explicit flush_threshold sysfs
 attribute
Thread-Topic: [PATCH v3] scsi: ufs: wb: Add explicit flush_threshold sysfs
 attribute
Thread-Index: AQHZmn8WXHATsQGzxEGzwCQ9x4MRgK+CE/6w
Date:   Fri, 9 Jun 2023 07:35:54 +0000
Message-ID: <DM6PR04MB65753B9E63B07D53A82EB8DDFC51A@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20230609030357.64360-1-luhongfei@vivo.com>
In-Reply-To: <20230609030357.64360-1-luhongfei@vivo.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?mXh/oMDEx6/6TPusWt4CDGFmRNriK3IJA7TPqo7VN4LG51YQJd7GYiEXYPNt?=
 =?us-ascii?Q?PnHaI0bFDQu5/ngB1Ao6CG9+AbNoOF34FLEvHXBrwRMi5F17FyVHHQYXuUQb?=
 =?us-ascii?Q?Sh7h1j6ufBTWbjHuroyqxYcW5KLJ/VRE0d/nrzILEXCxsmPz36GPzreSAKvq?=
 =?us-ascii?Q?dAZL2lsX0cjvQkcnkveW41Xc9gK8KwzNRoLcsC2gt+cDcrQz9UB73SVuVayh?=
 =?us-ascii?Q?F6b86h60SxUvNCNtqtORWo0t4yygpqGk7uSD8oVC8R6O0bUwCuoh+iOmrYsJ?=
 =?us-ascii?Q?ki6MAG2G/a26hVmUdWNSemP2s0dSusqKXnbvpOBI7UTyKvC39XXuZrewhAax?=
 =?us-ascii?Q?iyRRzzChBcxNoHn2F7FgNi8XEG59Swr6MKdOJXOiGaDqtFdwpiy1sorgMeor?=
 =?us-ascii?Q?WgkzgIczSHjxWL/OOrSR/1MeEHKddMA6wLPQ8nwd+LI4873CzVgqNKNRACnR?=
 =?us-ascii?Q?diFa3u/pv2POebAFfWLKOsEw1FkhUW1Xr1DcHR04/HID0h5yXJNSevE12v2N?=
 =?us-ascii?Q?qgq3XeJi91pYBAEWiaFo66cg9+mZMSXwZCWLZDO6SHQPZz5qH25a5b7ZwuSo?=
 =?us-ascii?Q?ecy4IncMUO4bJcrbQRDabvHpaRA9m8xsmK0XgRgYXxEGAYNye7y30vuKfcIS?=
 =?us-ascii?Q?Jm8DraVzeY7boaeILTjSBtr+kwMOgGXve/iXp20cuTn68v6u4zkueROB9wMj?=
 =?us-ascii?Q?nT/H1asGlRnkmkYcgaGnEvgYMzBl3gmQsE7N2SmQNc8BpBfVnWwwjT/Hc1ki?=
 =?us-ascii?Q?Sq265oRts5PM2050JdwVzt3emJyngh/7X9YcWZ+ylHH1PNWNrLSLf8C95O9e?=
 =?us-ascii?Q?NCgJhWP95u9NXFBrSaxPIyzRAnJ8dJbZQmLcRPi+nTZN/4UQ+/7//Fso5lId?=
 =?us-ascii?Q?0mwMvEKHMZEizXWFoInkX0xukB4IQchR6BYikFvBMQCrZGEkDqhrDWPFNMIg?=
 =?us-ascii?Q?+JH9aagVgEjhrzqEShOSDNT5UpjL3n2wCcdfrOaX1WGXVkB9tx2sVUT13JI2?=
 =?us-ascii?Q?VasKISZpuUV6PnLD2u4SrE2rJYZ/+bxh2TL3JvR6PxGzYDNmPjkNgi4siERv?=
 =?us-ascii?Q?zdDVR3W3XMO7XJZ6ftD0rGXuO70ZIFDNOwGpecTQJhqE3uertYfZO5ZxRbxm?=
 =?us-ascii?Q?p2smm09nLRmmBzd15225Io4MpYjJEa+Mpn8kLtlXA0lKOiq2XrZIkG+I3rtf?=
 =?us-ascii?Q?GJvR3JYOVLHBm0px/palBBELq0NbsUR8r81kWuhPkLDlJZdYfzjfdP4uRjjx?=
 =?us-ascii?Q?+qnE9fnVdY8X7sYF9MbbrQ+p+5iDgtao567sXnE3BCMoE1BPsHk6PigvpZ0V?=
 =?us-ascii?Q?Zjx9cBBK1zStloT47aEPfUY516VC8BnmgtKZjxelqevB6PkaNJ64o8RUj3Zd?=
 =?us-ascii?Q?KVtrqHua2c6uUY2NTUS4pZaQLClRIbClldot5L9Kri5p336gvWX+VvSilj08?=
 =?us-ascii?Q?VbAk7IGZ/mJy7a/8u5O8dMxVFOyvG2IrU+tdq9yjs3WZttil6zXA6mMqXjbE?=
 =?us-ascii?Q?BeLUTRaOG+phI6E9gGAQgD1+iizJ73RJ5DGGwbJqyXXH8XB3HhgCBPoNs8Jq?=
 =?us-ascii?Q?0+NMzFqksXgbF2SELo9A96H9b/0CgmpohlFL4jYi?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?9bCpT7URD5RdZHOKzMdhB4PgQwoCN1HNCJ5Q+cjZPhy2MjsvN3MWexyqWGpy?=
 =?us-ascii?Q?OwB2ZOtu82ojOn8FTxdGh0yeN0g0LA+N96E+Sxs98z6Pz6YvuNUGnI9mLwDG?=
 =?us-ascii?Q?NU75z4n/pbwEIlHC/QQoUTq8louS0/ssJmcOXYEuL+Myi2e+3VFKwBl6gurj?=
 =?us-ascii?Q?fHa+fTe8KhYdaTplTdCzby5Hgra3zjx8uiJ7E9Ew0TjfZZKJ5edDB1f/asGC?=
 =?us-ascii?Q?JVFPWgmBBx53vc2JdUnBi31FINqb/5m5pt4Bj98Eo05c0/vkGNtLhzumAdr3?=
 =?us-ascii?Q?dh/npoz3PypzMRh/A56fmiG9wK9+owj4yAse/Bjah1G8ufrvEcGzaYf04FCI?=
 =?us-ascii?Q?IXUpfzI0HWakiNNX76PuCfg60bJHV/z9R6xseRHL5gWga0YWzzmyVgYDfYy9?=
 =?us-ascii?Q?cX1j1CEpLLKIyywGe26d6j0/W22Ur/h/I89AOIr+owOad3llR4XfSSPoylSV?=
 =?us-ascii?Q?NbZ77RXIZYBYqbh95FAQAp2oK5ZVEEwAoN7L90M4cZMknAbdxKrUlfusShk8?=
 =?us-ascii?Q?tpavxPcau4Z4KBAb784I11c6gfGcIW9PMw9u4qAyFJtDoGgfl6CtQHwyIXG6?=
 =?us-ascii?Q?dRClhffRNe05Ihrv+rALGhR2n596PvpaHAGSusLt72IPxMZB/sg2d5CeMy7T?=
 =?us-ascii?Q?CFnC9D807J2a7iqmjtX2IBOlnB50ZpNekzMnvpLdtijOkynjMb4Z0bwPMg3b?=
 =?us-ascii?Q?ye6Rs0yz/R4DaF911xU+/r6spLqwdO1mfaIAdqemOt1kEXRr8u3Z2uaAkIu/?=
 =?us-ascii?Q?myE3a42IJwNfYqRsEH3rurgIN+M8S9RaEfNid7pog4WsocngfYZJtUtdqTtN?=
 =?us-ascii?Q?IO+sKP93I/VihGBggJCEyU9VT5mMo9iZVS9oESYOUB+qFeHmiSCf5hn7pcPk?=
 =?us-ascii?Q?6KuEV5wj32DOYUkUbSbTmNQgc7odcJXRm+6jO9FF9vqW30ymID8yIpx52d9Y?=
 =?us-ascii?Q?EZzH6jtSlztHytrdCusEOYDt0cNycNeq82IpZAauHavEZ84J6EO9FkZxjbbg?=
 =?us-ascii?Q?6ZbtVcuG5x1/dmMwLaoLmLs61A=3D=3D?=
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: 660346b1-c139-4f97-8b2c-08db68bc2891
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jun 2023 07:35:54.6231
 (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: YHOUG9JPEjnbnVvamaEjfjttrTkW+ZmeJwAkNVXy8HxFOyS3Vwv53FsMDdwn0C+6qY/vPlCFtyAsbrjf+gNC4A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR04MB8978
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham 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

>=20
> There are three flags that control Write Booster Feature:
>=20
>     1. WB ON/OFF
>     2. WB Hibern Flush ON/OFF (implicitly)
>     3. WB Flush ON/OFF (explicit)
>=20
> In the case of "Hibern Flush", one of the conditions for flush WB buffer =
is that
> avail_wb_buff < wb_flush_threshold.
>=20
> As we know, different users have different requirements for power
> consumption and performance. Therefore, we need the ability to manually
> set wb_flush_threshold, so that users can easily and flexibly adjust the
> wb_flush_threshold value, thereby achieving a balance between power
> consumption and performance.
>=20
> So the sysfs attribute that controls this is necessary.
>=20
> The meaning of wb_flush_threshold is the percentage of WB's total size, s=
uch
> as 1 representing 10%, 2 representing 20%, and so on.
>=20
> Signed-off-by: Lu Hongfei <luhongfei@vivo.com>
> ---
> The modifications made compared to the previous version are as follows:
> 1. Using 'wb_flush_threshold =3D=3D 0' insted of 'wb_flush_threshold <=3D=
 0'.
>=20
>  drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)

You are missing sysfs documentation.
>=20
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c =
index
> cdf3d5f2b77b..347207f4e8ee
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -298,6 +298,37 @@ static ssize_t enable_wb_buf_flush_store(struct
> device *dev,
>         return res < 0 ? res : count;
>  }
>=20
> +static ssize_t wb_flush_threshold_show(struct device *dev,
> +                                        struct device_attribute *attr,
> +                                        char *buf) {
> +       struct ufs_hba *hba =3D dev_get_drvdata(dev);
> +
> +       return sysfs_emit(buf, "%u\n", hba->vps->wb_flush_threshold); }
> +
> +static ssize_t wb_flush_threshold_store(struct device *dev,
> +                                         struct device_attribute *attr,
> +                                         const char *buf, size_t count)
> +{
> +       struct ufs_hba *hba =3D dev_get_drvdata(dev);
> +       unsigned int wb_flush_threshold;
> +
> +       if (kstrtouint(buf, 0, &wb_flush_threshold))
> +               return -EINVAL;
Maybe also make note if wb not supported:
if (!ufshcd_is_wb_allowed(hba)  {
               dev_info(dev, "It is not allowed to configure WB buf flushin=
g!\n");
               return -EOPNOTSUPP;
       }

Thanks,
Avri
> +
> +       /* The range of values for wb_flush_threshold is (0,10] */
> +       if (wb_flush_threshold > UFS_WB_BUF_REMAIN_PERCENT(100) ||
> +               wb_flush_threshold =3D=3D 0) {
> +               dev_err(dev, "The value of wb_flush_threshold is invalid!=
\n");
> +               return -EINVAL;
> +       }
> +
> +       hba->vps->wb_flush_threshold =3D wb_flush_threshold;
> +
> +       return count;
> +}
> +
>  static DEVICE_ATTR_RW(rpm_lvl);
>  static DEVICE_ATTR_RO(rpm_target_dev_state);
>  static DEVICE_ATTR_RO(rpm_target_link_state);
> @@ -307,6 +338,7 @@ static DEVICE_ATTR_RO(spm_target_link_state);
>  static DEVICE_ATTR_RW(auto_hibern8);
>  static DEVICE_ATTR_RW(wb_on);
>  static DEVICE_ATTR_RW(enable_wb_buf_flush);
> +static DEVICE_ATTR_RW(wb_flush_threshold);
>=20
>  static struct attribute *ufs_sysfs_ufshcd_attrs[] =3D {
>         &dev_attr_rpm_lvl.attr,
> @@ -318,6 +350,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] =3D=
 {
>         &dev_attr_auto_hibern8.attr,
>         &dev_attr_wb_on.attr,
>         &dev_attr_enable_wb_buf_flush.attr,
> +       &dev_attr_wb_flush_threshold.attr,
>         NULL
>  };
>=20
> --
> 2.39.0