Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <fa89e2a960e98b016d4935490fa2905aab0868f7.camel@linux.ibm.com>
Subject: Re: [PATCH v13 0/3] scsi: ufs: Add Host Performance Booster Support
From:   James Bottomley <jejb@linux.ibm.com>
Reply-To: jejb@linux.ibm.com
To:     Greg KH <greg@kroah.com>, Christoph Hellwig <hch@infradead.org>
Cc:     Daejun Park <daejun7.park@samsung.com>,
        "avri.altman@wdc.com" <avri.altman@wdc.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
        "beanhuo@micron.com" <beanhuo@micron.com>,
        "stanley.chu@mediatek.com" <stanley.chu@mediatek.com>,
        "cang@codeaurora.org" <cang@codeaurora.org>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "tomas.winkler@intel.com" <tomas.winkler@intel.com>,
        ALIM AKHTAR <alim.akhtar@samsung.com>,
        "gregkh@google.com" <gregkh@google.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Sang-yoon Oh <sangyoon.oh@samsung.com>,
        Sung-Jun Park <sungjun07.park@samsung.com>,
        yongmyung lee <ymhungry.lee@samsung.com>,
        Jinyoung CHOI <j-young.choi@samsung.com>,
        Adel Choi <adel.choi@samsung.com>,
        BoRam Shin <boram.shin@samsung.com>,
        SEUNGUK SHIN <seunguk.shin@samsung.com>
Date:   Mon, 07 Dec 2020 10:54:58 -0800
In-Reply-To: <X85116BXkgTtRDKV@kroah.com>
References: <CGME20201103044021epcms2p8f1556853fc23414442b9e958f20781ce@epcms2p8>
         <2038148563.21604378702426.JavaMail.epsvc@epcpadp3>
         <X85sxxgpdtFXiKsg@kroah.com> <20201207180655.GA30657@infradead.org>
         <X85zEFduHeUr4YKR@kroah.com> <20201207182603.GA2499@infradead.org>
         <X85116BXkgTtRDKV@kroah.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 2020-12-07 at 19:35 +0100, Greg KH wrote:
> On Mon, Dec 07, 2020 at 06:26:03PM +0000, Christoph Hellwig wrote:
> > On Mon, Dec 07, 2020 at 07:23:12PM +0100, Greg KH wrote:
> > > What "real workload" test can be run on this to help show if it
> > > is useful or not?  These vendors seem to think it helps for some
> > > reason, otherwise they wouldn't have added it to their silicon :)
> > > 
> > > Should they run fio?  If so, any hints on a config that would be
> > > good to show any performance increases?
> > 
> > A real actual workload that matters.  Then again that was Martins
> > request to even justify it.  I don't think the broken addressing
> > that breaks a whole in the SCSI addressing has absolutely not
> > business being supported in Linux ever.  The vendors should have
> > thought about the design before committing transistors to something
> > that fundamentally does not make sense.

Actually, that's not the way it works: vendors add commands because
standards mandate.  That's why people who want weird commands go and
join standard committees.  Unfortunately this means that a lot of the
commands the standard mandates end up not being very useful in
practice.  For instance in SCSI we really only implement a fraction of
the commands in the standard.

In this case, the industry already tried a very similar approach with
GEN 1 hybrid drives and it turned into a complete disaster, which is
why the mode became optional in shingle drives and much better modes,
which didn't have the huge shared state problem, superseded it.  Plus
truncating the LBA of a READ 16 to 4 bytes is asking for capacity
problems down the line, so even the actual implementation seems to be
problematic.

All in all, this looks like a short term fix which will go away when
the drive capacity improves and thus all the effort changing the driver
will eventually be wasted.

> So "time to boot an android system with this enabled and disabled"
> would be a valid workload, right?  I'm guessing that's what the
> vendors here actually care about, otherwise there is no real stress-
> test on a UFS system that I know of.

Um, does it?  I don't believe even the UFS people have claimed this. 
The problem is that HPB creates a shared state between the driver and
the device.  That shared state has to be populated, which has to happen
at start of day, so it's entirely unclear if this is a win or a slow
down for boot.

James