Re: [PATCH v2] block: Improve limiting the bio size

[Changheun Lee <nanich.lee@samsung.com>]

> On 4/26/21 5:28 PM, Changheun Lee wrote:
> > __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