Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
> > When performing the UNMAPS, maybe setting timeout to something > > really big, and at the same time also dropping queue-depth to something > > really small so that the guest kernel will not be able to send so > > many UNMAPs concurrently. > > How is this relevant to the fact, that i can't even work with SSH any > longer with paolo's patch while UNMAPs are running? With my patchset > you can still work on the machine. It is relevant because if you tweak the timeouts you will not hit the cancel at all. I would like to get a trace of the commands that are sent, so that we can attack the problem in the guest kernel. For example, if it's sending UNMAPs that span 100GB or so, we could enforce a limit in the guest kernel on the maximum number of discarded blocks per UNMAP. But if the problem is the _number_ of UNMAPs rather than the size, it would need a different heuristic. Paolo
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Am 21.08.2012 00:36, schrieb ronnie sahlberg: On Mon, Aug 20, 2012 at 6:12 PM, Stefan Priebe - Profihost AG wrote: Hi Ronnie, Am 20.08.2012 10:08, schrieb Paolo Bonzini: That's because the "big QEMU lock" is held by the thread that called qemu_aio_cancel. and i also see no cancellation message in kernel log. And that's because the UNMAP actually ultimately succeeds. You'll probably see soft lockup messages though. The solution here is to bump the timeout of the UNMAP command (either in the kernel or in libiscsi, I didn't really understand who's at fault). What's your suggestion / idea about that? There are no timeouts in libiscsi itself. But you can probably tweak it through the guest kernel : $ cat /sys/bus/scsi/devices/0\:0\:0\:0/timeout 30 should be the default scsi timeout for this device, and $ cat /sys/bus/scsi/devices/0\:0\:0\:0/queue_depth 31 would be how many concurrent i/o that the guest will drive to the device. When performing the UNMAPS, maybe setting timeout to something really big, and at the same time also dropping queue-depth to something really small so that the guest kernel will not be able to send so many UNMAPs concurrently. How is this relevant to the fact, that i can't even work with SSH any longer with paolo's patch while UNMAPs are running? With my patchset you can still work on the machine. Greets, Stefan
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
On Mon, Aug 20, 2012 at 6:12 PM, Stefan Priebe - Profihost AG wrote: > Hi Ronnie, > > Am 20.08.2012 10:08, schrieb Paolo Bonzini: > >> That's because the "big QEMU lock" is held by the thread that called >> qemu_aio_cancel. >> >>> and i also see >>> no cancellation message in kernel log. >> >> >> And that's because the UNMAP actually ultimately succeeds. You'll >> probably see soft lockup messages though. >> >> The solution here is to bump the timeout of the UNMAP command (either in >> the kernel or in libiscsi, I didn't really understand who's at fault). > > > What's your suggestion / idea about that? > There are no timeouts in libiscsi itself. But you can probably tweak it through the guest kernel : $ cat /sys/bus/scsi/devices/0\:0\:0\:0/timeout 30 should be the default scsi timeout for this device, and $ cat /sys/bus/scsi/devices/0\:0\:0\:0/queue_depth 31 would be how many concurrent i/o that the guest will drive to the device. When performing the UNMAPS, maybe setting timeout to something really big, and at the same time also dropping queue-depth to something really small so that the guest kernel will not be able to send so many UNMAPs concurrently. ronnie s
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Hi Ronnie, Am 20.08.2012 10:08, schrieb Paolo Bonzini: That's because the "big QEMU lock" is held by the thread that called qemu_aio_cancel. and i also see no cancellation message in kernel log. And that's because the UNMAP actually ultimately succeeds. You'll probably see soft lockup messages though. The solution here is to bump the timeout of the UNMAP command (either in the kernel or in libiscsi, I didn't really understand who's at fault). What's your suggestion / idea about that? Greets, Stefan
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Il 20/08/2012 09:34, Stefan Priebe - Profihost AG ha scritto: >>> Booting works fine now. But the VM starts to hang after trying to unmap >>> large regions. No segfault or so just not reacting anymore. >> >> This is expected; unfortunately cancellation right now is a synchronous >> operation in the block layer. SCSI is the first big user of >> cancellation, and it would indeed benefit from asynchronous cancellation. >> >> Without these three patches, you risk corruption in case the following >> happens: >> >> qemu target >>--- >> send unmap > >> cancel unmap --> >> send write > >> < complete write >> >> < complete unmap >> < cancellation done (unmap complete) > > mhm OK that makes sense. But i cannot even login via SSH That's because the "big QEMU lock" is held by the thread that called qemu_aio_cancel. > and i also see > no cancellation message in kernel log. And that's because the UNMAP actually ultimately succeeds. You'll probably see soft lockup messages though. The solution here is to bump the timeout of the UNMAP command (either in the kernel or in libiscsi, I didn't really understand who's at fault). Paolo
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Am 20.08.2012 09:22, schrieb Paolo Bonzini: Il 19/08/2012 21:22, Stefan Priebe - Profihost AG ha scritto: No problem, my fault---I'm just back and I haven't really started again all my stuff, so the patch was not tested. This should fix it, though. Booting works fine now. But the VM starts to hang after trying to unmap large regions. No segfault or so just not reacting anymore. This is expected; unfortunately cancellation right now is a synchronous operation in the block layer. SCSI is the first big user of cancellation, and it would indeed benefit from asynchronous cancellation. Without these three patches, you risk corruption in case the following happens: qemu target --- send unmap > cancel unmap --> send write > < complete write < complete unmap < cancellation done (unmap complete) mhm OK that makes sense. But i cannot even login via SSH and i also see no cancellation message in kernel log. So what is the right way to test these patches? With my version i can still work via SSH. Stefan
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Il 19/08/2012 21:22, Stefan Priebe - Profihost AG ha scritto: > >> No problem, my fault---I'm just back and I haven't really started again >> all my stuff, so the patch was not tested. >> >> This should fix it, though. > > Booting works fine now. But the VM starts to hang after trying to unmap > large regions. No segfault or so just not reacting anymore. This is expected; unfortunately cancellation right now is a synchronous operation in the block layer. SCSI is the first big user of cancellation, and it would indeed benefit from asynchronous cancellation. Without these three patches, you risk corruption in case the following happens: qemu target --- send unmap > cancel unmap --> send write > < complete write < complete unmap < cancellation done (unmap complete) Paolo
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Am 19.08.2012 15:11, schrieb Paolo Bonzini: No problem, my fault---I'm just back and I haven't really started again all my stuff, so the patch was not tested. This should fix it, though. Booting works fine now. But the VM starts to hang after trying to unmap large regions. No segfault or so just not reacting anymore. Stefan
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Il 19/08/2012 09:55, Stefan Priebe ha scritto: > Hi Paolo, > > Am 18.08.2012 23:49, schrieb Paolo Bonzini: >> Hi Stefan, >> >> this is my version of your patch. I think the flow of the code is a >> bit simpler (or at least matches other implementations of cancellation). >> Can you test it on your test case? > I'm really sorry but your patch doesn't work at all. I'm not even able > to start the VM. KVM process hangs and never detaches itself. No problem, my fault---I'm just back and I haven't really started again all my stuff, so the patch was not tested. This should fix it, though. Paolo diff --git a/block/iscsi.c b/block/iscsi.c index 74ada64..0b96165 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -247,6 +247,7 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num, acb->qiov = qiov; acb->canceled = 0; +acb->bh = NULL; acb->status = -EINPROGRESS; /* XXX we should pass the iovec to write16 to avoid the extra copy */ @@ -341,6 +342,7 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num, acb->qiov = qiov; acb->canceled= 0; +acb->bh = NULL; acb->status = -EINPROGRESS; acb->read_size = qemu_read_size; acb->buf = NULL; @@ -442,6 +444,7 @@ iscsi_aio_flush(BlockDriverState *bs, acb->iscsilun = iscsilun; acb->canceled = 0; +acb->bh = NULL; acb->status = -EINPROGRESS; acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun, @@ -494,6 +497,7 @@ iscsi_aio_discard(BlockDriverState *bs, acb->iscsilun = iscsilun; acb->canceled = 0; +acb->bh = NULL; acb->status = -EINPROGRESS; list[0].lba = sector_qemu2lun(sector_num, iscsilun); @@ -568,6 +572,7 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, acb->iscsilun = iscsilun; acb->canceled= 0; +acb->bh = NULL; acb->status = -EINPROGRESS; acb->buf = NULL; acb->ioh = buf;
Re: [Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Hi Paolo, Am 18.08.2012 23:49, schrieb Paolo Bonzini: Hi Stefan, this is my version of your patch. I think the flow of the code is a bit simpler (or at least matches other implementations of cancellation). Can you test it on your test case? I'm really sorry but your patch doesn't work at all. I'm not even able to start the VM. KVM process hangs and never detaches itself. Greets, Stefan
[Qemu-devel] [PATCH RFT 0/3] iscsi: fix NULL dereferences / races between task completion and abort
Hi Stefan, this is my version of your patch. I think the flow of the code is a bit simpler (or at least matches other implementations of cancellation). Can you test it on your test case? Thanks! Paolo Paolo Bonzini (3): iscsi: move iscsi_schedule_bh and iscsi_readv_writev_bh_cb iscsi: simplify iscsi_schedule_bh iscsi: fix races between task completion and abort block/iscsi.c | 114 +++--- 1 file modificato, 52 inserzioni(+), 62 rimozioni(-) -- 1.7.11.2
