| Message ID | bfc20d2c-9198-2cb3-506a-4bae09c6cd68@cybozu.co.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] libceph: print fsid and client gid with osd id | expand |
On Mon, Jun 6, 2022 at 8:00 AM Daichi Mukai <daichi-mukai@cybozu.co.jp> wrote: > > Print fsid and client gid in libceph log messages to distinct from which > each message come. > > Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com> > Signed-off-by: Daichi Mukai <daichi-mukai@cybozu.co.jp> > --- > include/linux/ceph/osdmap.h | 2 +- > net/ceph/osd_client.c | 3 ++- > net/ceph/osdmap.c | 43 ++++++++++++++++++++++++++----------- > 3 files changed, 34 insertions(+), 14 deletions(-) > > * Changes since v3 > > - Rebased to latest mainline > > * Changes since v2 > > - Set scope of this patch to log message for osd > - Improved format of message > > * Changes since v1 > > - Added client gid to log message > > diff --git a/include/linux/ceph/osdmap.h b/include/linux/ceph/osdmap.h > index 5553019c3f07..a9216c64350c 100644 > --- a/include/linux/ceph/osdmap.h > +++ b/include/linux/ceph/osdmap.h > @@ -253,7 +253,7 @@ static inline int ceph_decode_pgid(void **p, void *end, struct ceph_pg *pgid) > struct ceph_osdmap *ceph_osdmap_alloc(void); > struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2); > struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, > - struct ceph_osdmap *map); > + struct ceph_osdmap *map, u64 gid); > extern void ceph_osdmap_destroy(struct ceph_osdmap *map); > > struct ceph_osds { > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 9d82bb42e958..e9bd6c27c5ad 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -3945,7 +3945,8 @@ static int handle_one_map(struct ceph_osd_client *osdc, > if (incremental) > newmap = osdmap_apply_incremental(&p, end, > ceph_msgr2(osdc->client), > - osdc->osdmap); > + osdc->osdmap, > + ceph_client_gid(osdc->client)); > else > newmap = ceph_osdmap_decode(&p, end, ceph_msgr2(osdc->client)); > if (IS_ERR(newmap)) > diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c > index 2823bb3cff55..cd65677baff3 100644 > --- a/net/ceph/osdmap.c > +++ b/net/ceph/osdmap.c > @@ -1549,8 +1549,23 @@ static int decode_primary_affinity(void **p, void *end, > return -EINVAL; > } > > +static __printf(3, 4) > +void print_osd_info(struct ceph_fsid *fsid, u64 gid, const char *fmt, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + va_start(args, fmt); > + vaf.fmt = fmt; > + vaf.va = &args; > + > + printk(KERN_INFO "%s (%pU %lld): %pV", KBUILD_MODNAME, fsid, gid, &vaf); > + > + va_end(args); > +} > + > static int decode_new_primary_affinity(void **p, void *end, > - struct ceph_osdmap *map) > + struct ceph_osdmap *map, u64 gid) > { > u32 n; > > @@ -1566,7 +1581,8 @@ static int decode_new_primary_affinity(void **p, void *end, > if (ret) > return ret; > > - pr_info("osd%d primary-affinity 0x%x\n", osd, aff); > + print_osd_info(&map->fsid, gid, "osd%d primary-affinity 0x%x\n", > + osd, aff); > } > > return 0; > @@ -1825,7 +1841,8 @@ struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2) > * new_state: { osd=6, xorstate=EXISTS } # clear osd_state > */ > static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, > - bool msgr2, struct ceph_osdmap *map) > + bool msgr2, struct ceph_osdmap *map, > + u64 gid) > { > void *new_up_client; > void *new_state; > @@ -1864,9 +1881,10 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, > osd = ceph_decode_32(p); > w = ceph_decode_32(p); > BUG_ON(osd >= map->max_osd); > - pr_info("osd%d weight 0x%x %s\n", osd, w, > - w == CEPH_OSD_IN ? "(in)" : > - (w == CEPH_OSD_OUT ? "(out)" : "")); > + print_osd_info(&map->fsid, gid, "osd%d weight 0x%x %s\n", > + osd, w, > + w == CEPH_OSD_IN ? "(in)" : > + (w == CEPH_OSD_OUT ? "(out)" : "")); > map->osd_weight[osd] = w; > > /* > @@ -1898,10 +1916,11 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, > BUG_ON(osd >= map->max_osd); > if ((map->osd_state[osd] & CEPH_OSD_UP) && > (xorstate & CEPH_OSD_UP)) > - pr_info("osd%d down\n", osd); > + print_osd_info(&map->fsid, gid, "osd%d down\n", osd); > if ((map->osd_state[osd] & CEPH_OSD_EXISTS) && > (xorstate & CEPH_OSD_EXISTS)) { > - pr_info("osd%d does not exist\n", osd); > + print_osd_info(&map->fsid, gid, "osd%d does not exist\n", > + osd); > ret = set_primary_affinity(map, osd, > CEPH_OSD_DEFAULT_PRIMARY_AFFINITY); > if (ret) > @@ -1931,7 +1950,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, > > dout("%s osd%d addr %s\n", __func__, osd, ceph_pr_addr(&addr)); > > - pr_info("osd%d up\n", osd); > + print_osd_info(&map->fsid, gid, "osd%d up\n", osd); > map->osd_state[osd] |= CEPH_OSD_EXISTS | CEPH_OSD_UP; > map->osd_addr[osd] = addr; > } > @@ -1947,7 +1966,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, > * decode and apply an incremental map update. > */ > struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, > - struct ceph_osdmap *map) > + struct ceph_osdmap *map, u64 gid) > { > struct ceph_fsid fsid; > u32 epoch = 0; > @@ -2033,7 +2052,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, > } > > /* new_up_client, new_state, new_weight */ > - err = decode_new_up_state_weight(p, end, struct_v, msgr2, map); > + err = decode_new_up_state_weight(p, end, struct_v, msgr2, map, gid); > if (err) > goto bad; > > @@ -2051,7 +2070,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, > > /* new_primary_affinity */ > if (struct_v >= 2) { > - err = decode_new_primary_affinity(p, end, map); > + err = decode_new_primary_affinity(p, end, map, gid); > if (err) > goto bad; > } > -- > 2.25.1 Hi Daichi, I played with this a bit and decided that printing gid is not needed after all, given that only osdmap messages are changed. Instead, I switched to printing epoch which can be very useful when debugging stuck OSD requests. Here is the patch I ended up with: https://github.com/ceph/ceph-client/commit/b5f9965fad5a4f3a8d17aa234167e8a85e1b9105 Could you please take a look and let me know if you are OK with it? Thanks, Ilya
On 2022/06/14 22:12, Ilya Dryomov wrote: > On Mon, Jun 6, 2022 at 8:00 AM Daichi Mukai <daichi-mukai@cybozu.co.jp> wrote: >> >> Print fsid and client gid in libceph log messages to distinct from which >> each message come. >> >> Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com> >> Signed-off-by: Daichi Mukai <daichi-mukai@cybozu.co.jp> >> --- >> include/linux/ceph/osdmap.h | 2 +- >> net/ceph/osd_client.c | 3 ++- >> net/ceph/osdmap.c | 43 ++++++++++++++++++++++++++----------- >> 3 files changed, 34 insertions(+), 14 deletions(-) >> >> * Changes since v3 >> >> - Rebased to latest mainline >> >> * Changes since v2 >> >> - Set scope of this patch to log message for osd >> - Improved format of message >> >> * Changes since v1 >> >> - Added client gid to log message >> >> diff --git a/include/linux/ceph/osdmap.h b/include/linux/ceph/osdmap.h >> index 5553019c3f07..a9216c64350c 100644 >> --- a/include/linux/ceph/osdmap.h >> +++ b/include/linux/ceph/osdmap.h >> @@ -253,7 +253,7 @@ static inline int ceph_decode_pgid(void **p, void *end, struct ceph_pg *pgid) >> struct ceph_osdmap *ceph_osdmap_alloc(void); >> struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2); >> struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, >> - struct ceph_osdmap *map); >> + struct ceph_osdmap *map, u64 gid); >> extern void ceph_osdmap_destroy(struct ceph_osdmap *map); >> >> struct ceph_osds { >> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c >> index 9d82bb42e958..e9bd6c27c5ad 100644 >> --- a/net/ceph/osd_client.c >> +++ b/net/ceph/osd_client.c >> @@ -3945,7 +3945,8 @@ static int handle_one_map(struct ceph_osd_client *osdc, >> if (incremental) >> newmap = osdmap_apply_incremental(&p, end, >> ceph_msgr2(osdc->client), >> - osdc->osdmap); >> + osdc->osdmap, >> + ceph_client_gid(osdc->client)); >> else >> newmap = ceph_osdmap_decode(&p, end, ceph_msgr2(osdc->client)); >> if (IS_ERR(newmap)) >> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c >> index 2823bb3cff55..cd65677baff3 100644 >> --- a/net/ceph/osdmap.c >> +++ b/net/ceph/osdmap.c >> @@ -1549,8 +1549,23 @@ static int decode_primary_affinity(void **p, void *end, >> return -EINVAL; >> } >> >> +static __printf(3, 4) >> +void print_osd_info(struct ceph_fsid *fsid, u64 gid, const char *fmt, ...) >> +{ >> + struct va_format vaf; >> + va_list args; >> + >> + va_start(args, fmt); >> + vaf.fmt = fmt; >> + vaf.va = &args; >> + >> + printk(KERN_INFO "%s (%pU %lld): %pV", KBUILD_MODNAME, fsid, gid, &vaf); >> + >> + va_end(args); >> +} >> + >> static int decode_new_primary_affinity(void **p, void *end, >> - struct ceph_osdmap *map) >> + struct ceph_osdmap *map, u64 gid) >> { >> u32 n; >> >> @@ -1566,7 +1581,8 @@ static int decode_new_primary_affinity(void **p, void *end, >> if (ret) >> return ret; >> >> - pr_info("osd%d primary-affinity 0x%x\n", osd, aff); >> + print_osd_info(&map->fsid, gid, "osd%d primary-affinity 0x%x\n", >> + osd, aff); >> } >> >> return 0; >> @@ -1825,7 +1841,8 @@ struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2) >> * new_state: { osd=6, xorstate=EXISTS } # clear osd_state >> */ >> static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, >> - bool msgr2, struct ceph_osdmap *map) >> + bool msgr2, struct ceph_osdmap *map, >> + u64 gid) >> { >> void *new_up_client; >> void *new_state; >> @@ -1864,9 +1881,10 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, >> osd = ceph_decode_32(p); >> w = ceph_decode_32(p); >> BUG_ON(osd >= map->max_osd); >> - pr_info("osd%d weight 0x%x %s\n", osd, w, >> - w == CEPH_OSD_IN ? "(in)" : >> - (w == CEPH_OSD_OUT ? "(out)" : "")); >> + print_osd_info(&map->fsid, gid, "osd%d weight 0x%x %s\n", >> + osd, w, >> + w == CEPH_OSD_IN ? "(in)" : >> + (w == CEPH_OSD_OUT ? "(out)" : "")); >> map->osd_weight[osd] = w; >> >> /* >> @@ -1898,10 +1916,11 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, >> BUG_ON(osd >= map->max_osd); >> if ((map->osd_state[osd] & CEPH_OSD_UP) && >> (xorstate & CEPH_OSD_UP)) >> - pr_info("osd%d down\n", osd); >> + print_osd_info(&map->fsid, gid, "osd%d down\n", osd); >> if ((map->osd_state[osd] & CEPH_OSD_EXISTS) && >> (xorstate & CEPH_OSD_EXISTS)) { >> - pr_info("osd%d does not exist\n", osd); >> + print_osd_info(&map->fsid, gid, "osd%d does not exist\n", >> + osd); >> ret = set_primary_affinity(map, osd, >> CEPH_OSD_DEFAULT_PRIMARY_AFFINITY); >> if (ret) >> @@ -1931,7 +1950,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, >> >> dout("%s osd%d addr %s\n", __func__, osd, ceph_pr_addr(&addr)); >> >> - pr_info("osd%d up\n", osd); >> + print_osd_info(&map->fsid, gid, "osd%d up\n", osd); >> map->osd_state[osd] |= CEPH_OSD_EXISTS | CEPH_OSD_UP; >> map->osd_addr[osd] = addr; >> } >> @@ -1947,7 +1966,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, >> * decode and apply an incremental map update. >> */ >> struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, >> - struct ceph_osdmap *map) >> + struct ceph_osdmap *map, u64 gid) >> { >> struct ceph_fsid fsid; >> u32 epoch = 0; >> @@ -2033,7 +2052,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, >> } >> >> /* new_up_client, new_state, new_weight */ >> - err = decode_new_up_state_weight(p, end, struct_v, msgr2, map); >> + err = decode_new_up_state_weight(p, end, struct_v, msgr2, map, gid); >> if (err) >> goto bad; >> >> @@ -2051,7 +2070,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, >> >> /* new_primary_affinity */ >> if (struct_v >= 2) { >> - err = decode_new_primary_affinity(p, end, map); >> + err = decode_new_primary_affinity(p, end, map, gid); >> if (err) >> goto bad; >> } >> -- >> 2.25.1 > > Hi Daichi, > > I played with this a bit and decided that printing gid is not needed > after all, given that only osdmap messages are changed. Instead, > I switched to printing epoch which can be very useful when debugging > stuck OSD requests. Here is the patch I ended up with: > > https://github.com/ceph/ceph-client/commit/b5f9965fad5a4f3a8d17aa234167e8a85e1b9105 > > Could you please take a look and let me know if you are OK with it? Hi Ilya, I have looked the patch and tried it. It looks good to me. Thank you very much! Best, Daichi
diff --git a/include/linux/ceph/osdmap.h b/include/linux/ceph/osdmap.h index 5553019c3f07..a9216c64350c 100644 --- a/include/linux/ceph/osdmap.h +++ b/include/linux/ceph/osdmap.h @@ -253,7 +253,7 @@ static inline int ceph_decode_pgid(void **p, void *end, struct ceph_pg *pgid) struct ceph_osdmap *ceph_osdmap_alloc(void); struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2); struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, - struct ceph_osdmap *map); + struct ceph_osdmap *map, u64 gid); extern void ceph_osdmap_destroy(struct ceph_osdmap *map); struct ceph_osds { diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index 9d82bb42e958..e9bd6c27c5ad 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -3945,7 +3945,8 @@ static int handle_one_map(struct ceph_osd_client *osdc, if (incremental) newmap = osdmap_apply_incremental(&p, end, ceph_msgr2(osdc->client), - osdc->osdmap); + osdc->osdmap, + ceph_client_gid(osdc->client)); else newmap = ceph_osdmap_decode(&p, end, ceph_msgr2(osdc->client)); if (IS_ERR(newmap)) diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c index 2823bb3cff55..cd65677baff3 100644 --- a/net/ceph/osdmap.c +++ b/net/ceph/osdmap.c @@ -1549,8 +1549,23 @@ static int decode_primary_affinity(void **p, void *end, return -EINVAL; } +static __printf(3, 4) +void print_osd_info(struct ceph_fsid *fsid, u64 gid, const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, fmt); + vaf.fmt = fmt; + vaf.va = &args; + + printk(KERN_INFO "%s (%pU %lld): %pV", KBUILD_MODNAME, fsid, gid, &vaf); + + va_end(args); +} + static int decode_new_primary_affinity(void **p, void *end, - struct ceph_osdmap *map) + struct ceph_osdmap *map, u64 gid) { u32 n; @@ -1566,7 +1581,8 @@ static int decode_new_primary_affinity(void **p, void *end, if (ret) return ret; - pr_info("osd%d primary-affinity 0x%x\n", osd, aff); + print_osd_info(&map->fsid, gid, "osd%d primary-affinity 0x%x\n", + osd, aff); } return 0; @@ -1825,7 +1841,8 @@ struct ceph_osdmap *ceph_osdmap_decode(void **p, void *end, bool msgr2) * new_state: { osd=6, xorstate=EXISTS } # clear osd_state */ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, - bool msgr2, struct ceph_osdmap *map) + bool msgr2, struct ceph_osdmap *map, + u64 gid) { void *new_up_client; void *new_state; @@ -1864,9 +1881,10 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, osd = ceph_decode_32(p); w = ceph_decode_32(p); BUG_ON(osd >= map->max_osd); - pr_info("osd%d weight 0x%x %s\n", osd, w, - w == CEPH_OSD_IN ? "(in)" : - (w == CEPH_OSD_OUT ? "(out)" : "")); + print_osd_info(&map->fsid, gid, "osd%d weight 0x%x %s\n", + osd, w, + w == CEPH_OSD_IN ? "(in)" : + (w == CEPH_OSD_OUT ? "(out)" : "")); map->osd_weight[osd] = w; /* @@ -1898,10 +1916,11 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, BUG_ON(osd >= map->max_osd); if ((map->osd_state[osd] & CEPH_OSD_UP) && (xorstate & CEPH_OSD_UP)) - pr_info("osd%d down\n", osd); + print_osd_info(&map->fsid, gid, "osd%d down\n", osd); if ((map->osd_state[osd] & CEPH_OSD_EXISTS) && (xorstate & CEPH_OSD_EXISTS)) { - pr_info("osd%d does not exist\n", osd); + print_osd_info(&map->fsid, gid, "osd%d does not exist\n", + osd); ret = set_primary_affinity(map, osd, CEPH_OSD_DEFAULT_PRIMARY_AFFINITY); if (ret) @@ -1931,7 +1950,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, dout("%s osd%d addr %s\n", __func__, osd, ceph_pr_addr(&addr)); - pr_info("osd%d up\n", osd); + print_osd_info(&map->fsid, gid, "osd%d up\n", osd); map->osd_state[osd] |= CEPH_OSD_EXISTS | CEPH_OSD_UP; map->osd_addr[osd] = addr; } @@ -1947,7 +1966,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v, * decode and apply an incremental map update. */ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, - struct ceph_osdmap *map) + struct ceph_osdmap *map, u64 gid) { struct ceph_fsid fsid; u32 epoch = 0; @@ -2033,7 +2052,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, } /* new_up_client, new_state, new_weight */ - err = decode_new_up_state_weight(p, end, struct_v, msgr2, map); + err = decode_new_up_state_weight(p, end, struct_v, msgr2, map, gid); if (err) goto bad; @@ -2051,7 +2070,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end, bool msgr2, /* new_primary_affinity */ if (struct_v >= 2) { - err = decode_new_primary_affinity(p, end, map); + err = decode_new_primary_affinity(p, end, map, gid); if (err) goto bad; }