Re: [PATCH V2 1/5] nvdimm: virtio_pmem: always wake -ENOSPC waiters

[Li Chen]

Hi Pankaj,

 ---- On Fri, 02 Jan 2026 20:29:59 +0800  Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx> wrote --- 
 > +CC MST
 > > virtio_pmem_host_ack() reclaims virtqueue descriptors with
 > > virtqueue_get_buf(). The -ENOSPC waiter wakeup is tied to completing the
 > > returned token.
 > >
 > > If token completion is skipped for any reason, reclaimed descriptors may
 > > not wake a waiter and the submitter may sleep forever waiting for a free
 > > slot.
 > >
 > > Always wake one -ENOSPC waiter for each virtqueue completion before
 > > touching the returned token.
 > >
 > > Use READ_ONCE()/WRITE_ONCE() for the wait_event() flags (done and
 > > wq_buf_avail). They are observed by waiters without pmem_lock, so make
 > > the accesses explicit single loads/stores and avoid compiler
 > > reordering/caching across the wait/wake paths.
 > >
 > > Signed-off-by: Li Chen <me@linux.beauty>
 > > ---
 > >  drivers/nvdimm/nd_virtio.c | 35 +++++++++++++++++++++--------------
 > >  1 file changed, 21 insertions(+), 14 deletions(-)
 > >
 > > diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
 > > index c3f07be4aa22..6f9890361d0b 100644
 > > --- a/drivers/nvdimm/nd_virtio.c
 > > +++ b/drivers/nvdimm/nd_virtio.c
 > > @@ -9,26 +9,33 @@
 > >  #include "virtio_pmem.h"
 > >  #include "nd.h"
 > >
 > > +static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
 > > +{
 > > +       struct virtio_pmem_request *req_buf;
 > > +
 > > +       if (list_empty(&vpmem->req_list))
 > > +               return;
 > > +
 > > +       req_buf = list_first_entry(&vpmem->req_list,
 > > +                                 struct virtio_pmem_request, list);
 > 
 > [...]
 > > +       list_del_init(&req_buf->list);
 > > +       WRITE_ONCE(req_buf->wq_buf_avail, true);
 > > +       wake_up(&req_buf->wq_buf);
 > 
 > Seems with the above change (3 line fix), you are allowing to wakeup a waiter
 > before accessing the token. Maybe simplify the patch by just
 > keeping this change in the single patch & other changes (READ_ONCE/WRITE_ONCE)
 > onto separate patch with corresponding commit log.

Sorry for the late reply, I just realized I somehow missed this email :-p

Thanks for the suggestion. I'll do it in v3.

Regards,
Li​