Re: [PATCH block/for-next v2 01/16] block: add a new helper to get inode from block_device

[Yu Kuai <yukuai1@xxxxxxxxxxxxxxx>]

Hi,

在 2023/11/28 0:32, Christoph Hellwig 写道:
> On Mon, Nov 27, 2023 at 09:07:22PM +0800, Yu Kuai wrote:
> > 1) Is't okay to add a new helper to pass in bdev for following apis?
>
> For some we already have them (e.g. bdev_nr_bytes to read the bdev)
> size, for some we need to add them.  The big thing that seems to
> stick out is page cache API, and I think that is where we need to
> define maintainable APIs for file systems and others to use the
> block device page cache.  Probably only in folio versions and not
> pages once if we're touching the code anyay
Thanks for the advice! In case I'm understanding correctly, do you mean
that all other fs/drivers that is using pages versions can safely switch
to folio versions now?

By the way, my orginal idea was trying to add a new field 'bd_flags'
in block_devcie, and then add a new bit so that bio_check_ro() will
only warn once for each partition. Now that this patchset will be quite
complex, I'll add a new bool field 'bd_ro_warned' to fix the above
problem first, and then add 'bd_flags' once this patchset is done.

Thanks,
Kuai

> > 2) For the file fs/buffer.c, there are some special usage like
> > following that I don't think it's good to add a helper:
> >
> > spin_lock(&bd_inode->i_mapping->private_lock);
> >
> > Is't okay to move following apis from fs/buffer.c directly to
> > block/bdev.c?
> >
> > __find_get_block
> > bdev_getblk
> I'm not sure moving is a good idea, but we might end up the
> some kind of low-level access from buffer.c, be that special
> helpers, a separate header or something else.  Let's sort out
> the rest of the kernel first.
>
> .