Dear Zhang Yi Thank you for your quick response. Please see my comments below:
On Tue, Feb 24, 2026 at 6:32 PM Zhang Yi <[email protected]> wrote: > > Hi Robert! > > On 2/25/2026 8:05 AM, Robert Pang wrote: > > Dear Zhang Yi, > > > > In reviewing your patch series implementing support for the > > FALLOC_FL_WRITE_ZEROES flag, I noted the logic propagating > > max_write_zeroes_sectors to max_hw_wzeroes_unmap_sectors in commit > > 545fb46e5bc6 > > "nvme: set max_hw_wzeroes_unmap_sectors if device supports DEAC bit" [1]. > > This > > appears to be intended for devices that support the Write Zeroes command > > alongside the DEAC bit to indicate unmap capability. > > > > Furthermore, within core.c, the NVME_QUIRK_DEALLOCATE_ZEROES quirk already > > identifies devices that deterministically return zeroes after a deallocate > > command [2]. This quirk currently enables Write Zeroes support via discard > > in > > existing implementations [3, 4]. > > > > Given this, would it be appropriate to respect NVME_QUIRK_DEALLOCATE_ZEROES > > also > > to enable unmap Write Zeroes for these devices, following the prior commit > > 6e02318eaea5 "nvme: add support for the Write Zeroes command" [5]? I have > > included a proposed change to nvme_update_ns_info_block() below for your > > consideration. > > > > Thank you for your point. Overall, this makes sense to me, but I have one > question below. > > > Best regards > > Robert Pang > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > index f5ebcaa2f859..9c7e2cabfab3 100644 > > --- a/drivers/nvme/host/core.c > > +++ b/drivers/nvme/host/core.c > > @@ -2422,7 +2422,9 @@ static int nvme_update_ns_info_block(struct nvme_ns > > *ns, > > * require that, it must be a no-op if reads from deallocated data > > * do not return zeroes. > > */ > > - if ((id->dlfeat & 0x7) == 0x1 && (id->dlfeat & (1 << 3))) { > > + if ((id->dlfeat & 0x7) == 0x1 && (id->dlfeat & (1 << 3)) || > > + (ns->ctrl->quirks & NVME_QUIRK_DEALLOCATE_ZEROES) && > > + (ns->ctrl->oncs & NVME_CTRL_ONCS_DSM)) { > ^^^^^^^^^^^^^^^^^^ > Why do you want to add a check for NVME_CTRL_ONCS_DSM? In > nvme_config_discard(), > it appears that we prioritize ctrl->dmrsl, allowing discard to still be > supported even on some non-standard devices where NVME_CTRL_ONCS_DSM is not > set. > In nvme_update_disk_info(), if the device only has > NVME_QUIRK_DEALLOCATE_ZEROES, > we still populate lim->max_write_zeroes_sectors (which might be non-zero on > devices that support NVME_CTRL_ONCS_WRITE_ZEROES). Right? So I'm not sure if > we > only need to check for NVME_QUIRK_DEALLOCATE_ZEROES here. > The check for NVME_CTRL_ONCS_DSM is to follow the same check in [3]. There, the check was added by 58a0c875ce02 "nvme: don't apply NVME_QUIRK_DEALLOCATE_ZEROES when DSM is not supported" [6]. The idea is to limit NVME_QUIRK_DEALLOCATE_ZEROES to those devices that support DSM. > > ns->head->features |= NVME_NS_DEAC; > > I think we should not set NVME_NS_DEAC for the quirks case. > Make sense. In that case, will it be more appropriate to set max_hw_wzeroes_unmap_sectors in nvme_update_disk_info() where NVME_QUIRK_DEALLOCATE_ZEROES is checked? I.e. diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index f5ebcaa2f859..3f5dd3f867e9 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2120,9 +2120,10 @@ static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id, lim->io_min = phys_bs; lim->io_opt = io_opt; if ((ns->ctrl->quirks & NVME_QUIRK_DEALLOCATE_ZEROES) && - (ns->ctrl->oncs & NVME_CTRL_ONCS_DSM)) + (ns->ctrl->oncs & NVME_CTRL_ONCS_DSM)) { lim->max_write_zeroes_sectors = UINT_MAX; - else + lim->max_hw_wzeroes_unmap_sectors = UINT_MAX; + } else lim->max_write_zeroes_sectors = ns->ctrl->max_zeroes_sectors; return valid; } Best regards Robert > Cheers, > Yi. > > > lim.max_hw_wzeroes_unmap_sectors = > > lim.max_write_zeroes_sectors; > > } > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=545fb46e5bc6 > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/nvme.h#n72 > > [3] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c#n938 > > [4] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c#n2122 > > [5] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6e02318eaea5 [6] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=58a0c875ce02
