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:     Avri Altman <Avri.Altman@wdc.com>, 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/6wgAAPqZA=
Date:   Fri, 9 Jun 2023 08:29:30 +0000
Message-ID: <DM6PR04MB657509E3C4F9E9727F020ABEFC51A@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20230609030357.64360-1-luhongfei@vivo.com>
 <DM6PR04MB65753B9E63B07D53A82EB8DDFC51A@DM6PR04MB6575.namprd04.prod.outlook.com>
In-Reply-To: <DM6PR04MB65753B9E63B07D53A82EB8DDFC51A@DM6PR04MB6575.namprd04.prod.outlook.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?irMuvqk+5QvO1dItsxh7bCy3DKYrdrthZarmMhrL8O3VIdOAtaOCBX0g3kqt?=
 =?us-ascii?Q?dPA5L/A4AgjvZJMGRyIdV9HPdKNvazfZMMs1MyPx/q4N4lpbmpU1VjW0/U1Z?=
 =?us-ascii?Q?JnsF3wD5q0+VfPeAKMakW88Fos2tpPNIjSnO3Z4ZV3xOEyQOCJG54enBZvn2?=
 =?us-ascii?Q?kBtn9wxagesNQ8zXgzNbOxCB75TDH2PbLhHB7vGVkvuIa1QDDRsbYzLJu8Ax?=
 =?us-ascii?Q?e8BjFAF+COwyi4u2SpGpeYjsp124va2TX/a0fzvBdxRf7eb7yv3JiexNvFvH?=
 =?us-ascii?Q?F+UhHHDSfNeMGHFJZTNHYjezYkigwBIJb3nUhpWqvVZbgb5SjtkrnHuxr3sI?=
 =?us-ascii?Q?AfG7qD/iQKXuThG9XF8POHo2VQKAAMHvf1/ujLTcm74x+5QSKR33RMmBVtkV?=
 =?us-ascii?Q?f09ZNwOxIhBQNF4QGHh7K7vJd2SOjHkSyB18uX8u7s8nyZnHSbUmENR528F9?=
 =?us-ascii?Q?KToebMqIjRIH2hgsLDXp6Scz34lRQ66urqP+k5aCNEUgpTtKEo97ZzHsVcRM?=
 =?us-ascii?Q?cDsWvkdwL3ocOdlecM+H1zIeW/0c5wlPVvNDPXC/LYzSJShD4z1Pzrilmdtt?=
 =?us-ascii?Q?duixYILev/sZLFb+TfZIwb0F4beawT0KhW0nrnlyezqfusK3WyKYfUux4p+u?=
 =?us-ascii?Q?2lE+JfDwX/89SzdMSf+tTWflkoKc+5+O7EocTi6IPvKjmiZJ9hdDdvy8GEV8?=
 =?us-ascii?Q?tsdPT7CutRof4zgnjPDbC1sV5mfy7dnuhErhh4Q9uz6Ho+mCDZSbKzNF7dca?=
 =?us-ascii?Q?Y7W04aQDg+pH+vvzAu0Q0cOjo698PDV1bzr6kanhxomIzKgC+iBhh0iTMKT1?=
 =?us-ascii?Q?9BH80Ku08bbRM+2ibJbQSHRUXDMtF+MoHvaVeqyxqXKTsCUx9fF7kVsbBY5/?=
 =?us-ascii?Q?Rd1VF5cdyZinLYM/JIOq+L9WK29JIo8lpv5o4KFAwQIE5fxYMmKDuY0JuybQ?=
 =?us-ascii?Q?gSrzDeMJovDa5VNwm4SzZi715vNit5uN2A3+LFJz+t6nFmEe/yCaKJ6vH7pZ?=
 =?us-ascii?Q?s1Tj2OPcpbeyGoQcEO7s8JR9s5fBinwH//Kl9/4ZkTn20vEvWkIcOOlQfIQJ?=
 =?us-ascii?Q?9/wBaz/z6xAVOZPOQOoqTt2oyDIUFTMJL/4UO7eOkuCD8o/riJk8nb06nzl4?=
 =?us-ascii?Q?jYsa5/8j+aI2k+DHiKk3qH98uvWMz9xM6z/83oC2lKLuUUrH1fC4BS0V+xKM?=
 =?us-ascii?Q?teioTnedf4N55UrNiCiFfsKYRMsxCA6T+5LL6s9zMcLKRn5r6dlNffyG0MnV?=
 =?us-ascii?Q?VA1BzPaIl3KRcIjKB4wKRalB/gAxScNiLCR/asIaorDttl9uxE1omG9ReT4V?=
 =?us-ascii?Q?lxL4aBIkcpMuRJaIYNK5aagNlKoQAbElpFdXDDe4pEhDZUwBJlwIOB4vrbR3?=
 =?us-ascii?Q?NCJ55j0gBOaxoUaOVFcZsTYnM1MNaPApYZW9Oo/RqPWWGgSzannpxImmUJ1t?=
 =?us-ascii?Q?sJCHHJNVJjFRFtHeOklcajr7pp0yyXVn9qfDKhlAvpPxaKcmYm6UfNh7CK6R?=
 =?us-ascii?Q?rhCtZ6Zix0FZ2yN9gTsIS1LSC4h79Qp/GPONyWGtAaFYbFKdLIEI3TWl+33j?=
 =?us-ascii?Q?gKwNMToYe0gA0W5vOS7qok5ppmbm8onebJ15zRXI?=
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?DviX++xoITSsZIyo6A/OM+1/SFHkawGAeT643BMhj6BE/evmEP3FO7o3VioW?=
 =?us-ascii?Q?svqprFDUAE84wLH5e9Iq03A1rmWJYcH7OzzOflbcL+u8Vp1xz2At+XPBA3Bu?=
 =?us-ascii?Q?JLADcuIdQkNYmIKAs4ORI6i/bkNUfsiVqGHkw4RXcSctW3PCU8iKbqclgUiA?=
 =?us-ascii?Q?199PoghMiNENpkY0tjwbtUgI4/oeo+DMnHATbON/2v/rRt96v+sSKexyn64X?=
 =?us-ascii?Q?+0lCdKOKVVq9qzG6Wb8PNz1ZUiGsmu1qbUvajStwVk0fSjtNQKRMQ+rrl1SS?=
 =?us-ascii?Q?0CzddEtFJiQ1gxF5iG0x3dpjv5HTR97qfGQfdsb7C0Ct1cITvrIHIBHEn9ef?=
 =?us-ascii?Q?bvZ1NfwpBCT57BSL6KhLPVpAr/wefHl3TIpY1eu135XXZhMl56oRVApnWCDF?=
 =?us-ascii?Q?vRzEgCv8H8J2zJHIOZjQsCCXZOaRehK6Y/vt2HkzjSN9Yaomn/E1U8FYVUIU?=
 =?us-ascii?Q?dFHTRafu4ultvSn8fQ1iLpMC0grR1NtyJlvq+g1LH9uTon2FafF68fSug4Vc?=
 =?us-ascii?Q?aw5QO54M3ZHek9jq10ASqZaSJBeRp1g3QC4ddF//c+5DbVL6rD1VWeG6Eog7?=
 =?us-ascii?Q?4kPC7ojMNa9uPEJJ2L5PEArs4pXPu1f+BCQKIG0WkFd8813c6Yw5pBLtI5Wn?=
 =?us-ascii?Q?C6RvlYcWwFwVr1Y3RGBCp4a6lYetVpTB82g3Lw2DEN3BgVc8pRrsPrh9tmrM?=
 =?us-ascii?Q?4fxQP/YO6KGjqvyWuElXZqBNAWyKxCHRunRjSMG+jKKFN6/2Vd+gxCMc9BD1?=
 =?us-ascii?Q?ZLDkOyvaT1c2hTQ7vAuI9Fxyv/+gYK9Rfjh8sK1qjVryhhecn6E+zJKXBqEt?=
 =?us-ascii?Q?CVvXOJFgvHGyuayGraBvssHU4U9KnUR/HWs1+f3n5Cv5YSOyjM86E8+sfh1t?=
 =?us-ascii?Q?I+b637NrY4lTWEj8bnkwM+Lsv1gsQ3MObLU+9NMdba1JjXpz7N6d3la6Y7Qb?=
 =?us-ascii?Q?h78PDxKoTodWaFQPPgGbXzOddg2ySxBuG0U3X1IwjpYBNQXnO0ISa6wQk0zM?=
 =?us-ascii?Q?PnxEhTGCy2+IgeEUxR2LIQydZg=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: dbe36b40-2cf7-48a5-b24e-08db68c3a51a
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jun 2023 08:29:30.0875
 (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: vhebsEFkUbGYAYNKHC9/uIcZCkO2SvBTuC1SoWYJ66z2qG4+j/WWVwwYkuMongSnWFd7IqvJ1QDOQCgir5YZhg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR04MB8062
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

> >
> > There are three flags that control Write Booster Feature:
> >
> >     1. WB ON/OFF
> >     2. WB Hibern Flush ON/OFF (implicitly)
> >     3. WB Flush ON/OFF (explicit)
> >
> > In the case of "Hibern Flush", one of the conditions for flush WB
> > buffer is that avail_wb_buff < wb_flush_threshold.
> >
> > 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.
> >
> > So the sysfs attribute that controls this is necessary.
> >
> > The meaning of wb_flush_threshold is the percentage of WB's total
> > size, such as 1 representing 10%, 2 representing 20%, and so on.
> >
> > 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'.
> >
> >  drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
>=20
> You are missing sysfs documentation.
> >
> > 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;
> >  }
> >
> > +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 flush=
ing!\n");
>                return -EOPNOTSUPP;
>        }
I just noticed that Bart asked you to leave it out - sorry.  Please ignore.

Thanks,
Avri