On Fri, May 10, 2024 at 07:10:15PM +0530, Kanchan Joshi wrote: > Flexible Data Placement (FDP), as ratified in TP 4146a, allows the host > to control the placement of logical blocks so as to reduce the SSD WAF. > > Userspace can send the data lifetime information using the write hints. > The SCSI driver (sd) can already pass this information to the SCSI > devices. This patch does the same for NVMe. > > Fetches the placement-identifiers (plids) if the device supports FDP. > And map the incoming write-hints to plids. Just some additional background since this looks similiar to when the driver supported "streams". Supporting streams in the driver was pretty a non-issue. The feature was removed because devices didn't work with streams as expected, and supporting it carried more maintenance overhead for the upper layers. Since the block layer re-introduced write hints anyway outside of this use case, this looks fine to me to re-introduce support for those hints. So why not re-add stream support back? As far as I know, devices never implemented that feature as expected, the driver had to enable it on start up, and there's no required feedback mechanism to see if it's even working or hurting. For FDP, the user had to have configured the namespace that way in order to get this, so it's still an optional, opt-in feature. It's also mandatory for FDP capable drives to report WAF through the endurance log, so users can see the effects of using it. It would be nice to compare endurance logs with and without the FDP configuration enabled for your various workloads. This will be great to discuss at LSFMM next week. > +static int nvme_fetch_fdp_plids(struct nvme_ns *ns, u32 nsid) > +{ > + struct nvme_command c = {}; > + struct nvme_fdp_ruh_status *ruhs; > + struct nvme_fdp_ruh_status_desc *ruhsd; > + int size, ret, i; > + > + size = sizeof(*ruhs) + NVME_MAX_PLIDS * sizeof(*ruhsd); size = struct_size(ruhs, ruhsd, MAX_PLIDS); > +#define NVME_MAX_PLIDS (128) > + > /* > * Anchor structure for namespaces. There is one for each namespace in a > * NVMe subsystem that any of our controllers can see, and the namespace > @@ -457,6 +459,8 @@ struct nvme_ns_head { > bool shared; > bool passthru_err_log_enabled; > int instance; > + u16 nr_plids; > + u16 plids[NVME_MAX_PLIDS]; The largest index needed is WRITE_LIFE_EXTREME, which is "5", so I think NVME_MAX_PLIDS should be the same value. And it will save space in the struct.