Hi,
This is a *compile only RFC* which adds a generic helper to initialize
the various fields of the bio that is repeated all the places in
file-systems, block layer, and drivers.
The new helper allows callers to initialize non-optional members of bio
such as bdev, sector, op, opflags, max_bvecs and gfp_mask by
encapsulating new bio allocation with bio alloc with initialization
at one place.
The objective of this RFC is to only start a discussion, this it not
completely tested at all.
-ck
Chaitanya Kulkarni (34):
block: move common code into blk_next_bio()
block: introduce and use bio_new
drdb: use bio_new in drdb
drdb: use bio_new() in submit_one_flush
xen-blkback: use bio_new
zram: use bio_new
dm: use bio_new in dm-log-writes
dm-zoned: use bio_new in get_mblock_slow
dm-zoned: use bio_new in dmz_write_mblock
dm-zoned: use bio_new in dmz_rdwr_block
nvmet: use bio_new in nvmet_bdev_execute_rw
scsi: target/iblock: use bio_new
block: use bio_new in __blkdev_direct_IO
fs/buffer: use bio_new in submit_bh_wbc
fscrypt: use bio_new in fscrypt_zeroout_range
fs/direct-io: use bio_new in dio_bio_alloc
iomap: use bio_new in iomap_dio_zero
iomap: use bio_new in iomap_dio_bio_actor
fs/jfs/jfs_logmgr.c: use bio_new in lbmRead
fs/jfs/jfs_logmgr.c: use bio_new in lbmStartIO
fs/jfs/jfs_metapage.c: use bio_new in metapage_writepage
fs/jfs/jfs_metapage.c: use bio_new in metapage_readpage
fs/mpage.c: use bio_new mpage_alloc
fs/nilfs: use bio_new nilfs_alloc_seg_bio
ocfs/cluster: use bio_new in dm-log-writes
xfs: use bio_new in xfs_rw_bdev
xfs: use bio_new in xfs_buf_ioapply_map
zonefs: use bio_new
power/swap: use bio_new in hib_submit_io
hfsplus: use bio_new in hfsplus_submit_bio()
iomap: use bio_new in iomap_readpage_actor
mm: use bio_new in __swap_writepage
mm: use bio_new in swap_readpage
mm: add swap_bio_new common bio helper
block/blk-lib.c | 34 ++++++++++-------------------
block/blk-zoned.c | 4 +---
block/blk.h | 5 +++--
drivers/block/drbd/drbd_receiver.c | 12 +++++-----
drivers/block/xen-blkback/blkback.c | 20 +++++++++++------
drivers/block/zram/zram_drv.c | 5 ++---
drivers/md/dm-log-writes.c | 30 +++++++++----------------
drivers/md/dm-zoned-metadata.c | 18 +++++----------
drivers/nvme/target/io-cmd-bdev.c | 9 +++-----
drivers/target/target_core_iblock.c | 5 ++---
fs/block_dev.c | 6 ++---
fs/buffer.c | 16 ++++++--------
fs/crypto/bio.c | 5 ++---
fs/direct-io.c | 6 ++---
fs/hfsplus/wrapper.c | 5 +----
fs/iomap/buffered-io.c | 12 +++++-----
fs/iomap/direct-io.c | 11 ++++------
fs/jfs/jfs_logmgr.c | 13 ++++-------
fs/jfs/jfs_metapage.c | 15 +++++--------
fs/mpage.c | 18 +++++----------
fs/nilfs2/segbuf.c | 10 ++-------
fs/ocfs2/cluster/heartbeat.c | 6 ++---
fs/xfs/xfs_bio_io.c | 7 ++----
fs/xfs/xfs_buf.c | 6 ++---
fs/zonefs/super.c | 6 ++---
include/linux/bio.h | 25 +++++++++++++++++++++
kernel/power/swap.c | 7 +++---
mm/page_io.c | 30 +++++++++++++------------
28 files changed, 151 insertions(+), 195 deletions(-)
--
2.22.1
@@ -1277,8 +1277,10 @@ static void one_flush_endio(struct bio *bio)staticvoidsubmit_one_flush(structdrbd_device*device,structissue_flush_context*ctx){-structbio*bio=bio_alloc(GFP_NOIO,0);+structblock_device*bdev=device->ldev->backing_bdev;+structbio*bio=bio_new(bdev,0,REQ_OP_FLUSH,REQ_PREFLUSH,0,GFP_NOIO);structone_flush_context*octx=kmalloc(sizeof(*octx),GFP_NOIO);+if(!bio||!octx){drbd_warn(device,"Could not allocate a bio, CANNOT ISSUE FLUSH\n");/* FIXME: what else can I do now? disconnecting or detaching
@@ -148,12 +148,11 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,return-EINVAL;/* This always succeeds since __GFP_DIRECT_RECLAIM is set. */-bio=bio_alloc(GFP_NOFS,nr_pages);+bio=bio_alloc(inode->i_sb->s_bdev,0,REQ_OP_WRITE,0,nr_pages,+GFP_NOFS);do{-bio_set_dev(bio,inode->i_sb->s_bdev);bio->bi_iter.bi_sector=pblk<<(blockbits-9);-bio_set_op_attrs(bio,REQ_OP_WRITE,0);i=0;offset=0;
@@ -2121,16 +2121,14 @@ static void lbmStartIO(struct lbuf * bp)jfs_info("lbmStartIO");-bio=bio_alloc(GFP_NOFS,1);-bio->bi_iter.bi_sector=bp->l_blkno<<(log->l2bsize-9);-bio_set_dev(bio,log->bdev);+bio=bio_new(log->bdev,bp->l_blkno<<(log->l2bsize-9),+REQ_OP_WRITE|REQ_SYNC,0,1,GFP_NOFS);bio_add_page(bio,bp->l_page,LOGPSIZE,bp->l_offset);BUG_ON(bio->bi_iter.bi_size!=LOGPSIZE);bio->bi_end_io=lbmIODone;bio->bi_private=bp;-bio->bi_opf=REQ_OP_WRITE|REQ_SYNC;/* check if journaling to disk has been disabled */if(log->no_integrity){
@@ -515,12 +515,13 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,unsignedintcs=*current_slot;structbio*bio;structpage*page;+sector_tsect=(reg->hr_start_block+cs)<<(bits-9);/* Testing has shown this allocation to take long enough under*GFP_KERNELthatthelocalnodecangetfenced.Itwouldbe*nicestifwecouldpre-allocatethesebiosandavoidthis*alltogether.*/-bio=bio_alloc(GFP_ATOMIC,16);+bio=bio_new(reg->hr_bdev,sect,op,op_flags,16,GFP_ATOMIC);if(!bio){mlog(ML_ERROR,"Could not alloc slots BIO!\n");bio=ERR_PTR(-ENOMEM);
@@ -528,11 +529,8 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,}/* Must put everything in 512 byte sectors for the bio... */-bio->bi_iter.bi_sector=(reg->hr_start_block+cs)<<(bits-9);-bio_set_dev(bio,reg->hr_bdev);bio->bi_private=wc;bio->bi_end_io=o2hb_bio_end_io;-bio_set_op_attrs(bio,op,op_flags);vec_start=(cs<<bits)%PAGE_SIZE;while(cs<max_slots){
@@ -271,13 +271,12 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,structhib_bio_batch*hb){structpage*page=virt_to_page(addr);+sector_tsect=page_off*(PAGE_SIZE>>9);structbio*bio;interror=0;-bio=bio_alloc(GFP_NOIO|__GFP_HIGH,1);-bio->bi_iter.bi_sector=page_off*(PAGE_SIZE>>9);-bio_set_dev(bio,hib_resume_bdev);-bio_set_op_attrs(bio,op,op_flags);+bio=bio_new(hib_resume_bdev,sect,op,op_flags,1,+GFP_NOIO|__GFP_HIGH);if(bio_add_page(bio,page,PAGE_SIZE,0)<PAGE_SIZE){pr_err("Adding page to bio failed at %llu\n",
From: Damien Le Moal <hidden> Date: 2021-01-28 07:25:44
On 2021/01/28 16:12, Chaitanya Kulkarni wrote:
quoted hunk
Introduce bio_new() helper and use it in blk-lib.c to allocate and
initialize various non-optional or semi-optional members of the bio
along with bio allocation done with bio_alloc(). Here we also calmp the
max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc().
Signed-off-by: Chaitanya Kulkarni <redacted>
---
block/blk-lib.c | 6 +-----
include/linux/bio.h | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 5 deletions(-)
@@ -68,25 +68,21 @@ static struct bio *mpage_bio_submit(int op, int op_flags, struct bio *bio)}staticstructbio*-mpage_alloc(structblock_device*bdev,-sector_tfirst_sector,intnr_vecs,-gfp_tgfp_flags)+mpage_alloc(structblock_device*bdev,sector_tfirst_sector,intnr_vecs,+gfp_tgfp_flags){structbio*bio;/* Restrict the given (page cache) mask for slab allocations */gfp_flags&=GFP_KERNEL;-bio=bio_alloc(gfp_flags,nr_vecs);+bio=bio_new(bdev,first_sector,0,0,nr_vecs,gfp_flags);if(bio==NULL&&(current->flags&PF_MEMALLOC)){while(!bio&&(nr_vecs/=2))-bio=bio_alloc(gfp_flags,nr_vecs);+bio=bio_new(bdev,first_sector,0,0,nr_vecs,+gfp_flags);}-if(bio){-bio_set_dev(bio,bdev);-bio->bi_iter.bi_sector=first_sector;-}returnbio;}
@@ -304,9 +300,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)gotoout;}args->bio=mpage_alloc(bdev,blocks[0]<<(blkbits-9),-min_t(int,args->nr_pages,-BIO_MAX_PAGES),-gfp);+args->nr_pages,gfp);if(args->bio==NULL)gotoconfused;}
From: Damien Le Moal <hidden> Date: 2021-01-28 07:31:06
On 2021/01/28 16:21, Damien Le Moal wrote:
On 2021/01/28 16:12, Chaitanya Kulkarni wrote:
quoted
Introduce bio_new() helper and use it in blk-lib.c to allocate and
initialize various non-optional or semi-optional members of the bio
along with bio allocation done with bio_alloc(). Here we also calmp the
max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc().
Signed-off-by: Chaitanya Kulkarni <redacted>
---
block/blk-lib.c | 6 +-----
include/linux/bio.h | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 5 deletions(-)
@@ -416,12 +416,11 @@ static int metapage_writepage(struct page *page, struct writeback_control *wbc)}len=min(xlen,(int)JFS_SBI(inode->i_sb)->nbperpage);-bio=bio_alloc(GFP_NOFS,1);-bio_set_dev(bio,inode->i_sb->s_bdev);-bio->bi_iter.bi_sector=pblock<<(inode->i_blkbits-9);+bio=bio_new(inode->i_sb->s_bdev,+pblock<<(inode->i_blkbits-9),REQ_OP_WRITE,+0,1,GFP_NOFS);bio->bi_end_io=metapage_write_end_io;bio->bi_private=page;-bio_set_op_attrs(bio,REQ_OP_WRITE,0);/* Don't call bio_add_page yet, we may add to this vec */bio_offset=offset;
@@ -1979,17 +1979,14 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)bp->l_flag|=lbmREAD;-bio=bio_alloc(GFP_NOFS,1);--bio->bi_iter.bi_sector=bp->l_blkno<<(log->l2bsize-9);-bio_set_dev(bio,log->bdev);+bio=bio_new(log->bdev,bp->l_blkno<<(log->l2bsize-9),+REQ_OP_READ,0,1,GFP_NOFS);bio_add_page(bio,bp->l_page,LOGPSIZE,bp->l_offset);BUG_ON(bio->bi_iter.bi_size!=LOGPSIZE);bio->bi_end_io=lbmIODone;bio->bi_private=bp;-bio->bi_opf=REQ_OP_READ;/*check if journaling to disk has been disabled*/if(log->no_integrity){bio->bi_iter.bi_size=0;
@@ -148,7 +148,7 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,return-EINVAL;/* This always succeeds since __GFP_DIRECT_RECLAIM is set. */-bio=bio_alloc(inode->i_sb->s_bdev,0,REQ_OP_WRITE,0,nr_pages,+bio=bio_new(inode->i_sb->s_bdev,0,REQ_OP_WRITE,0,nr_pages,GFP_NOFS);do{
@@ -1643,6 +1643,7 @@ int drbd_submit_peer_request(struct drbd_device *device,structbio*bio;structpage*page=peer_req->pages;sector_tsector=peer_req->i.sector;+structblock_device*bdev=device->ldev->backing_bdev;unsigneddata_size=peer_req->i.size;unsignedn_bios=0;unsignednr_pages=(data_size+PAGE_SIZE-1)>>PAGE_SHIFT;
@@ -1687,15 +1688,12 @@ int drbd_submit_peer_request(struct drbd_device *device,*generatedbio,butabioallocatedonbehalfofthepeer.*/next_bio:-bio=bio_alloc(GFP_NOIO,nr_pages);+bio=bio_new(bdev,sector,op,op_flags,GFP_NOIO,nr_pages);if(!bio){drbd_err(device,"submit_ee: Allocation of a bio failed (nr_pages=%u)\n",nr_pages);gotofail;}/* > peer_req->i.sector, unless this is the first bio */-bio->bi_iter.bi_sector=sector;-bio_set_dev(bio,device->ldev->backing_bdev);-bio_set_op_attrs(bio,op,op_flags);bio->bi_private=peer_req;bio->bi_end_io=drbd_peer_request_endio;
@@ -3023,12 +3023,16 @@ static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,if(test_set_buffer_req(bh)&&(op==REQ_OP_WRITE))clear_buffer_write_io_error(bh);-bio=bio_alloc(GFP_NOIO,1);+if(buffer_meta(bh))+op_flags|=REQ_META;+if(buffer_prio(bh))+op_flags|=REQ_PRIO;++bio=bio_new(bh->b_bdev,bh->b_blocknr*(bh->b_size>>9),op,+op_flags,GFP_NOIO,1);fscrypt_set_bio_crypt_ctx_bh(bio,bh,GFP_NOIO);-bio->bi_iter.bi_sector=bh->b_blocknr*(bh->b_size>>9);-bio_set_dev(bio,bh->b_bdev);bio->bi_write_hint=write_hint;bio_add_page(bio,bh->b_page,bh->b_size,bh_offset(bh));
@@ -3037,12 +3041,6 @@ static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,bio->bi_end_io=end_bio_bh_io_sync;bio->bi_private=bh;-if(buffer_meta(bh))-op_flags|=REQ_META;-if(buffer_prio(bh))-op_flags|=REQ_PRIO;-bio_set_op_attrs(bio,op,op_flags);-/* Take care of bh's that straddle the end of the device */guard_bio_eod(bio);
Create a wrapper on the tio of the bio_new() named get_new_bio() & use
it in the dispatch_rw_block_io().
p
Signed-off-by: Chaitanya Kulkarni <redacted>
---
drivers/block/xen-blkback/blkback.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
blk_next_bio() is the central function which allocates the bios for
discard, write-same, write-zeroes and zone-mgmt. The initialization of
various bio members is duplicated in disacrd, write-same, write-zeores.
In this preparation patch we add bdev, sector, op, and opf arguments to
the blk_next_bio() to reduce the duplication.
In the next patch we introduce bio_new(), this prepration patch allows
us to call it inside blk_next_bio().
Signed-off-by: Chaitanya Kulkarni <redacted>
---
block/blk-lib.c | 36 +++++++++++++++---------------------
block/blk-zoned.c | 4 +---
block/blk.h | 5 +++--
3 files changed, 19 insertions(+), 26 deletions(-)
@@ -19,6 +21,10 @@ struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp)submit_bio(bio);}+new->bi_iter.bi_sector=sect;+bio_set_dev(new,bdev);+bio_set_op_attrs(new,op,opf);+returnnew;}
@@ -94,11 +100,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,WARN_ON_ONCE((req_sects<<9)>UINT_MAX);-bio=blk_next_bio(bio,0,gfp_mask);-bio->bi_iter.bi_sector=sector;-bio_set_dev(bio,bdev);-bio_set_op_attrs(bio,op,0);-+bio=blk_next_bio(bio,bdev,sector,op,0,0,gfp_mask);bio->bi_iter.bi_size=req_sects<<9;sector+=req_sects;nr_sects-=req_sects;
Introduce bio_new() helper and use it in blk-lib.c to allocate and
initialize various non-optional or semi-optional members of the bio
along with bio allocation done with bio_alloc(). Here we also calmp the
max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc().
Signed-off-by: Chaitanya Kulkarni <redacted>
---
block/blk-lib.c | 6 +-----
include/linux/bio.h | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 5 deletions(-)
@@ -271,13 +271,12 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,structhib_bio_batch*hb){structpage*page=virt_to_page(addr);+sector_tsect=page_off*(PAGE_SIZE>>9);structbio*bio;interror=0;-bio=bio_alloc(GFP_NOIO|__GFP_HIGH,1);-bio->bi_iter.bi_sector=page_off*(PAGE_SIZE>>9);-bio_set_dev(bio,hib_resume_bdev);-bio_set_op_attrs(bio,op,op_flags);+bio=bio_new(hib_resume_bdev,sect,op,op_flags,1,+GFP_NOIO|__GFP_HIGH);if(bio_add_page(bio,page,PAGE_SIZE,0)<PAGE_SIZE){pr_err("Adding page to bio failed at %llu\n",--
@@ -515,12 +515,13 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,unsignedintcs=*current_slot;structbio*bio;structpage*page;+sector_tsect=(reg->hr_start_block+cs)<<(bits-9);/* Testing has shown this allocation to take long enough under*GFP_KERNELthatthelocalnodecangetfenced.Itwouldbe*nicestifwecouldpre-allocatethesebiosandavoidthis*alltogether.*/-bio=bio_alloc(GFP_ATOMIC,16);+bio=bio_new(reg->hr_bdev,sect,op,op_flags,16,GFP_ATOMIC);if(!bio){mlog(ML_ERROR,"Could not alloc slots BIO!\n");bio=ERR_PTR(-ENOMEM);
@@ -528,11 +529,8 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,}/* Must put everything in 512 byte sectors for the bio... */-bio->bi_iter.bi_sector=(reg->hr_start_block+cs)<<(bits-9);-bio_set_dev(bio,reg->hr_bdev);bio->bi_private=wc;bio->bi_end_io=o2hb_bio_end_io;-bio_set_op_attrs(bio,op,op_flags);vec_start=(cs<<bits)%PAGE_SIZE;while(cs<max_slots){
@@ -1979,17 +1979,14 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)bp->l_flag|=lbmREAD;-bio=bio_alloc(GFP_NOFS,1);--bio->bi_iter.bi_sector=bp->l_blkno<<(log->l2bsize-9);-bio_set_dev(bio,log->bdev);+bio=bio_new(log->bdev,bp->l_blkno<<(log->l2bsize-9),+REQ_OP_READ,0,1,GFP_NOFS);bio_add_page(bio,bp->l_page,LOGPSIZE,bp->l_offset);BUG_ON(bio->bi_iter.bi_size!=LOGPSIZE);bio->bi_end_io=lbmIODone;bio->bi_private=bp;-bio->bi_opf=REQ_OP_READ;/*check if journaling to disk has been disabled*/if(log->no_integrity){bio->bi_iter.bi_size=0;
op == 0? It seems a little odd to me that we'd set the field to zero
and then construct bi_opf later.
It also strikes me as a little strange that bi_opf is combined from the
third and fourth parameters, but maybe some day you'll want to do some
parameter verification on debug kernels or something...?
--D
@@ -271,13 +271,12 @@ static int hib_submit_io(int op, int op_flags, pgoff_t page_off, void *addr,structhib_bio_batch*hb){structpage*page=virt_to_page(addr);+sector_tsect=page_off*(PAGE_SIZE>>9);structbio*bio;interror=0;-bio=bio_alloc(GFP_NOIO|__GFP_HIGH,1);-bio->bi_iter.bi_sector=page_off*(PAGE_SIZE>>9);-bio_set_dev(bio,hib_resume_bdev);-bio_set_op_attrs(bio,op,op_flags);+bio=bio_new(hib_resume_bdev,sect,op,op_flags,1,+GFP_NOIO|__GFP_HIGH);
C function with 6 arguments... dunno. Old version looks comparable or
even more readable...
Best regards,
Pavel
The library functions that are in the kernel tree which are used
in different file-systems and fabrics drivers do take 6 arguments.
Plus what is the point of duplicating code for mandatory
parameters all over the kernel ?