On 7/25/19 12:01 PM, Richard W.M. Jones wrote:
On Thu, Jul 25, 2019 at 10:43:05AM -0500, Eric Blake wrote:
On 7/25/19 8:07 AM, Richard W.M. Jones wrote:
@@ -499,7 +499,7 @@ /* Call the caller's extent function. */ int error = cmd->error;
- if (cmd->cb.fn.extent (cmd->cb.fn_user_data, + if (cmd->cb.fn.extent (LIBNBD_CALLBACK_VALID, cmd->cb.fn_user_data, meta_context->name, cmd->offset, &h->bs_entries[1], (length-4) / 4, &error) == -1) if (cmd->error == 0)
Hmm - no change to the FINISH state, which means you are relying on command retirement to free chunk/extent instead. As long as that happens, we should be okay, though.
I didn't understand this one. Is it wrong to add the VALID flag here?
No, the VALID flag here is correct; my comment (with insufficient context) was tied to the fact that this is the last hunk of changes to this file, which means we did not change the FINISH state to call read(FREE) or extent(FREE) (and instead defer the freeing to when we retire the command). So nothing to change in what you have here. In fact, thinking a bit more - my followup patch to call read(VALID|FREE) in more places is worthwhile (because it reduces the number of callbacks compared to read(VALID);read(FREE)); but teaching FINISH to call read(FREE) does not optimize how many callbacks are made, only the point in time in which they are made (freeing the read callback as soon as we know the command is done, and before the completion callback runs), nor does it remove the requirement to still have code for read(FREE) in the retirement code (for when the command is stranded). An argument for calling read(FREE) in FINISH is to avoid the surprise mentioned in the other email where callback(FREE, data) && read(FREE, data) (because the user passed the same data to both callbacks) results in the read callback seeing a stale pointer, but then again, it's the caller's burden to not dereference the pointer if they share the data between callbacks, and not necessarily a case where we MUST call read(FREE) as soon as possible. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org