On Mon, Jun 1, 2020 at 12:49 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Sat, 2020-05-30 at 17:34 +0200, Ilya Dryomov wrote:
> > These would be matched with the provided client location to calculate
> > the locality value.
> >
> > Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
> > ---
> > include/linux/crush/crush.h | 6 +++
> > net/ceph/crush/crush.c | 3 ++
> > net/ceph/osdmap.c | 85 ++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 92 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/crush/crush.h b/include/linux/crush/crush.h
> > index 38b0e4d50ed9..29b0de2e202b 100644
> > --- a/include/linux/crush/crush.h
> > +++ b/include/linux/crush/crush.h
> > @@ -301,6 +301,12 @@ struct crush_map {
> >
> > __u32 *choose_tries;
> > #else
> > + /* device/bucket type id -> type name (CrushWrapper::type_map) */
> > + struct rb_root type_names;
> > +
> > + /* device/bucket id -> name (CrushWrapper::name_map) */
> > + struct rb_root names;
> > +
>
> I'm not as familiar with the OSD client-side code, but I don't see any
> mention of locking here. What protects these new rbtrees? From reading
> over the code, I'd assume the osdc->lock, but it might be nice to have a
> comment here that makes that clear.
Yes, everything osdmap and CRUSH (except for the actual invocation) is
protected by osdc->lock. But these trees don't even need that -- they
are static and could be easily replaced with sorted arrays to save two
pointers per name.
Thanks,
Ilya
