| Message ID | 20231211140552.973290-1-yukuai1@huaweicloud.com |
|---|---|
| Headers | show |
| Series | block: don't access bd_inode directly from other modules | expand |
On 2023/12/11 22:07, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > Avoid to access bd_inode directly, prepare to remove bd_inode from > block_devcie. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > fs/erofs/data.c | 18 ++++++++++++------ > fs/erofs/internal.h | 2 ++ > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/fs/erofs/data.c b/fs/erofs/data.c > index c98aeda8abb2..8cf3618190ab 100644 > --- a/fs/erofs/data.c > +++ b/fs/erofs/data.c > @@ -32,8 +32,7 @@ void erofs_put_metabuf(struct erofs_buf *buf) > void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr, > enum erofs_kmap_type type) > { > - struct inode *inode = buf->inode; > - erofs_off_t offset = (erofs_off_t)blkaddr << inode->i_blkbits; > + erofs_off_t offset = (erofs_off_t)blkaddr << buf->blkszbits; I'd suggest that use `buf->blkszbits` only for bdev_read_folio() since erofs_init_metabuf() is not always called before erofs_bread() is used. For example, buf->inode can be one of directory inodes other than initialized by erofs_init_metabuf(). Thanks, Gao Xiang > pgoff_t index = offset >> PAGE_SHIFT; > struct page *page = buf->page; > struct folio *folio; > @@ -43,7 +42,9 @@ void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr, > erofs_put_metabuf(buf); > > nofs_flag = memalloc_nofs_save(); > - folio = read_cache_folio(inode->i_mapping, index, NULL, NULL); > + folio = buf->inode ? > + read_mapping_folio(buf->inode->i_mapping, index, NULL) : > + bdev_read_folio(buf->bdev, offset); > memalloc_nofs_restore(nofs_flag); > if (IS_ERR(folio)) > return folio; > @@ -67,10 +68,15 @@ void *erofs_bread(struct erofs_buf *buf, erofs_blk_t blkaddr, > > void erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb) > { > - if (erofs_is_fscache_mode(sb)) > + if (erofs_is_fscache_mode(sb)) { > buf->inode = EROFS_SB(sb)->s_fscache->inode; > - else > - buf->inode = sb->s_bdev->bd_inode; > + buf->bdev = NULL; > + buf->blkszbits = buf->inode->i_blkbits; > + } else { > + buf->inode = NULL; > + buf->bdev = sb->s_bdev; > + buf->blkszbits = EROFS_SB(sb)->blkszbits; > + } > } > > void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb, > diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h > index b0409badb017..c9206351b485 100644 > --- a/fs/erofs/internal.h > +++ b/fs/erofs/internal.h > @@ -224,8 +224,10 @@ enum erofs_kmap_type { > > struct erofs_buf { > struct inode *inode; > + struct block_device *bdev; > struct page *page; > void *base; > + u8 blkszbits; > enum erofs_kmap_type kmap_type; > }; > #define __EROFS_BUF_INITIALIZER ((struct erofs_buf){ .page = NULL })
On Mon, Dec 11, 2023 at 10:07:53PM +0800, Yu Kuai wrote: > +static __always_inline int buffer_uptodate_or_error(struct buffer_head *bh) > +{ > + /* > + * If the buffer has the write error flag, data was failed to write > + * out in the block. In this case, set buffer uptodate to prevent > + * reading old data. > + */ > + if (buffer_write_io_error(bh)) > + set_buffer_uptodate(bh); > + return buffer_uptodate(bh); > +} So - risking this blows up into a lot of nasty work: Why do we even clear the uptodate flag on write errors? Doing so makes not sense to me as the data isn't any less uptodate just because we failed to write it..
From: Yu Kuai <yukuai3@huawei.com> Changes in v2: - remove some bdev apis that is not necessary; - pass in offset for bdev_read_folio() and __bdev_get_folio(); - remove bdev_gfp_constraint() and add a new helper in fs/buffer.c to prevent access bd_indoe() directly from mapping_gfp_constraint() in ext4.(patch 15, 16); - remove block_device_ejected() from ext4. Noted that following is not changed yet since v1: - Chirstoph suggested to remove invalidate_inode_pages2() from xen_update_blkif_status(), however, this sync_bdev() + invalidate_bdev() is used from many modules, and I'll leave this for later if we want to kill all of them. - Matthew suggested that pass in valid file_ra_state for cramfs, however, I don't see an easy way to do this for cramfs_lookup() and cramfs_read_super(). Patch 1 add some bdev apis, then follow up patches will use these apis to avoid access bd_inode directly, and hopefully the field bd_inode can be removed eventually(after figure out a way for fs/buffer.c). Yu Kuai (18): block: add some bdev apis xen/blkback: use bdev api in xen_update_blkif_status() bcache: use bdev api in read_super() mtd: block2mtd: use bdev apis s390/dasd: use bdev api in dasd_format() scsicam: use bdev api in scsi_bios_ptable() bcachefs: remove dead function bdev_sectors() bio: export bio_add_folio_nofail() btrfs: use bdev apis cramfs: use bdev apis in cramfs_blkdev_read() erofs: use bdev api gfs2: use bdev api nilfs2: use bdev api in nilfs_attach_log_writer() jbd2: use bdev apis buffer: add a new helper to read sb block ext4: use new helper to read sb block ext4: remove block_device_ejected() ext4: use bdev apis block/bdev.c | 70 ++++++++++++++++++++++++++ block/bio.c | 1 + block/blk.h | 2 - drivers/block/xen-blkback/xenbus.c | 3 +- drivers/md/bcache/super.c | 11 ++-- drivers/mtd/devices/block2mtd.c | 81 +++++++++++++----------------- drivers/s390/block/dasd_ioctl.c | 5 +- drivers/scsi/scsicam.c | 4 +- fs/bcachefs/util.h | 5 -- fs/btrfs/disk-io.c | 71 ++++++++++++-------------- fs/btrfs/volumes.c | 17 +++---- fs/btrfs/zoned.c | 15 +++--- fs/buffer.c | 68 +++++++++++++++++-------- fs/cramfs/inode.c | 36 +++++-------- fs/erofs/data.c | 18 ++++--- fs/erofs/internal.h | 2 + fs/ext4/dir.c | 6 +-- fs/ext4/ext4.h | 13 ----- fs/ext4/ext4_jbd2.c | 6 +-- fs/ext4/inode.c | 8 +-- fs/ext4/super.c | 66 ++++-------------------- fs/ext4/symlink.c | 2 +- fs/gfs2/glock.c | 2 +- fs/gfs2/ops_fstype.c | 2 +- fs/jbd2/journal.c | 3 +- fs/jbd2/recovery.c | 6 +-- fs/nilfs2/segment.c | 2 +- include/linux/blkdev.h | 17 +++++++ include/linux/buffer_head.h | 18 ++++++- 29 files changed, 301 insertions(+), 259 deletions(-)