Re: [PATCH v2] block: Improve limiting the bio size
From: Changheun Lee <hidden>
Date: 2021-04-27 04:30:30
On 4/26/21 5:28 PM, Changheun Lee wrote:quoted
__device_add_disk() do not call bio_max_size(). I just imagined bio operation on disk without request queue. Disk can be added without queue via device_add_disk_no_queue_reg(). It might be my miss-understood about it. I didn't check bio operation is possible on disk without request queue yet.Inside __device_add_disk() I found the following: WARN_ON_ONCE(!blk_get_queue(disk->queue)); I'm not sure how that could work without initializing disk->queue first? Thanks, Bart.
I think so. But I just asked whether queue should be checked, or not.
Anyway we should determine which pointer variables must be checked in
bio_max_size(). Candidates are bio->bi_bdev, bi_bdev->bd_disk, and
bd_disk->queue I think.
Actually I checked "bio->bi_disk" at first as below. It works well.
unsigned int bio_max_size(struct bio *bio)
{
struct request_queue *q;
if (!bio->bi_disk)
return UINT_MAX;
q = bio->bi_disk->queue;
return q->limits.bio_max_bytes;
}
But I removed bi_disk check condition after applying of Christoph's patch.
Because I got a feedback that is not needed more.
Without bi_disk checking, booting was failed like now. call stack was below.
[ 13.341949] pc : __bio_add_pc_page+0x120/0x190
[ 13.341951] lr : bio_copy_kern+0xf8/0x20c
[ 13.341953] sp : ffffffc012e23950
[ 13.341954] x29: ffffffc012e23950 x28: 0000000000001000
[ 13.341955] x27: ffffffff209fa140 x26: 0000000000000200
[ 13.341957] x25: ffffff888d151400 x24: ffffff8919479830
[ 13.341959] x23: ffffff8919479830 x22: ffffffff209fa140
[ 13.341960] x21: 0000000000000000 x20: ffffff882db2f300
[ 13.341962] x19: 0000000000000200 x18: ffffffc012e65048
[ 13.341963] x17: 0000000000000000 x16: 00000000000002fe
[ 13.341965] x15: 0000000000000000 x14: 0000000005bae400
[ 13.341966] x13: 00000000000019f2 x12: ffffffc912fb2000
[ 13.341968] x11: 0000000000000000 x10: 000000000002784d
[ 13.341970] x9 : 0000000000000000 x8 : 0000000000000000
[ 13.341971] x7 : 0000000000000000 x6 : 000000000000003f
[ 13.341973] x5 : ffffffc012e23994 x4 : 0000000000000000
[ 13.341974] x3 : 0000000000000200 x2 : ffffffff209fa140
[ 13.341976] x1 : ffffff882db2f300 x0 : ffffff8919479830
[ 13.341977] Call trace:
[ 13.341979] __bio_add_pc_page+0x120/0x190
[ 13.341981] bio_copy_kern+0xf8/0x20c
[ 13.341984] blk_rq_map_kern+0xa4/0x154
[ 13.341985] __scsi_execute+0x88/0x1c0
[ 13.341988] srpmb_scsi_ioctl+0x264/0x444 [scsi_srpmb]
[ 13.341990] srpmb_worker+0x134/0x5f8 [scsi_srpmb]
[ 13.341992] process_one_work+0x2f8/0x5b8
[ 13.341994] worker_thread+0x28c/0x518
[ 13.341996] kthread+0x16c/0x17c
[ 13.341998] ret_from_fork+0x10/0x18
I think "bi_bdev->bd_disk" checking might be needed too.
Thanks,
Changheun Lee