Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
> "Lars" == Lars Ellenberg writes: Lars> Because the way we do some internal bookkeeping, we announce a max Lars> discard of 4 MiB. Yikes! Lars> Similar for the WRITE_SAME interface (which we do not properly Lars> support on the DRBD protocol level yet -- backward compatibility Lars> concerns -- but intend to support "soon"). Yeah, WRITE SAME can take a long time to complete too. However, we're typically issuing them 32 megs at a time. Lars> If we only have a synchronous interface, we will probably have to Lars> either add some "async wrapper", or defer such submissions to Lars> worker threads. I'd prefer to have an async submit path. OK, I'll chew on it. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
On Tue, Jun 24, 2014 at 07:11:47PM -0400, Martin K. Petersen wrote: > > "Lars" == Lars Ellenberg writes: > > Lars> We are receiving (from network) and submitting (to lower level IO > Lars> stack) in the same context and would like the submit to be async. > > Lars> Do you intend to provide an asynchronous interface? > > I guess we can look into that if there is a need. > > Do different clients share that context? I.e. does a synchronous discard > block other clients from accessing the drbd server? Uhm, it's not like exactly like that, really. Because the way we do some internal bookkeeping, we announce a max discard of 4 MiB. So if some user on the "active" (Primary) DRBD does large discards, you will end up submitting lots of bios, and these are async. Bios are the entry point to DRBD. So DRBD ships these discard-bios over to the peer, which then right now submits them as bios, again async. So we do some pipelining, may have a number of discard bios in flight, and effectively the latency will be increased by something in the order of the network rtt. If we now have to use the synchronous interface on the peer for each discard bio, there is no longer any pipelining, and the overall latency of a single "user" level discard (that ends up doing many discard bios) will noticeably increase. Also, since the "receiver" is blocked in "submit", we cannot meanwhile interleave other, "normal" BIOs, so a larger discard will block all write (and depending on configuration and current state, also read) within that DRBD resource (which again may be one or more DRBD minor devices or "volumes"). I don't have real-life numbers on how much that may hurt. Similar for the WRITE_SAME interface (which we do not properly support on the DRBD protocol level yet -- backward compatibility concerns -- but intend to support "soon"). If we only have a synchronous interface, we will probably have to either add some "async wrapper", or defer such submissions to worker threads. I'd prefer to have an async submit path. Lars -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
> "Lars" == Lars Ellenberg writes: Lars> We are receiving (from network) and submitting (to lower level IO Lars> stack) in the same context and would like the submit to be async. Lars> Do you intend to provide an asynchronous interface? I guess we can look into that if there is a need. Do different clients share that context? I.e. does a synchronous discard block other clients from accessing the drbd server? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
On Mon, Jun 23, 2014 at 03:37:03PM -0400, Martin K. Petersen wrote: > > "Lars" == Lars Ellenberg writes: > > Lars, > > Thanks for fixing this. > > I'd still like to see you use the lib call instead like you do for > zeroout. I have some patches in the pipeline for multi-range discard > support and things are going to break for drbd if you manually roll > bios. Okay, thanks for the heads up. I think we just did this so we would not have to use a synchronous interface there. We are receiving (from network) and submitting (to lower level IO stack) in the same context and would like the submit to be async. Do you intend to provide an asynchronous interface? Lars -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
> "Lars" == Lars Ellenberg writes: Lars, Thanks for fixing this. I'd still like to see you use the lib call instead like you do for zeroout. I have some patches in the pipeline for multi-range discard support and things are going to break for drbd if you manually roll bios. Lars> linux upstream DRBD also does blk_queue_max_write_same_sectors(q, Lars> 0) and blk_queue_max_discard_sectors(q, DRBD_MAX_DISCARD_SECTORS) Great! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
On Sat, Jun 21, 2014 at 07:48:22PM +0200, Stefan Priebe wrote: > Hi Lars, > Am 20.06.2014 20:29, schrieb Lars Ellenberg: > >On Fri, Jun 20, 2014 at 12:49:39PM -0400, Martin K. Petersen wrote: > >>>"Lars" == Lars Ellenberg writes: > >> > >>Lars, > >> > >>Lars> Any bio allocated that will be passed down with REQ_DISCARD has to > >>Lars> be allocated with nr_iovecs = 1 (at least), even though it must > >>Lars> not contain any bio_vec payload. > >> > >>True. Although the correct answer is: Any discard request must be issued > >>by blkdev_issue_discard(). That's the interface. > >> > >>The hacks we do to carry the information inside the bio constitute an > >>internal interface that is subject to change (it is just about to, > >>actually). > >> > >>Lars> Though DRBD in 3.10 is not supposed to accept discard requests. > >>Lars> So I'm not sure how it manages to pass them down? > > your're absolutely right - a collegue installed drbd 8.4.4 as a > module. I didn't knew that. Sorry. That is (again) incorrect/incomplete. Your original post: > while using vanilla 3.10.44 with drbd on top of a md raid1. ... > CPU: 0 PID: 636 Comm: md124_raid1 Tainted: G O 3.10.41+76-ph #1 > Modules linked in: ... drbd ... So it's not vanilla, its not 3.10.44, and its not 3.10.41 either, and its not even a "clean" external module. But its "something" based on 3.10.41, where you added your own patches or "backports", and now complain to the upstream maintainers that it explodes, and don't bother to tell them that it is modified code. > So your attached patch will fix it? No. For the out-of-tree module it is fixed. You just need to upgrade. This is for the 3.16-rc1 and later in-tree DRBD, where this fix apparently slipt through when preparing the pull request. It has not even been in a released mainline kernel yet. But thanks anyways for reporting it, it may have ended up unnoticed in 3.16. Lars -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
Hi Lars,
Am 20.06.2014 20:29, schrieb Lars Ellenberg:
On Fri, Jun 20, 2014 at 12:49:39PM -0400, Martin K. Petersen wrote:
"Lars" == Lars Ellenberg writes:
Lars,
Lars> Any bio allocated that will be passed down with REQ_DISCARD has to
Lars> be allocated with nr_iovecs = 1 (at least), even though it must
Lars> not contain any bio_vec payload.
True. Although the correct answer is: Any discard request must be issued
by blkdev_issue_discard(). That's the interface.
The hacks we do to carry the information inside the bio constitute an
internal interface that is subject to change (it is just about to,
actually).
Lars> Though DRBD in 3.10 is not supposed to accept discard requests.
Lars> So I'm not sure how it manages to pass them down?
your're absolutely right - a collegue installed drbd 8.4.4 as a module.
I didn't knew that. Sorry.
So your attached patch will fix it?
drbd_receiver.c:
static unsigned long wire_flags_to_bio(struct drbd_conf *mdev, u32 dpf)
{
return (dpf & DP_RW_SYNC ? REQ_SYNC : 0) |
(dpf & DP_FUA ? REQ_FUA : 0) |
(dpf & DP_FLUSH ? REQ_FLUSH : 0) |
(dpf & DP_DISCARD ? REQ_DISCARD : 0);
}
[...]
/* mirrored write */
static int receive_Data(struct drbd_tconn *tconn, struct packet_info
*pi)
{
[...]
dp_flags = be32_to_cpu(p->dp_flags);
rw |= wire_flags_to_bio(mdev, dp_flags);
[...]
That's pretty busticated. I suggest you simply remove REQ_DISCARD from
that helper for now.
It's also a good idea to disable discard and write same on the client
side when you set up the request queue:
blk_queue_max_discard_sectors(q, 0);
blk_queue_max_write_same_sectors(q, 0);
Our main development still happens out-of-tree,
trying to be compatible to a large range of kernel versions.
linux upstream DRBD is supposed to handle discards "correctly"
(even though not using the proper interface blkdev_issue_discard).
But it does not, because one fix apparently slipped through
when preparing the pull request.
So linux upstream needs:
diff --git a/drivers/block/drbd/drbd_receiver.c
b/drivers/block/drbd/drbd_receiver.c
index b6c8aaf..5b17ec8 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -1337,8 +1337,11 @@ int drbd_submit_peer_request(struct drbd_device *device,
return 0;
}
+ /* Discards don't have any payload.
+* But the scsi layer still expects a bio_vec it can use internally,
+* see sd_setup_discard_cmnd() and blk_add_request_payload(). */
if (peer_req->flags & EE_IS_TRIM)
- nr_pages = 0; /* discards don't have any payload. */
+ nr_pages = 1;
/* In most cases, we will only need one bio. But in case the lower
* level restrictions happen to be different at this offset on this
I'll prepare a proper patch with commit message later.
linux upstream DRBD also does blk_queue_max_write_same_sectors(q, 0)
and blk_queue_max_discard_sectors(q, DRBD_MAX_DISCARD_SECTORS)
---
For linux 3.10, things are different.
DRBD in linux 3.10 does not set QUEUE_FLAG_DISCARD,
and does not announce discard capabilities in any other way,
even though it already contains some preparation steps
(those pieces your grep foo managed to find above...)
DRBD does a handshake, and if there is no discard capability announced,
the peer is supposed to never send discards (and stop announcing them
on his side), even if the peer's DRBD version already supports
and announces discard capabilities.
So I'm still not really seeing how discard requests would be issued
by that version of DRBD.
The local submit path should not allow them (no QUEUE_FLAG_DISCARD set)
and the remote submit path should not allow them either,
for the same reason, and because the DRBD handshake does not allow them.
So my current guess would be that Stefan prepared a 3.10.44
+ "upstream DRBD", but unfortunately not upstream enough?
Stefan, please give more details how to trigger this,
with which exact DRBD versions on the peers, and what action.
Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
On Fri, Jun 20, 2014 at 12:49:39PM -0400, Martin K. Petersen wrote:
> > "Lars" == Lars Ellenberg writes:
>
> Lars,
>
> Lars> Any bio allocated that will be passed down with REQ_DISCARD has to
> Lars> be allocated with nr_iovecs = 1 (at least), even though it must
> Lars> not contain any bio_vec payload.
>
> True. Although the correct answer is: Any discard request must be issued
> by blkdev_issue_discard(). That's the interface.
>
> The hacks we do to carry the information inside the bio constitute an
> internal interface that is subject to change (it is just about to,
> actually).
>
> Lars> Though DRBD in 3.10 is not supposed to accept discard requests.
> Lars> So I'm not sure how it manages to pass them down?
>
> drbd_receiver.c:
>
> static unsigned long wire_flags_to_bio(struct drbd_conf *mdev, u32 dpf)
> {
> return (dpf & DP_RW_SYNC ? REQ_SYNC : 0) |
> (dpf & DP_FUA ? REQ_FUA : 0) |
> (dpf & DP_FLUSH ? REQ_FLUSH : 0) |
> (dpf & DP_DISCARD ? REQ_DISCARD : 0);
> }
>
> [...]
>
> /* mirrored write */
> static int receive_Data(struct drbd_tconn *tconn, struct packet_info
> *pi)
> {
> [...]
>dp_flags = be32_to_cpu(p->dp_flags);
>rw |= wire_flags_to_bio(mdev, dp_flags);
> [...]
>
> That's pretty busticated. I suggest you simply remove REQ_DISCARD from
> that helper for now.
>
> It's also a good idea to disable discard and write same on the client
> side when you set up the request queue:
>
> blk_queue_max_discard_sectors(q, 0);
> blk_queue_max_write_same_sectors(q, 0);
Our main development still happens out-of-tree,
trying to be compatible to a large range of kernel versions.
linux upstream DRBD is supposed to handle discards "correctly"
(even though not using the proper interface blkdev_issue_discard).
But it does not, because one fix apparently slipped through
when preparing the pull request.
So linux upstream needs:
diff --git a/drivers/block/drbd/drbd_receiver.c
b/drivers/block/drbd/drbd_receiver.c
index b6c8aaf..5b17ec8 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -1337,8 +1337,11 @@ int drbd_submit_peer_request(struct drbd_device *device,
return 0;
}
+ /* Discards don't have any payload.
+* But the scsi layer still expects a bio_vec it can use internally,
+* see sd_setup_discard_cmnd() and blk_add_request_payload(). */
if (peer_req->flags & EE_IS_TRIM)
- nr_pages = 0; /* discards don't have any payload. */
+ nr_pages = 1;
/* In most cases, we will only need one bio. But in case the lower
* level restrictions happen to be different at this offset on this
I'll prepare a proper patch with commit message later.
linux upstream DRBD also does blk_queue_max_write_same_sectors(q, 0)
and blk_queue_max_discard_sectors(q, DRBD_MAX_DISCARD_SECTORS)
---
For linux 3.10, things are different.
DRBD in linux 3.10 does not set QUEUE_FLAG_DISCARD,
and does not announce discard capabilities in any other way,
even though it already contains some preparation steps
(those pieces your grep foo managed to find above...)
DRBD does a handshake, and if there is no discard capability announced,
the peer is supposed to never send discards (and stop announcing them
on his side), even if the peer's DRBD version already supports
and announces discard capabilities.
So I'm still not really seeing how discard requests would be issued
by that version of DRBD.
The local submit path should not allow them (no QUEUE_FLAG_DISCARD set)
and the remote submit path should not allow them either,
for the same reason, and because the DRBD handshake does not allow them.
So my current guess would be that Stefan prepared a 3.10.44
+ "upstream DRBD", but unfortunately not upstream enough?
Stefan, please give more details how to trigger this,
with which exact DRBD versions on the peers, and what action.
Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
> "Lars" == Lars Ellenberg writes:
Lars,
Lars> Any bio allocated that will be passed down with REQ_DISCARD has to
Lars> be allocated with nr_iovecs = 1 (at least), even though it must
Lars> not contain any bio_vec payload.
True. Although the correct answer is: Any discard request must be issued
by blkdev_issue_discard(). That's the interface.
The hacks we do to carry the information inside the bio constitute an
internal interface that is subject to change (it is just about to,
actually).
Lars> Though DRBD in 3.10 is not supposed to accept discard requests.
Lars> So I'm not sure how it manages to pass them down?
drbd_receiver.c:
static unsigned long wire_flags_to_bio(struct drbd_conf *mdev, u32 dpf)
{
return (dpf & DP_RW_SYNC ? REQ_SYNC : 0) |
(dpf & DP_FUA ? REQ_FUA : 0) |
(dpf & DP_FLUSH ? REQ_FLUSH : 0) |
(dpf & DP_DISCARD ? REQ_DISCARD : 0);
}
[...]
/* mirrored write */
static int receive_Data(struct drbd_tconn *tconn, struct packet_info
*pi)
{
[...]
dp_flags = be32_to_cpu(p->dp_flags);
rw |= wire_flags_to_bio(mdev, dp_flags);
[...]
That's pretty busticated. I suggest you simply remove REQ_DISCARD from
that helper for now.
It's also a good idea to disable discard and write same on the client
side when you set up the request queue:
blk_queue_max_discard_sectors(q, 0);
blk_queue_max_write_same_sectors(q, 0);
--
Martin K. Petersen Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
On Thu, Jun 19, 2014 at 11:08:22PM -0400, Martin K. Petersen wrote: > > "Stefan" == Stefan Priebe <- Profihost AG > > > writes: > > Stefan> Hi, while using vanilla 3.10.44 with drbd on top of a md raid1. > > Stefan> I'm pretty often hitting the followin kernel bug. > > Stefan> [] blk_add_request_payload+0xc/0x90 > > That's really messed up. This means we received a request with no bio. No. That means you received a bio that has been allocated with bio_alloc(... , nr_iovecs = 0); thus bio->bi_io_vec is NULL, but blk_add_request_payload insists on using it anyways. Even though it also requires that bio->bi_vcnt = 0 (because it then explicitly sets that to 1). This is some subtlety with discard requests that has bitten some stacking drivers now. Any bio allocated that will be passed down with REQ_DISCARD has to be allocated with nr_iovecs = 1 (at least), even though it must not contain any bio_vec payload. Though DRBD in 3.10 is not supposed to accept discard requests. So I'm not sure how it manages to pass them down? Lars -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
> "Stefan" == Stefan Priebe <- Profihost AG > writes: Stefan> Hi, while using vanilla 3.10.44 with drbd on top of a md raid1. Stefan> I'm pretty often hitting the followin kernel bug. Stefan> [] blk_add_request_payload+0xc/0x90 That's really messed up. This means we received a request with no bio. Does this happen with later kernels? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
sd_setup_discard_cmnd: BUG: unable to handle kernel NULL pointer dereference at (null)
Hi, while using vanilla 3.10.44 with drbd on top of a md raid1. I'm pretty often hitting the followin kernel bug. It reminds me of: http://lists.openwall.net/linux-kernel/2014/02/19/428 But i don't use bcache. [sched_delayed] sched: RT throttling activated BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] blk_add_request_payload+0xc/0x90 PGD 0 Oops: 0002 [#1] SMP Modules linked in: vhost_net tun macvtap macvlan dlm sctp netconsole drbd lru_cache xt_multiport iptable_filter ip_tables x_tables iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi nfsd auth_rpcgss oid_registry bonding ext2 8021q garp fuse acpi_cpufreq mperf coretemp kvm_intel kvm microcode i2c_i801 i7core_edac edac_core button dm_mod raid1 md_mod usb_storage ohci_hcd usbhid sg sd_mod uhci_hcd ehci_pci ehci_hcd ahci libahci usbcore ixgbe(O) usb_common igb i2c_algo_bit mpt2sas i2c_core raid_class ptp scsi_transport_sas pps_core CPU: 0 PID: 636 Comm: md124_raid1 Tainted: G O 3.10.41+76-ph #1 Hardware name: Supermicro X8DT3/X8DT3, BIOS 2.1 03/17/2012 task: 880811e16400 ti: 88081324c000 task.ti: 88081324c000 RIP: 0010:[] [] blk_add_request_payload+0xc/0x90 RSP: 0018:88081324dac8 EFLAGS: 00010086 RAX: 88080b686b40 RBX: 88080af19028 RCX: RDX: 0018 RSI: ea00202b7200 RDI: 88080af19028 RBP: 88081324dac8 R08: ea00202b7200 R09: 006f4a62 R10: 243d R11: R12: 8808145b9400 R13: 8810139a6000 R14: 0440 R15: 88080adc8000 FS: () GS:88081fc0() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: CR3: 01a0b000 CR4: 07f0 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400Stack: 88081324db18 a01b515f ea00202b7200 0b637800 88081324db18 88080af19028 880812f18818 88081324dca8 8810139a6000 8808145b9800 88081324db88 a01b5318 Call Trace: [] sd_setup_discard_cmnd+0x13f/0x260 [sd_mod] [] sd_prep_fn+0x98/0xbb0 [sd_mod] [] ? merge+0x50/0xa0 [] ? blk_start_plug+0x50/0x50 [] blk_peek_request+0x132/0x250 [] scsi_request_fn+0x46/0x520 [] __blk_run_queue+0x32/0x40 [] queue_unplugged+0x34/0xb0 [] blk_flush_plug_list+0x183/0x220 [] blk_finish_plug+0x13/0x50 [] raid1d+0x706/0xee0 [raid1] [] ? try_to_del_timer_sync+0x4a/0x60 [] ? del_timer_sync+0x52/0x60 [] ? usleep_range+0x40/0x40 [] md_thread+0x11d/0x170 [md_mod] [] ? finish_wait+0x80/0x80 Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
