Re: [PATCH v2 31/34] block: use file->f_op to indicate restricted writes

[Christian Brauner <brauner@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

On Thu, Feb 01, 2024 at 06:36:31PM +0100, Jan Kara wrote:
> On Thu 01-02-24 17:16:02, Christian Brauner wrote:
> > On Thu, Feb 01, 2024 at 12:08:58PM +0100, Jan Kara wrote:
> > > On Tue 23-01-24 14:26:48, Christian Brauner wrote:
> > > > Make it possible to detected a block device that was opened with
> > > > restricted write access solely based on its file operations that it was
> > > > opened with. This avoids wasting an FMODE_* flag.
> > > > 
> > > > def_blk_fops isn't needed to check whether something is a block device
> > > > checking the inode type is enough for that. And def_blk_fops_restricted
> > > > can be kept private to the block layer.
> > > > 
> > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > 
> > > I don't think we need def_blk_fops_restricted. If we have BLK_OPEN_WRITE
> > > file against a bdev with bdev_writes_blocked() == true, we are sure this is
> > > the handle blocking other writes so we can unblock them in
> > > bdev_yield_write_access()...
> 
> ...
> 
> > -       if (mode & BLK_OPEN_RESTRICT_WRITES)
> > +       if (mode & BLK_OPEN_WRITE) {
> > +               if (bdev_writes_blocked(bdev))
> > +                       bdev_unblock_writes(bdev);
> > +               else
> > +                       bdev->bd_writers--;
> > +       }
> > +       if (bdev_file->f_op == &def_blk_fops_restricted)
> 
> Uh, why are you leaving def_blk_fops_restricted check here? I'd expect you
> can delete def_blk_fops_restricted completely...

Copy-paste error when dumping this into here. Here's the full patch.

[-- Attachment #2: 0001-block-don-t-rely-on-BLK_OPEN_RESTRICT_WRITES-when-yi.patch --]
[-- Type: text/x-diff, Size: 1910 bytes --]

From 25609e947674d2c24fb18edd0dabb5a49f27f23d Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 23 Jan 2024 14:26:48 +0100
Subject: [PATCH] block: don't rely on BLK_OPEN_RESTRICT_WRITES when yielding
 write access

Make it possible to detected a block device that was opened with
restricted write access based only on BLK_OPEN_WRITE and
bdev->bd_writers < 0 so we won't have to claim another FMODE_* flag.

Link: https://lore.kernel.org/r/20240123-vfs-bdev-file-v2-31-adbd023e19cc@kernel.org
base-commit: 0bd1bf95a554f5f877724c27dbe33d4db0af4d0c
change-id: 20240129-vfs-bdev-file-bd_inode-385a56c57a51
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 block/bdev.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 9be8c3c683ae..b19cbcd6a4bf 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -799,16 +799,21 @@ static void bdev_claim_write_access(struct block_device *bdev, blk_mode_t mode)
 		bdev->bd_writers++;
 }
 
-static void bdev_yield_write_access(struct block_device *bdev, blk_mode_t mode)
+static void bdev_yield_write_access(struct file *bdev_file, blk_mode_t mode)
 {
+	struct block_device *bdev;
+
 	if (bdev_allow_write_mounted)
 		return;
 
+	bdev = file_bdev(bdev_file);
 	/* Yield exclusive or shared write access. */
-	if (mode & BLK_OPEN_RESTRICT_WRITES)
-		bdev_unblock_writes(bdev);
-	else if (mode & BLK_OPEN_WRITE)
-		bdev->bd_writers--;
+	if (mode & BLK_OPEN_WRITE) {
+		if (bdev_writes_blocked(bdev))
+			bdev_unblock_writes(bdev);
+		else
+			bdev->bd_writers--;
+	}
 }
 
 /**
@@ -1020,7 +1025,7 @@ void bdev_release(struct file *bdev_file)
 		sync_blockdev(bdev);
 
 	mutex_lock(&disk->open_mutex);
-	bdev_yield_write_access(bdev, handle->mode);
+	bdev_yield_write_access(bdev_file, handle->mode);
 
 	if (handle->holder)
 		bd_end_claim(bdev, handle->holder);
-- 
2.43.0