On Tue, Jun 20, 2023 at 12:41:11PM +0200, Jan Kara wrote:
> I can see several solutions but since you've just reworked the code and I'm
> not 100% certain about the motivation, I figured I'll ask you first before
> spending significant time on something you won't like:
>
> 1) Just return the mode argument to blkdev_put().
>
> 2) Only pass to blkdev_put() whether we have write access or not as a
> separate argument.
>
> 3) Don't track number of opens for writing, instead check whether writes
> are blocked on each write access. I think this has a number of downsides
> but I mention it for completeness. One problem is we have to add checks to
> multiple places (buffered IO, direct IO) and existing mmap in particular
> will be very hard to deal with (need to add page_mkwrite() handler). All
> these checks add performance overhead. It is practically impossible
> (without significant performance overhead or new percpu datastructures) to
> properly synchronize open that wants to block writers against already
> running writes.
I think 3 is out for the reasons you mention. 2 feels a bit ugly,
but so does 1 given that we only really need the write flag. One thing
I through about earlier but decided was overkill at that time is to
return a cookie from blkdev_get_* that needs to be passed back to
blkdev_put. That could either be opaque to the caller, or replace the
bdev ala:
struct bdev_handle {
struct block_device *bdev;
void *holder;
bool for_write;
};
Given that fixups we needed to pass the right holder back in, it feels
like that would be the least error prone API, even if it is a fair
amount of churn.
