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 lars.ellenb...@linbit.com 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 lars.ellenb...@linbit.com 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 Mon, Jun 23, 2014 at 03:37:03PM -0400, Martin K. Petersen wrote: Lars == Lars Ellenberg lars.ellenb...@linbit.com 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 lars.ellenb...@linbit.com 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 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 lars.ellenb...@linbit.com 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)
Lars == Lars Ellenberg lars.ellenb...@linbit.com 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)
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 lars.ellenb...@linbit.com 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 Thu, Jun 19, 2014 at 11:08:22PM -0400, Martin K. Petersen wrote: Stefan == Stefan Priebe - Profihost AG s.pri...@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 [8128105c] 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)
Lars == Lars Ellenberg lars.ellenb...@linbit.com 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 Fri, Jun 20, 2014 at 12:49:39PM -0400, Martin K. Petersen wrote:
Lars == Lars Ellenberg lars.ellenb...@linbit.com 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)
Stefan == Stefan Priebe - Profihost AG s.pri...@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 [8128105c] 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
