>>> On 6/19/2012 at 11:33 PM, in message
<Pine.LNX.4.64.1206190828510.21853@xxxxxxxxxxxxxxxxxx>, Sage Weil
<sage@xxxxxxxxxxx> wrote:
> [Sorry for not responding earlier!]
>
> On Tue, 19 Jun 2012, Guan Jun He wrote:
>> Hi,
>>
>> Do you think this is needed?
>> The osdmap update need to hold this sem.As in function
>> void ceph_osdc_handle_map(struct ceph_osd_client *osdc, struct ceph_msg
> *msg)
>> and
>> function static int __map_request(),
>> etc.
>
> I'm skeptical that this is necessary. At the point that we're tearing
> down the osd_client there shouldn't be anybody else referencing us, or the
> osdmap.
>
> Actually, looking at the code a bit, I think the only kind of race we need
> to worry about is an incoming message. The correct fix is probably a call
> to the messenger to drop all incoming traffic before we start to shut
> everything else down. Even with this, we might still end up with a new
> osdmap that we leak, but stopping the msgr earlier would fix both issues.
Yes, stopping the msgr earlier is a better fix.
I will do some test and produce a new patch to this.
thanks,
Guanjun
>
> sage
>
>>
>> Thanks a lot for your reply!
>>
>> thanks,
>> Guanjun
>>
>> >>> On 6/15/2012 at 04:32 PM, in message
>> <1339749120-7007-1-git-send-email-gjhe@xxxxxxxx>, Guanjun He
>> <heguanbo@xxxxxxxxx> wrote:
>> > From: Guanjun He <heguanbo@xxxxxxxxx>
>> >
>> > I think the osdmap destroy code shoule be protected by map_sem.
>> > If I was wrong, please correct me, thanks!
>> >
>> > Signed-off-by: Guanjun He <gjhe@xxxxxxxx>
>> > ---
>> > net/ceph/osd_client.c | 2 ++
>> > 1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
>> > index 1ffebed..718b100 100644
>> > --- a/net/ceph/osd_client.c
>> > +++ b/net/ceph/osd_client.c
>> > @@ -1872,11 +1872,13 @@ void ceph_osdc_stop(struct ceph_osd_client *osdc)
>> > destroy_workqueue(osdc->notify_wq);
>> > cancel_delayed_work_sync(&osdc->timeout_work);
>> > cancel_delayed_work_sync(&osdc->osds_timeout_work);
>> > + down_write(&osdc->map_sem);
>> > if (osdc->osdmap) {
>> > ceph_osdmap_destroy(osdc->osdmap);
>> > osdc->osdmap = NULL;
>> > }
>> > remove_all_osds(osdc);
>> > + up_write(&osdc->map_sem);
>> > mempool_destroy(osdc->req_mempool);
>> > ceph_msgpool_destroy(&osdc->msgpool_op);
>> > ceph_msgpool_destroy(&osdc->msgpool_op_reply);
>>
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
