* [Qemu-devel] Node deletion in aio_set_fd_handler
@ 2018-08-03 2:42 Fam Zheng
2018-08-03 6:13 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2018-08-03 2:42 UTC (permalink / raw)
To: pbonzini; +Cc: qemu-devel
Paolo, is this else branch dead code (or bug?):
/* If the lock is held, just mark the node as deleted */
if (qemu_lockcnt_count(&ctx->list_lock)) {
node->deleted = 1;
node->pfd.revents = 0;
} else {
/* Otherwise, delete it for real. We can't just mark it as
* deleted because deleted nodes are only cleaned up while
* no one is walking the handlers list.
*/
QLIST_REMOVE(node, node);
deleted = true;
}
Since we are in a qemu_lockcnt_lock/unlock section?
Fam
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] Node deletion in aio_set_fd_handler
2018-08-03 2:42 [Qemu-devel] Node deletion in aio_set_fd_handler Fam Zheng
@ 2018-08-03 6:13 ` Paolo Bonzini
2018-08-03 6:36 ` Fam Zheng
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2018-08-03 6:13 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel
On 03/08/2018 04:42, Fam Zheng wrote:
> Paolo, is this else branch dead code (or bug?):
>
> /* If the lock is held, just mark the node as deleted */
> if (qemu_lockcnt_count(&ctx->list_lock)) {
> node->deleted = 1;
> node->pfd.revents = 0;
> } else {
> /* Otherwise, delete it for real. We can't just mark it as
> * deleted because deleted nodes are only cleaned up while
> * no one is walking the handlers list.
> */
> QLIST_REMOVE(node, node);
> deleted = true;
> }
>
> Since we are in a qemu_lockcnt_lock/unlock section?
No, qemu_lockcnt_count returns the number of nested qemu_lockcnt_inc/dec
(read-side) sections. lock/unlock is for the write side.
The comment should be improved (e.g. "if a read is in progress, just
mark the node as deleted").
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] Node deletion in aio_set_fd_handler
2018-08-03 6:13 ` Paolo Bonzini
@ 2018-08-03 6:36 ` Fam Zheng
0 siblings, 0 replies; 3+ messages in thread
From: Fam Zheng @ 2018-08-03 6:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Fri, 08/03 08:13, Paolo Bonzini wrote:
> On 03/08/2018 04:42, Fam Zheng wrote:
> > Paolo, is this else branch dead code (or bug?):
> >
> > /* If the lock is held, just mark the node as deleted */
> > if (qemu_lockcnt_count(&ctx->list_lock)) {
> > node->deleted = 1;
> > node->pfd.revents = 0;
> > } else {
> > /* Otherwise, delete it for real. We can't just mark it as
> > * deleted because deleted nodes are only cleaned up while
> > * no one is walking the handlers list.
> > */
> > QLIST_REMOVE(node, node);
> > deleted = true;
> > }
> >
> > Since we are in a qemu_lockcnt_lock/unlock section?
>
> No, qemu_lockcnt_count returns the number of nested qemu_lockcnt_inc/dec
> (read-side) sections. lock/unlock is for the write side.
>
> The comment should be improved (e.g. "if a read is in progress, just
> mark the node as deleted").
Yeah, that explains! Agree about the comment, I'll send a patch then. Thanks.
Fam
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-03 6:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-03 2:42 [Qemu-devel] Node deletion in aio_set_fd_handler Fam Zheng
2018-08-03 6:13 ` Paolo Bonzini
2018-08-03 6:36 ` Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).