From: Wouter Verhelst <w@uter.be>
To: Christoph Hellwig <hch@lst.de>
Cc: Samuel Holland <samuel.holland@sifive.com>,
Al Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>,
Jens Axboe <axboe@kernel.dk>, Denis Efremov <efremov@linux.com>,
Josef Bacik <josef@toxicpanda.com>,
Stefan Haberland <sth@linux.ibm.com>,
Jan Hoeppner <hoeppner@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
"Darrick J . Wong" <djwong@kernel.org>, Chris Mason <clm@fb.com>,
David Sterba <dsterba@suse.com>,
linux-block@vger.kernel.org, nbd@other.debian.org,
linux-s390@vger.kernel.org, linux-btrfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Subject: Re: [PATCH 07/17] nbd: call blk_mark_disk_dead in nbd_clear_sock_ioctl
Date: Sun, 1 Oct 2023 19:10:53 +0200 [thread overview]
Message-ID: <ZRmoHaSojk6ra5OU@pc220518.home.grep.be> (raw)
In-Reply-To: <20230925074838.GA28522@lst.de>
On Mon, Sep 25, 2023 at 09:48:38AM +0200, Christoph Hellwig wrote:
> On Wed, Sep 20, 2023 at 03:41:11PM -0500, Samuel Holland wrote:
> > [ 14.619101] Buffer I/O error on dev nbd0, logical block 0, async page read
> >
> > [ 14.630490] nbd0: unable to read partition table
> >
> > I wonder if disk_force_media_change() is the right thing to call here instead.
>
> So what are the semantics of clearing the socket?
>
> The <= 6.5 behavior of invalidating fs caches, but not actually marking
> the fs shutdown is pretty broken, especially if this expects to resurrect
> the device and thus the file system later on.
nbd-client -d calls
ioctl(nbd, NBD_DISCONNECT);
ioctl(nbd, NBD_CLEAR_SOCK);
(error handling removed for clarity)
where "nbd" is the file handle to the nbd device. This expects that the
device is cleared and that then the device can be reused for a different
connection, much like "losetup -d". Expecting that the next connection
would talk to the same file system is wrong.
In netlink mode, it obviously doesn't use the ioctl()s, but instead
sends an NBD_CMD_DISCONNECT command, without any NBD_CLEAR_SOCK, for
which no equivalent message exists. At this point, obviously the same
result is expected in userspace, i.e., the device should now be
available for the next connection that may or may not be the same one.
nbd-client also has "-persist" option that used to work. This does
expect to resurrect the device and file system. It depends on semantics
where the kernel would block IO to the device until the nbd-client
process that initiated the connection exits, thus allowing it to
re-establish the connection if possible. When doing this, we don't issue
a DISCONNECT or CLEAR_SOCK message and obviously the client is expected
to re-establish a connection to the same device, thus some state should
be retained.
These semantics have however been broken at some point over the past decade
or so, but I didn't notice that at the time, so I didn't complain, and
it's therefore probably not relevant anymore. We should perhaps rethink
whether this is still a good idea given the way the netlink mode does
not have a client waiting for a return of the ioctl() call, and if so
how to implement a replacement.
Kind regards,
--
w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}
I will have a Tin-Actinium-Potassium mixture, thanks.
next prev parent reply other threads:[~2023-10-01 17:51 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 10:08 remove get_super Christoph Hellwig
2023-08-11 10:08 ` [PATCH 01/17] FOLD: reverts part of "fs: use the super_block as holder when mounting file systems" Christoph Hellwig
2023-08-11 10:44 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 02/17] btrfs: always open the device read-only in btrfs_scan_one_device Christoph Hellwig
2023-08-11 12:00 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 03/17] btrfs: call btrfs_close_devices from ->kill_sb Christoph Hellwig
2023-08-11 12:03 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 04/17] btrfs: split btrfs_fs_devices.opened Christoph Hellwig
2023-08-11 12:40 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 05/17] btrfs: open block devices after superblock creation Christoph Hellwig
2023-08-11 12:44 ` Christian Brauner
2023-08-11 13:11 ` David Sterba
2023-08-17 13:24 ` David Sterba
2023-08-11 10:08 ` [PATCH 06/17] btrfs: use the super_block as holder when mounting file systems Christoph Hellwig
2023-08-11 12:45 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 07/17] nbd: call blk_mark_disk_dead in nbd_clear_sock_ioctl Christoph Hellwig
2023-09-20 20:41 ` Samuel Holland
2023-09-25 7:48 ` Christoph Hellwig
2023-10-01 17:10 ` Wouter Verhelst [this message]
2023-10-02 6:21 ` Christoph Hellwig
2023-10-02 19:15 ` Samuel Holland
2023-08-11 10:08 ` [PATCH 08/17] block: simplify the disk_force_media_change interface Christoph Hellwig
2023-08-11 10:08 ` [PATCH 09/17] floppy: call disk_force_media_change when changing the format Christoph Hellwig
2023-08-11 10:08 ` [PATCH 10/17] amiflop: don't call fsync_bdev in FDFMTBEG Christoph Hellwig
2023-08-11 10:08 ` [PATCH 11/17] dasd: also call __invalidate_device when setting the device offline Christoph Hellwig
2023-08-11 10:08 ` [PATCH 12/17] block: drop the "busy inodes on changed media" log message Christoph Hellwig
2023-08-11 10:08 ` [PATCH 13/17] block: consolidate __invalidate_device and fsync_bdev Christoph Hellwig
2023-08-12 10:51 ` Christoph Hellwig
2023-08-12 17:04 ` Heiko Carstens
2023-08-12 17:28 ` Heiko Carstens
2023-08-12 20:43 ` Matthew Wilcox
2023-08-11 10:08 ` [PATCH 14/17] block: call into the file system for bdev_mark_dead Christoph Hellwig
2023-08-11 10:08 ` [PATCH 15/17] block: call into the file system for ioctl BLKFLSBUF Christoph Hellwig
2023-08-11 14:06 ` Josef Bacik
2023-08-11 10:08 ` [PATCH 16/17] fs: remove get_super Christoph Hellwig
2023-08-11 12:46 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 17/17] fs: simplify invalidate_inodes Christoph Hellwig
2023-08-11 12:48 ` Christian Brauner
2023-08-11 13:58 ` remove get_super Josef Bacik
2023-08-11 19:05 ` Josef Bacik
2023-08-14 19:19 ` David Sterba
2023-09-12 17:42 ` David Sterba
2023-09-14 8:48 ` Jan Kara
2023-09-14 12:03 ` David Sterba
2023-09-14 12:54 ` Jan Kara
2023-09-15 17:28 ` Jan Kara
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=ZRmoHaSojk6ra5OU@pc220518.home.grep.be \
--to=w@uter.be \
--cc=agordeev@linux.ibm.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=clm@fb.com \
--cc=djwong@kernel.org \
--cc=dsterba@suse.com \
--cc=efremov@linux.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hch@lst.de \
--cc=hoeppner@linux.ibm.com \
--cc=josef@toxicpanda.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=nbd@other.debian.org \
--cc=samuel.holland@sifive.com \
--cc=shinichiro.kawasaki@wdc.com \
--cc=sth@linux.ibm.com \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).