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;
IronPort-SDR: Z676J+D6L2SAV2Vd3O4VETapYq2WbDP0JsjXBlFReSWfdTMaNpObVj6q4VAkhbjfIZjwqLufMq
 m0rjSP9v4nZo32WQz+I6RyNHOAXCe5YAan5cItVarMm8p5z3yvUgfK/IAXiYYbbv1Qk2mnsy0q
 22NN5TWk16maypjwjqmziAiyaVDO41maOaXjgbpncHXUiATAyb+pzJcfWU+OTMZnTVTm3ki5rJ
 WLyA6a29QFTLPcgwtkxczORDUHlVR4Fjch5DMVS0J1MBHV4u4a/T821T1n/b8C0g3b53RaENu4
 jII=
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Can Guo <cang@codeaurora.org>,
        Zang Leigang <zangleigang@hisilicon.com>
CC:     "James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        Bart Van Assche <bvanassche@acm.org>,
        yongmyung lee <ymhungry.lee@samsung.com>,
        Daejun Park <daejun7.park@samsung.com>,
        "alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
        "asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
        Avi Shchislowski <Avi.Shchislowski@wdc.com>,
        Bean Huo <beanhuo@micron.com>,
        "stanley.chu@mediatek.com" <stanley.chu@mediatek.com>
Subject: RE: [PATCH v6 02/10] scsi: ufshpb: Add host control mode support to
 rsp_upiu
Thread-Topic: [PATCH v6 02/10] scsi: ufshpb: Add host control mode support to
 rsp_upiu
Thread-Index: AQHXHvL4SUo2feYNMUG/LIqOH/XvgaqSflYAgAAH0oCAAJBfwA==
Date:   Wed, 24 Mar 2021 12:44:49 +0000
Message-ID: <DM6PR04MB657597BCF8C0E2093AA4656BFC639@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20210322081044.62003-1-avri.altman@wdc.com>
 <20210322081044.62003-3-avri.altman@wdc.com>
 <20210324033104.5kms7pez6arnkaoz@shaphisprc48410>
 <e3bfb8c4fcadc8e1b7f9fec0ee4d57f5@codeaurora.org>
In-Reply-To: <e3bfb8c4fcadc8e1b7f9fec0ee4d57f5@codeaurora.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 4e580120-7225-4dfd-d600-08d8eec29cdc
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2021 12:44:49.4843
 (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: 92Pd+fNKaK1EZjGRU4aAq//7F1zIHUQdz4dehIlcpX/PGa/NVjG8zTLSDEsx/Wv0veT+CdtQR3ZgZHU3QP5VtQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR04MB1260
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> >> @@ -1245,6 +1257,18 @@ static void
> ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              srgn_i =3D
> >>                      be16_to_cpu(rsp_field->hpb_active_field[i].active=
_srgn);
> >>
> >> +            rgn =3D hpb->rgn_tbl + rgn_i;
> >> +            if (hpb->is_hcm &&
> >> +                (rgn->rgn_state !=3D HPB_RGN_ACTIVE || is_rgn_dirty(r=
gn))) {
> >> +                    /*
> >> +                     * in host control mode, subregion activation
> >> +                     * recommendations are only allowed to active reg=
ions.
> >> +                     * Also, ignore recommendations for dirty regions=
 - the
> >> +                     * host will make decisions concerning those by h=
imself
> >> +                     */
> >> +                    continue;
> >> +            }
> >> +
> >
> > Hi Avri, host control mode also need the recommendations from device,
> > because the bkops would make the ppn invalid, is that right?
>=20
> Right, but ONLY recommandations to ACTIVE regions are of host's interest
> in host control mode. For those inactive regions, host makes its own
> decisions.
Correct.
Generally speaking, in host control mode, the host may ignore the device re=
commendation altogether.
In host mode the device is not allowed to send recommendation to Inactive r=
egions.
Furthermore, as the comment above indicates, we *chose* to ignore recommend=
ation for DIRTY regions.
We do so, because the host is aware of dirty regions and can make the decis=
ion per its own internal logic.
ONLY if the region is ACTIVE and CLEAN - the host is unaware that bkops too=
k place,
So we *chose* to act per the device recommendation. =20

Thanks,
Avri
>=20
> Can Guo.
>=20
> >
> >>              dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
> >>                      "activate(%d) region %d - %d\n", i, rgn_i, srgn_i=
);
> >>
> >> @@ -1252,7 +1276,6 @@ static void ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              ufshpb_update_active_info(hpb, rgn_i, srgn_i);
> >>              spin_unlock(&hpb->rsp_list_lock);
> >>
> >> -            rgn =3D hpb->rgn_tbl + rgn_i;
> >>              srgn =3D rgn->srgn_tbl + srgn_i;
> >>
> >>              /* blocking HPB_READ */
> >> @@ -1263,6 +1286,14 @@ static void
> ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              hpb->stats.rb_active_cnt++;
> >>      }
> >>
> >> +    if (hpb->is_hcm) {
> >> +            /*
> >> +             * in host control mode the device is not allowed to inac=
tivate
> >> +             * regions
> >> +             */
> >> +            goto out;
> >> +    }
> >> +
> >>      for (i =3D 0; i < rsp_field->inactive_rgn_cnt; i++) {
> >>              rgn_i =3D be16_to_cpu(rsp_field->hpb_inactive_field[i]);
> >>              dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
> >> @@ -1287,6 +1318,7 @@ static void ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              hpb->stats.rb_inactive_cnt++;
> >>      }
> >>
> >> +out:
> >>      dev_dbg(&hpb->sdev_ufs_lu->sdev_dev, "Noti: #ACT %u #INACT %u\n",
> >>              rsp_field->active_rgn_cnt, rsp_field->inactive_rgn_cnt);
> >>
> >> diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
> >> index 7df30340386a..032672114881 100644
> >> --- a/drivers/scsi/ufs/ufshpb.h
> >> +++ b/drivers/scsi/ufs/ufshpb.h
> >> @@ -121,6 +121,8 @@ struct ufshpb_region {
> >>
> >>      /* below information is used by lru */
> >>      struct list_head list_lru_rgn;
> >> +    unsigned long rgn_flags;
> >> +#define RGN_FLAG_DIRTY 0
> >>  };
> >>
> >>  #define for_each_sub_region(rgn, i, srgn)                           \
> >> --
> >> 2.25.1
> >>