linux-kernel - RE: [PATCH v3] scsi: ufs: wb: Add explicit flush

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <DM6PR04MB657509E3C4F9E9727F020ABEFC51A@DM6PR04MB6575.namprd04.prod.outlook.com>
Date:   Fri, 9 Jun 2023 08:29:30 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Avri Altman <Avri.Altman@....com>, Lu Hongfei <luhongfei@...o.com>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Bart Van Assche <bvanassche@....org>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Bean Huo <beanhuo@...ron.com>,
        Jinyoung Choi <j-young.choi@...sung.com>,
        Daniil Lunev <dlunev@...omium.org>,
        Peter Wang <peter.wang@...iatek.com>,
        "open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER" 
        <linux-scsi@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
CC:     "opensource.kernel@...o.com" <opensource.kernel@...o.com>
Subject: RE: [PATCH v3] scsi: ufs: wb: Add explicit flush_threshold sysfs
 attribute

> >
> > 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@...o.com>
> > ---
> > The modifications made compared to the previous version are as follows:
> > 1. Using 'wb_flush_threshold == 0' insted of 'wb_flush_threshold <= 0'.
> >
> >  drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> 
> 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 = 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 = 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 flushing!\n");
>                return -EOPNOTSUPP;
>        }
I just noticed that Bart asked you to leave it out - sorry.  Please ignore.

Thanks,
Avri