All of lore.kernel.org
 help / color / mirror / Atom feed
* ObjectContext : check that all references are gone
@ 2013-07-15 19:25 Loic Dachary
  2013-07-15 21:53 ` Sage Weil
  0 siblings, 1 reply; 3+ messages in thread
From: Loic Dachary @ 2013-07-15 19:25 UTC (permalink / raw)
  To: Sage Weil; +Cc: Ceph Development

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Hi Sage,

You mentionned that there is code somewhere checking the fact that all references to an ObjectContext are gone and that replacing manual reference counting with shared_ptr should preserve this ( i.e. in relation to https://github.com/ceph/ceph/pull/414 "replace ObjectContext pointers with shared_ptr" ). However, I've not been able to find such code. Would you be so kind as to show me where it is ?

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ObjectContext : check that all references are gone
  2013-07-15 19:25 ObjectContext : check that all references are gone Loic Dachary
@ 2013-07-15 21:53 ` Sage Weil
  2013-07-16 15:28   ` Loic Dachary
  0 siblings, 1 reply; 3+ messages in thread
From: Sage Weil @ 2013-07-15 21:53 UTC (permalink / raw)
  To: Loic Dachary; +Cc: Ceph Development

On Mon, 15 Jul 2013, Loic Dachary wrote:
> Hi Sage,
> 
> You mentionned that there is code somewhere checking the fact that all references to an ObjectContext are gone and that replacing manual reference counting with shared_ptr should preserve this ( i.e. in relation to https://github.com/ceph/ceph/pull/414 "replace ObjectContext pointers with shared_ptr" ). However, I've not been able to find such code. Would you be so kind as to show me where it is ?
> 
> Cheers

It's in ReplicatedPG.cc:

void ReplicatedPG::on_flushed()
{
  assert(object_contexts.empty());
}

which is called from the Reset state of the PG peering state machine.

sage

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ObjectContext : check that all references are gone
  2013-07-15 21:53 ` Sage Weil
@ 2013-07-16 15:28   ` Loic Dachary
  0 siblings, 0 replies; 3+ messages in thread
From: Loic Dachary @ 2013-07-16 15:28 UTC (permalink / raw)
  To: Sage Weil; +Cc: Ceph Development

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]



On 15/07/2013 23:53, Sage Weil wrote:
> On Mon, 15 Jul 2013, Loic Dachary wrote:
>> Hi Sage,
>>
>> You mentionned that there is code somewhere checking the fact that all references to an ObjectContext are gone and that replacing manual reference counting with shared_ptr should preserve this ( i.e. in relation to https://github.com/ceph/ceph/pull/414 "replace ObjectContext pointers with shared_ptr" ). However, I've not been able to find such code. Would you be so kind as to show me where it is ?
>>
>> Cheers
> 
> It's in ReplicatedPG.cc:
> 
> void ReplicatedPG::on_flushed()
> {
>   assert(object_contexts.empty());
> }
> 
> which is called from the Reset state of the PG peering state machine.

It is still here:

https://github.com/dachary/ceph/blob/1a7e390965e6b03f72360f016b2e7d852c455784/src/osd/ReplicatedPG.cc#L6659

and should be ok. Although the de-allocation happens after the last shared_ptr<> goes out of scope instead of being deleted by an explicit call to put_object_context, I believe it happens before on_flushed() is invoked at

https://github.com/dachary/ceph/blob/1a7e390965e6b03f72360f016b2e7d852c455784/src/osd/PG.cc#L5225

I can't imagine a case where it would assert with the proposed patch. 

Cheers
> 
> sage
> 

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-07-16 15:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-15 19:25 ObjectContext : check that all references are gone Loic Dachary
2013-07-15 21:53 ` Sage Weil
2013-07-16 15:28   ` Loic Dachary

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.