On 11/21/18 1:41 AM, Christoph Hellwig wrote:
> Hi all,
>
> you in the To list maintain or wrote SCSI drivers that set the
> DISABLE_CLUSTERING flag, which basically disable merges of any
> bio segments. We already have the actual max_segment size limit
> to say which length a segment should have, independent of merged
> or originally created, so this limit generally should rarely if
> ever be required, and mostly is an old cut an paste error.
>
> Can you go over your drivers and check if it could be removed?
>
I have tested changing this to ENABLE_CLUSTERING in iscsi_tcp.c. This
code is used for both the iscsi initiator and the target.
On the initiator side, there is no problem with enabling clustering of bios.
But on the target side, the code seems to assume that IOs do not cross
page boundaries, resulting in WARN_ON messages and failed IO when
ENABLE_CLUSTERING is set:
> [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0
> [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01 f4 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2 <0f> 0b 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25
> [ 78.644596] RSP: 0018:ffffa04b41eefbe0 EFLAGS: 00010206
> [ 78.644597] RAX: fffff7acc40de180 RBX: ffffa04b41eefd80 RCX: 002ffff80000a014
> [ 78.644598] RDX: 0000000000002000 RSI: 0000000000001000 RDI: fffff7acc0000000
> [ 78.644598] RBP: fffff7acc40de180 R08: ffff8f1703786000 R09: 0000000000002000
> [ 78.644599] R10: 00000000000002c0 R11: ffff8f16b3f31400 R12: 0000000000000000
> [ 78.644600] R13: 0000000000002000 R14: 0000000000000008 R15: 0000000000003800
> [ 78.644601] FS: 0000000000000000(0000) GS:ffff8f1739c00000(0000) knlGS:0000000000000000
> [ 78.644601] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 78.644602] CR2: 00007f0f54772000 CR3: 000000012e670004 CR4: 00000000003606f0
> [ 78.644624] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 78.644625] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 78.644625] Call Trace:
> [ 78.644631] skb_copy_datagram_iter+0x16f/0x270
> [ 78.644633] tcp_recvmsg+0x223/0xc30
> [ 78.644637] inet_recvmsg+0x4b/0xe0
> [ 78.644644] iscsit_do_rx_data.constprop.9+0x89/0x110 [iscsi_target_mod]
> [ 78.644650] rx_data+0x58/0x70 [iscsi_target_mod]
> [ 78.644654] iscsit_get_rx_pdu+0xa7b/0xd20 [iscsi_target_mod]
> [ 78.644657] ? preempt_count_sub+0x43/0x50
> [ 78.644659] ? _raw_spin_unlock_irq+0x22/0x40
> [ 78.644663] ? iscsi_target_tx_thread+0x1d0/0x1d0 [iscsi_target_mod]
> [ 78.644667] ? iscsi_target_rx_thread+0x71/0xd0 [iscsi_target_mod]
> [ 78.644670] iscsi_target_rx_thread+0x71/0xd0 [iscsi_target_mod]
> [ 78.644672] kthread+0x116/0x130
> [ 78.644673] ? kthread_create_worker_on_cpu+0x40/0x40
> [ 78.644675] ret_from_fork+0x24/0x50
> [ 78.644678] ---[ end trace a0d6d5ab0e8625ee ]---
>
The problem seems to be in iov_iter.c:copy_page_to_iter(), where it
first calls page_copy_sane(), which makes sure the copy does not cross a
page boundary:
> static inline bool page_copy_sane(struct page *page, size_t offset, size_t n)
> {
> struct page *head = compound_head(page);
> size_t v = n + offset + page_address(page) - page_address(head);
>
> if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head))))
> return true;
> WARN_ON(1);
> return false;
> }
I will submit a separate BUG email about this to the linux-scsi and
target-devel mailing lists, since it's not clear to me how to fix this.
In the mean time, the iscsi_tcp.c file still needs DISABLE_CLUSTERING.
--
Lee
"Deviants will be sacrificed to ensure group solidarity."
