All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <tom.leiming@gmail.com>
To: Caleb Sander Mateos <csander@purestorage.com>
Cc: Christian Brauner <brauner@kernel.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
	Chris Mason <mason@kernel.org>
Subject: Re: ublk: consequences of a dead server are only failing on the last /dev/ublkcN release
Date: Sun, 20 Sep 2026 06:35:10 -0500	[thread overview]
Message-ID: <aq_E7nsEMFKpBlpr@fedora-laptop> (raw)
In-Reply-To: <CADUfDZrjhhvKHh=7KoEvczqrRjcwnJC4p8jh5N4kpOS=Q2RTFQ@mail.gmail.com>

On Fri, Sep 18, 2026 at 09:34:54AM -0700, Caleb Sander Mateos wrote:
> On Fri, Sep 18, 2026 at 3:47 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Hi,
> >
> > Chris scanned a new series of mine for me and he reported some
> > interesting behavior that I dug into.
> >
> > A ublk request that was handed to the server is only failed once the
> > last reference to /dev/ublkcN is dropped. So holding the file open keeps
> > the request alive.
> >
> > This causes any waiter to be stuck in D state and blocks STOP_DEV.
> >
> > I'm attaching two reproducers:
> >
> > (1) ublk_inherited_fd_test.c   the general case, no vfork involved
> > (2) vfork_ublk_test.c          the holder of the reference is the waiter itself
> >
> > Re (1):
> >
> > A single-threaded ublk server fork()s a helper. The helper inherits the
> > fdtable including /dev/ublkcN. Now another process opens /dev/ublkbN and
> > writes 4 kb then fsync()s. The servers gets a WRITE and _exit()s without
> > completing it - say it crashes:
> >
> >   - The writer sleeps in folio_wait_writeback() in D state.
> >   - STOP_DEV issued from another process never returns.
> >
> >     The control command runs on an io-wq worker of that process, which
> >     calls ublk_stop_dev() -> del_gendisk() -> bdev_mark_dead() ->
> >     filemap_write_and_wait_range() and ends up waiting on the page that
> >     won't be written back.
> >
> >     Said process can't exit and io_wq_put_and_exit() waits for the
> >     worker.
> >
> > This can only be undone if the helper is SIGKILLed:
> >
> >   # ublk0 up, server 119
> >   # helper 123 forked by the server, holds the inherited fds
> >   # WRITER fsync
> >   # server 119 died with the WRITE in hand
> >   # 2s after the server died:
> >   # writer pid 124, state D:
> >   #   folio_wait_bit_common
> >   #   folio_wait_writeback
> >   #   __filemap_fdatawait_range
> >   #   file_write_and_wait_range
> >   #   blkdev_fsync
> >   #   do_fsync
> >   # STOP_DEV from pid 125 has not returned after 5s:
> >   # STOP_DEV issuer thread 126:
> >   #   folio_wait_bit_common
> >   #   folio_wait_writeback
> >   #   __filemap_fdatawait_range
> >   #   filemap_write_and_wait_range
> >   #   bdev_mark_dead
> >   #   blk_report_disk_dead
> >   #   __del_gendisk
> >   #   del_gendisk
> >   #   ublk_stop_dev_unlocked.part.0
> >   #   ublk_ctrl_uring_cmd
> >   #   io_uring_cmd
> >   #   io_wq_submit_work
> >   #   io_worker_handle_work
> >   #   io_wq_worker
> >   # killing helper 123
> >   # WRITER fsync returned -1 errno 5
> >   # STOP_DEV returned after the helper died
> >
> > What happens to a device after a server crash depends on whoever else
> > holds /dev/ublkcN open: A forked helper, a process that got the fd via
> > SCM_RIGHTS, or a vfork child.
> >
> > I'm not sure whether that's intended semantics but it surely has
> > confusion potential and should probably be at least documented.
> >
> > I suppose failing outstanding requests on server crash is intentionally
> > not done for recover reasons or whatever. But maybe STOP_DEV should be
> > made to work.
> >
> > Re (2):
> >
> > vfork() makes it really ugly. Say a single-threaded ublk server vfork()s
> > a child. The child opens /dev/ublkbN, writes some stuff and closes the
> > fd. Since close(2) runs bdev_release() -> sync_blockdev() it waits for
> > the write.
> >
> > The write gets dispatched to the server's task work. But the server
> > sleeps in wait_for_vfork_done() until the child execs or exits.
> >
> > But the child cannot exec or exit because close(2) doesn't return.
> >
> > ublk makes this more hairy though. Even a SIGKILL sent to the server aka
> > the parent won't help. The vfork()ed child hangs on the close(2).
> >
> > So that stays in D state forever with an undeletable device:
> >
> >     server, after SIGKILL: gone
> >     child:
> >       State:  D (disk sleep)
> >       folio_wait_bit_common
> >       folio_wait_writeback
> >       __filemap_fdatawait_range
> >       filemap_write_and_wait_range
> >       bdev_release
> >       blkdev_release
> >       __fput
> >       fput_close_sync
> >       __x64_sys_close
> >
> > Unrelated, but also a potential issue:
> >
> > The commit 7fc4da6a304b ("ublk: scan partition in async way") made
> > partition scans run from a workqueue that holds disk->open_mutex while
> > the server serves its reads and START_DEV returns before it's finished.
> >
> > Any server that treats START_DEV's return as a signal that the disk is
> > ready and doesn't serve requests hangs every open() of /dev/ublkbN on
> > uninterruptible on open_mutex. You can combine that with (1) and (2).
> 
> Indeed, we've also encountered this issue in our ublk server that was
> opening /dev/ublkb* to call ioctl(BLKROSET). (UBLK_ATTR_READ_ONLY only
> works to configure the read-only status *before* a ublk device is
> started, so the ioctl is needed to configure it when recovering a ublk
> device.)

It is supposed that ublk server doesn't deal with /dev/ublkbN directly.

We can add control commands for updating parameter runtime, or UBLK_CMD_SET_PARAMS
may be enough before ublk device is started.


Thanks, 
Ming

  reply	other threads:[~2026-09-20 11:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 10:29 ublk: consequences of a dead server are only failing on the last /dev/ublkcN release Christian Brauner
2026-09-18 16:34 ` Caleb Sander Mateos
2026-09-20 11:35   ` Ming Lei [this message]
2026-09-20 13:39 ` Ming Lei
2026-09-21  8:59   ` Hannes Reinecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aq_E7nsEMFKpBlpr@fedora-laptop \
    --to=tom.leiming@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=csander@purestorage.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mason@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.