On Wed, 20 Jun 2012, Guan Jun He wrote:
>
>
> >>> 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.
Ok. The trick here is that I think we want to set a flag to discard
incoming messages and then flush the workqueue, do all this other
tear down for osd_client etc., and then tear down the messenger. I don't
think we can tear down the msgr fully before that, though, because
osd_client etc have their fingers in it. We just need to prevent the race
of incoming work during teardown...
Thanks for looking at this!
sage
> 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
>
>
--
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
