Re: [Xen-devel] tools/libxl - Async Task Cancellation Query
Hi Ian, Any thoughts on this below? Regards, Koushik Chakravarty Mobile - +91-9663396424 -Original Message- From: Koushik Chakravarty Sent: Wednesday, April 8, 2015 5:44 PM To: Ian Jackson Cc: Euan Harris; 'xen-de...@lists.xensource.com' Subject: RE: tools/libxl - Async Task Cancellation Query Thanks Ian for the answers. I have a follow - up on the below: Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. When you say - That would require the caller to preserve the ao_how which seems awkward to me - whom do you refer as the caller? From what I picked up, the libxl_ao_cancel() API is passed with the ao_how. The libxl_ao_cancel then looks into the ctx-aos_inprogress list to find the ao that matches the ao_how. So what I was suggesting was that if the ao_how was allotted an 'id' from the original libxl api call (done using a counter increment from the global libxl ctx with a lock held), and the same id was saved in the ao entry in ctx-aos_inprogress, then searching/matching them in libxl_ao_cancel() would have been a little faster. Of course, the assumption here is that the caller keeps the ao_how struct and uses that to call libxl_ao_cancel() - but I see that is already the case. Let me know if I am not being very clear. Regards, Koushik Chakravarty Mobile - +91-9663396424 -Original Message- From: Ian Jackson [mailto:ian.jack...@eu.citrix.com] Sent: Wednesday, April 8, 2015 4:52 PM To: Koushik Chakravarty Cc: Euan Harris; 'xen-de...@lists.xensource.com' Subject: Re: tools/libxl - Async Task Cancellation Query Koushik Chakravarty writes (tools/libxl - Async Task Cancellation Query): I am currently looking into the asynchronous task cancellation in libxl and have a few very specific queries, if you could answer. Sure. Thanks for what has evidently been a careful review of the code. 1.In libxl_domain_resume(),why is libxl_ao_complete called before AO_INPROGRESS? I was going to refer you to the internal API documentation, but I find that it doesn't describe this situation. I have drafted a patch to the documentation which I hope will answer this question. I'll send it as a followup to this mail. 2.In libxl_ao_cancel() - the function goes through the ctx-aos_inprogress and tries to find a suitable libxl_ao that matches the input libxl_asyncop_how. It does so, by a few 'if' checks. Regarding this - a.Where does the libxl__ao get inserted to the ctx-aos_inprogress? I could not find that somehow - sorry if I overlooked. Thank you again for your review. I think you have spotted a bug. I will send a followup patch, again as a followup to this mail. b.Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. 3.In libxl_device_vkb_add(), shouldn't the function invoke libxl__ao_abort in the error path? I think this will be answered by my documentation patch. But, to summarise, the codepaths: assert(rc); libxl__ao_complete(egc, ao, rc); return AO_INPROGRESS; and assert(rc); return AO_ABORT(rc); are equivalent. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] tools/libxl - Async Task Cancellation Query [and 1 more messages]
Koushik Chakravarty writes (RE: tools/libxl - Async Task Cancellation Query): When you say - That would require the caller to preserve the ao_how which seems awkward to me - whom do you refer as the caller? By the caller I mean the program outside libxl which is calling libxl. From what I picked up, the libxl_ao_cancel() API is passed with the ao_how. The libxl_ao_cancel then looks into the ctx-aos_inprogress list to find the ao that matches the ao_how. Yes. So what I was suggesting was that if the ao_how was allotted an 'id' from the original libxl api call (done using a counter increment from the global libxl ctx with a lock held), and the same id was saved in the ao entry in ctx-aos_inprogress, then searching/matching them in libxl_ao_cancel() would have been a little faster. Sorry, I had misunderstood. I thought you were proposing some kind of arrangement where libxl would be able to look up the relevant ao in O(1). That would require either the lifetime of these ids to be controlled. But now I understand that you're just proposing a pure id which still requires a search. I don't think the performance benefit is really worthwhile for the extra complexity. This is particularly true given that currently the ao_how is const. So the caller might have defined it as `static const' and put it in the text segment. Changing the API to make this parameter non-const sometimes, in a backward compatible way, would be a bit complicated. But thanks anyway for your suggestion. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] tools/libxl - Async Task Cancellation Query
Thanks Ian for the answers. I have a follow - up on the below: Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. When you say - That would require the caller to preserve the ao_how which seems awkward to me - whom do you refer as the caller? From what I picked up, the libxl_ao_cancel() API is passed with the ao_how. The libxl_ao_cancel then looks into the ctx-aos_inprogress list to find the ao that matches the ao_how. So what I was suggesting was that if the ao_how was allotted an 'id' from the original libxl api call (done using a counter increment from the global libxl ctx with a lock held), and the same id was saved in the ao entry in ctx-aos_inprogress, then searching/matching them in libxl_ao_cancel() would have been a little faster. Let me know if I am not being very clear. Regards, Koushik Chakravarty Mobile - +91-9663396424 -Original Message- From: Ian Jackson [mailto:ian.jack...@eu.citrix.com] Sent: Wednesday, April 8, 2015 4:52 PM To: Koushik Chakravarty Cc: Euan Harris; 'xen-de...@lists.xensource.com' Subject: Re: tools/libxl - Async Task Cancellation Query Koushik Chakravarty writes (tools/libxl - Async Task Cancellation Query): I am currently looking into the asynchronous task cancellation in libxl and have a few very specific queries, if you could answer. Sure. Thanks for what has evidently been a careful review of the code. 1.In libxl_domain_resume(),why is libxl_ao_complete called before AO_INPROGRESS? I was going to refer you to the internal API documentation, but I find that it doesn't describe this situation. I have drafted a patch to the documentation which I hope will answer this question. I'll send it as a followup to this mail. 2.In libxl_ao_cancel() - the function goes through the ctx-aos_inprogress and tries to find a suitable libxl_ao that matches the input libxl_asyncop_how. It does so, by a few 'if' checks. Regarding this - a.Where does the libxl__ao get inserted to the ctx-aos_inprogress? I could not find that somehow - sorry if I overlooked. Thank you again for your review. I think you have spotted a bug. I will send a followup patch, again as a followup to this mail. b.Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. 3.In libxl_device_vkb_add(), shouldn't the function invoke libxl__ao_abort in the error path? I think this will be answered by my documentation patch. But, to summarise, the codepaths: assert(rc); libxl__ao_complete(egc, ao, rc); return AO_INPROGRESS; and assert(rc); return AO_ABORT(rc); are equivalent. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] tools/libxl - Async Task Cancellation Query
Koushik Chakravarty writes (tools/libxl - Async Task Cancellation Query): I am currently looking into the asynchronous task cancellation in libxl and have a few very specific queries, if you could answer. Sure. Thanks for what has evidently been a careful review of the code. 1.In libxl_domain_resume(),why is libxl_ao_complete called before AO_INPROGRESS? I was going to refer you to the internal API documentation, but I find that it doesn't describe this situation. I have drafted a patch to the documentation which I hope will answer this question. I'll send it as a followup to this mail. 2.In libxl_ao_cancel() - the function goes through the ctx-aos_inprogress and tries to find a suitable libxl_ao that matches the input libxl_asyncop_how. It does so, by a few 'if' checks. Regarding this - a.Where does the libxl__ao get inserted to the ctx-aos_inprogress? I could not find that somehow - sorry if I overlooked. Thank you again for your review. I think you have spotted a bug. I will send a followup patch, again as a followup to this mail. b.Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. 3.In libxl_device_vkb_add(), shouldn't the function invoke libxl__ao_abort in the error path? I think this will be answered by my documentation patch. But, to summarise, the codepaths: assert(rc); libxl__ao_complete(egc, ao, rc); return AO_INPROGRESS; and assert(rc); return AO_ABORT(rc); are equivalent. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] tools/libxl - Async Task Cancellation Query
Hi Ian, I am currently looking into the asynchronous task cancellation in libxl and have a few very specific queries, if you could answer. 1.In libxl_domain_resume(),why is libxl_ao_complete called before AO_INPROGRESS? 2.In libxl_ao_cancel() - the function goes through the ctx-aos_inprogress and tries to find a suitable libxl_ao that matches the input libxl_asyncop_how. It does so, by a few 'if' checks. Regarding this - a.Where does the libxl__ao get inserted to the ctx-aos_inprogress? I could not find that somehow - sorry if I overlooked. b.Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx-aos_inprogress in libxl_ao_cancel() quicker by looking for search-id == libxl_asyncop_how-id. 3.In libxl_device_vkb_add(), shouldn't the function invoke libxl__ao_abort in the error path? Thanks in advance! Regards, Koushik Chakravarty Mobile - +91-9663396424 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
