From fa1bff8f6f3afb858618a563b039af3bbf46153b Mon Sep 17 00:00:00 2001 From: Emoly Liu Date: Sat, 27 May 2017 12:05:44 +0800 Subject: [PATCH] LU-9399 llite: register mountpoint before process llog In ll_fill_super(), lprocfs_ll_register_mountpoint() should be called before lustre_process_log(), otherwise the directory /proc/fs/lustre/llite/* can't be created in time and the params "llite.*.*" won't be set correctly. Also, this patch adds sbi->ll_xattr_cache_set to mark the flag LL_SBI_XATTR_CACHE already set during lustre_process_log(), in case that it will be overwritten in client_common_fill_super(). conf-sanity.sh test_76d is added to verify this fix. Signed-off-by: Emoly Liu Change-Id: Ie1b4d2ef23bbe48a22d48f92f7d4efe64042eec4 Reviewed-on: https://review.whamcloud.com/27241 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: James Simmons Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/llite/llite_internal.h | 16 +++-- lustre/llite/llite_lib.c | 162 ++++++++++++++++++++++++------------------ lustre/llite/lproc_llite.c | 153 ++++++++++++++++++++------------------- lustre/tests/conf-sanity.sh | 28 ++++++++ 4 files changed, 210 insertions(+), 149 deletions(-) diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index 03ab7e6..4737a27 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -489,6 +489,7 @@ struct ll_sb_info { int ll_flags; unsigned int ll_umounting:1, ll_xattr_cache_enabled:1, + ll_xattr_cache_set:1, /* already set to 0/1 */ ll_client_common_fill_super_succeeded:1; struct lustre_client_ocd ll_lco; @@ -695,15 +696,18 @@ void cl_put_grouplock(struct ll_grouplock *lg); /* llite/lproc_llite.c */ #ifdef CONFIG_PROC_FS -int lprocfs_register_mountpoint(struct proc_dir_entry *parent, - struct super_block *sb, char *osc, char *mdc); -void lprocfs_unregister_mountpoint(struct ll_sb_info *sbi); +int lprocfs_ll_register_mountpoint(struct proc_dir_entry *parent, + struct super_block *sb); +int lprocfs_ll_register_obd(struct super_block *sb, const char *obdname); +void lprocfs_ll_unregister_mountpoint(struct ll_sb_info *sbi); void ll_stats_ops_tally(struct ll_sb_info *sbi, int op, int count); extern struct lprocfs_vars lprocfs_llite_obd_vars[]; #else -static inline int lprocfs_register_mountpoint(struct proc_dir_entry *parent, - struct super_block *sb, char *osc, char *mdc){return 0;} -static inline void lprocfs_unregister_mountpoint(struct ll_sb_info *sbi) {} +static inline int lprocfs_ll_register_mountpoint(struct proc_dir_entry *parent, + struct super_block *sb) {return 0; } +static inline int lprocfs_ll_register_obd(struct super_block *sb, + const char *obdname) {return 0; } +static inline void lprocfs_ll_unregister_mountpoint(struct ll_sb_info *sbi) {} static void ll_stats_ops_tally(struct ll_sb_info *sbi, int op, int count) {} #endif diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index e739193..a1a8163 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -370,7 +370,9 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt, if (!(data->ocd_connect_flags & OBD_CONNECT_MAX_EASIZE)) { LCONSOLE_INFO("%s: disabling xattr cache due to " "unknown maximum xattr size.\n", dt); - } else { + } else if (!sbi->ll_xattr_cache_set) { + /* If xattr_cache is already set (no matter 0 or 1) + * during processing llog, it won't be enabled here. */ sbi->ll_flags |= LL_SBI_XATTR_CACHE; sbi->ll_xattr_cache_enabled = 1; } @@ -577,12 +579,18 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt, OBD_FREE_PTR(data); if (osfs != NULL) OBD_FREE_PTR(osfs); - if (proc_lustre_fs_root != NULL) { - err = lprocfs_register_mountpoint(proc_lustre_fs_root, sb, - dt, md); + + if (sbi->ll_proc_root != NULL) { + err = lprocfs_ll_register_obd(sb, dt); if (err < 0) { - CERROR("%s: could not register mount in lprocfs: " - "rc = %d\n", ll_get_fsname(sb, NULL, 0), err); + CERROR("%s: could not register %s in llite: rc = %d\n", + dt, ll_get_fsname(sb, NULL, 0), err); + err = 0; + } + err = lprocfs_ll_register_obd(sb, md); + if (err < 0) { + CERROR("%s: could not register %s in llite: rc = %d\n", + md, ll_get_fsname(sb, NULL, 0), err); err = 0; } } @@ -683,22 +691,22 @@ int ll_set_default_mdsize(struct ll_sb_info *sbi, int lmmsize) static void client_common_put_super(struct super_block *sb) { - struct ll_sb_info *sbi = ll_s2sbi(sb); - ENTRY; + struct ll_sb_info *sbi = ll_s2sbi(sb); + ENTRY; - cl_sb_fini(sb); + cl_sb_fini(sb); obd_fid_fini(sbi->ll_dt_exp->exp_obd); - obd_disconnect(sbi->ll_dt_exp); - sbi->ll_dt_exp = NULL; + obd_disconnect(sbi->ll_dt_exp); + sbi->ll_dt_exp = NULL; - lprocfs_unregister_mountpoint(sbi); + lprocfs_ll_unregister_mountpoint(sbi); obd_fid_fini(sbi->ll_md_exp->exp_obd); - obd_disconnect(sbi->ll_md_exp); - sbi->ll_md_exp = NULL; + obd_disconnect(sbi->ll_md_exp); + sbi->ll_md_exp = NULL; - EXIT; + EXIT; } void ll_kill_super(struct super_block *sb) @@ -925,22 +933,24 @@ static inline int ll_bdi_register(struct backing_dev_info *bdi) int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) { - struct lustre_profile *lprof = NULL; - struct lustre_sb_info *lsi = s2lsi(sb); - struct ll_sb_info *sbi; - char *dt = NULL, *md = NULL; - char *profilenm = get_profile_name(sb); - struct config_llog_instance *cfg; - /* %p for void* in printf needs 16+2 characters: 0xffffffffffffffff */ - const int instlen = sizeof(cfg->cfg_instance) * 2 + 2; - int err; - ENTRY; + struct lustre_profile *lprof = NULL; + struct lustre_sb_info *lsi = s2lsi(sb); + struct ll_sb_info *sbi; + char *dt = NULL, *md = NULL; + char *profilenm = get_profile_name(sb); + struct config_llog_instance *cfg; + /* %p for void* in printf needs 16+2 characters: 0xffffffffffffffff */ + const int instlen = sizeof(cfg->cfg_instance) * 2 + 2; + int md_len = 0; + int dt_len = 0; + int err; + ENTRY; - CDEBUG(D_VFSTRACE, "VFS Op: sb %p\n", sb); + CDEBUG(D_VFSTRACE, "VFS Op: sb %p\n", sb); - OBD_ALLOC_PTR(cfg); - if (cfg == NULL) - RETURN(-ENOMEM); + OBD_ALLOC_PTR(cfg); + if (cfg == NULL) + RETURN(-ENOMEM); try_module_get(THIS_MODULE); @@ -952,9 +962,9 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) RETURN(-ENOMEM); } - err = ll_options(lsi->lsi_lmd->lmd_opts, &sbi->ll_flags); - if (err) - GOTO(out_free, err); + err = ll_options(lsi->lsi_lmd->lmd_opts, &sbi->ll_flags); + if (err) + GOTO(out_free, err); err = bdi_init(&lsi->lsi_bdi); if (err) @@ -969,57 +979,73 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) if (err) GOTO(out_free, err); - sb->s_bdi = &lsi->lsi_bdi; + sb->s_bdi = &lsi->lsi_bdi; #ifndef HAVE_DCACHE_LOCK /* kernel >= 2.6.38 store dentry operations in sb->s_d_op. */ sb->s_d_op = &ll_d_ops; #endif - /* Generate a string unique to this super, in case some joker tries - to mount the same fs at two mount points. - Use the address of the super itself.*/ - cfg->cfg_instance = sb; - cfg->cfg_uuid = lsi->lsi_llsbi->ll_sb_uuid; + /* Call lprocfs_ll_register_mountpoint() before lustre_process_log() + * so that "llite.*.*" params can be processed correctly. */ + if (proc_lustre_fs_root != NULL) { + err = lprocfs_ll_register_mountpoint(proc_lustre_fs_root, sb); + if (err < 0) { + CERROR("%s: could not register mountpoint in llite: " + "rc = %d\n", ll_get_fsname(sb, NULL, 0), err); + err = 0; + } + } + + /* Generate a string unique to this super, in case some joker tries + to mount the same fs at two mount points. + Use the address of the super itself.*/ + cfg->cfg_instance = sb; + cfg->cfg_uuid = lsi->lsi_llsbi->ll_sb_uuid; cfg->cfg_callback = class_config_llog_handler; cfg->cfg_sub_clds = CONFIG_SUB_CLIENT; - /* set up client obds */ - err = lustre_process_log(sb, profilenm, cfg); + /* set up client obds */ + err = lustre_process_log(sb, profilenm, cfg); if (err < 0) - GOTO(out_free, err); - - /* Profile set with LCFG_MOUNTOPT so we can find our mdc and osc obds */ - lprof = class_get_profile(profilenm); - if (lprof == NULL) { - LCONSOLE_ERROR_MSG(0x156, "The client profile '%s' could not be" - " read from the MGS. Does that filesystem " - "exist?\n", profilenm); - GOTO(out_free, err = -EINVAL); - } - CDEBUG(D_CONFIG, "Found profile %s: mdc=%s osc=%s\n", profilenm, - lprof->lp_md, lprof->lp_dt); - - OBD_ALLOC(dt, strlen(lprof->lp_dt) + instlen + 2); - if (!dt) - GOTO(out_free, err = -ENOMEM); - sprintf(dt, "%s-%p", lprof->lp_dt, cfg->cfg_instance); - - OBD_ALLOC(md, strlen(lprof->lp_md) + instlen + 2); - if (!md) - GOTO(out_free, err = -ENOMEM); - sprintf(md, "%s-%p", lprof->lp_md, cfg->cfg_instance); - - /* connections, registrations, sb setup */ - err = client_common_fill_super(sb, md, dt, mnt); + GOTO(out_proc, err); + + /* Profile set with LCFG_MOUNTOPT so we can find our mdc and osc obds */ + lprof = class_get_profile(profilenm); + if (lprof == NULL) { + LCONSOLE_ERROR_MSG(0x156, "The client profile '%s' could not be" + " read from the MGS. Does that filesystem " + "exist?\n", profilenm); + GOTO(out_proc, err = -EINVAL); + } + CDEBUG(D_CONFIG, "Found profile %s: mdc=%s osc=%s\n", profilenm, + lprof->lp_md, lprof->lp_dt); + + dt_len = strlen(lprof->lp_dt) + instlen + 2; + OBD_ALLOC(dt, dt_len); + if (!dt) + GOTO(out_proc, err = -ENOMEM); + snprintf(dt, dt_len - 1, "%s-%p", lprof->lp_dt, cfg->cfg_instance); + + md_len = strlen(lprof->lp_md) + instlen + 2; + OBD_ALLOC(md, md_len); + if (!md) + GOTO(out_proc, err = -ENOMEM); + snprintf(md, md_len - 1, "%s-%p", lprof->lp_md, cfg->cfg_instance); + + /* connections, registrations, sb setup */ + err = client_common_fill_super(sb, md, dt, mnt); if (err < 0) - GOTO(out_free, err); + GOTO(out_proc, err); sbi->ll_client_common_fill_super_succeeded = 1; +out_proc: + if (err < 0) + lprocfs_ll_unregister_mountpoint(sbi); out_free: if (md) - OBD_FREE(md, strlen(lprof->lp_md) + instlen + 2); + OBD_FREE(md, md_len); if (dt) - OBD_FREE(dt, strlen(lprof->lp_dt) + instlen + 2); + OBD_FREE(dt, dt_len); if (lprof != NULL) class_put_profile(lprof); if (err) diff --git a/lustre/llite/lproc_llite.c b/lustre/llite/lproc_llite.c index 9166966..2f61758 100644 --- a/lustre/llite/lproc_llite.c +++ b/lustre/llite/lproc_llite.c @@ -259,6 +259,7 @@ static ssize_t ll_xattr_cache_seq_write(struct file *file, return -ENOTSUPP; sbi->ll_xattr_cache_enabled = val; + sbi->ll_xattr_cache_set = 1; return count; } @@ -1235,14 +1236,12 @@ static const char *ra_stat_string[] = { LPROC_SEQ_FOPS_RO_TYPE(llite, name); LPROC_SEQ_FOPS_RO_TYPE(llite, uuid); -int lprocfs_register_mountpoint(struct proc_dir_entry *parent, - struct super_block *sb, char *osc, char *mdc) +int lprocfs_ll_register_mountpoint(struct proc_dir_entry *parent, + struct super_block *sb) { struct lprocfs_vars lvars[2]; struct lustre_sb_info *lsi = s2lsi(sb); struct ll_sb_info *sbi = ll_s2sbi(sb); - struct obd_device *obd; - struct proc_dir_entry *dir; char name[MAX_STRING_SIZE + 1], *ptr; int err, id, len, rc; ENTRY; @@ -1253,8 +1252,6 @@ int lprocfs_register_mountpoint(struct proc_dir_entry *parent, lvars[0].name = name; LASSERT(sbi != NULL); - LASSERT(mdc != NULL); - LASSERT(osc != NULL); /* Get fsname */ len = strlen(lsi->lsi_lmd->lmd_profile); @@ -1278,88 +1275,91 @@ int lprocfs_register_mountpoint(struct proc_dir_entry *parent, if (rc) CWARN("Error adding the dump_page_cache file\n"); - rc = lprocfs_seq_create(sbi->ll_proc_root, "extents_stats", 0644, - &ll_rw_extents_stats_fops, sbi); - if (rc) - CWARN("Error adding the extent_stats file\n"); - - rc = lprocfs_seq_create(sbi->ll_proc_root, "extents_stats_per_process", - 0644, &ll_rw_extents_stats_pp_fops, sbi); - if (rc) - CWARN("Error adding the extents_stats_per_process file\n"); - - rc = lprocfs_seq_create(sbi->ll_proc_root, "offset_stats", 0644, - &ll_rw_offset_stats_fops, sbi); - if (rc) - CWARN("Error adding the offset_stats file\n"); - - /* File operations stats */ - sbi->ll_stats = lprocfs_alloc_stats(LPROC_LL_FILE_OPCODES, - LPROCFS_STATS_FLAG_NONE); - if (sbi->ll_stats == NULL) - GOTO(out, err = -ENOMEM); - /* do counter init */ - for (id = 0; id < LPROC_LL_FILE_OPCODES; id++) { - __u32 type = llite_opcode_table[id].type; - void *ptr = NULL; - if (type & LPROCFS_TYPE_REGS) - ptr = "regs"; - else if (type & LPROCFS_TYPE_BYTES) - ptr = "bytes"; - else if (type & LPROCFS_TYPE_PAGES) - ptr = "pages"; - lprocfs_counter_init(sbi->ll_stats, - llite_opcode_table[id].opcode, - (type & LPROCFS_CNTR_AVGMINMAX), - llite_opcode_table[id].opname, ptr); - } - err = lprocfs_register_stats(sbi->ll_proc_root, "stats", sbi->ll_stats); - if (err) - GOTO(out, err); - - sbi->ll_ra_stats = lprocfs_alloc_stats(ARRAY_SIZE(ra_stat_string), - LPROCFS_STATS_FLAG_NONE); - if (sbi->ll_ra_stats == NULL) - GOTO(out, err = -ENOMEM); + rc = lprocfs_seq_create(sbi->ll_proc_root, "extents_stats", 0644, + &ll_rw_extents_stats_fops, sbi); + if (rc) + CWARN("Error adding the extent_stats file\n"); - for (id = 0; id < ARRAY_SIZE(ra_stat_string); id++) - lprocfs_counter_init(sbi->ll_ra_stats, id, 0, - ra_stat_string[id], "pages"); - err = lprocfs_register_stats(sbi->ll_proc_root, "read_ahead_stats", - sbi->ll_ra_stats); - if (err) - GOTO(out, err); + rc = lprocfs_seq_create(sbi->ll_proc_root, "extents_stats_per_process", + 0644, &ll_rw_extents_stats_pp_fops, sbi); + if (rc) + CWARN("Error adding the extents_stats_per_process file\n"); + rc = lprocfs_seq_create(sbi->ll_proc_root, "offset_stats", 0644, + &ll_rw_offset_stats_fops, sbi); + if (rc) + CWARN("Error adding the offset_stats file\n"); - err = lprocfs_add_vars(sbi->ll_proc_root, lprocfs_llite_obd_vars, sb); + /* File operations stats */ + sbi->ll_stats = lprocfs_alloc_stats(LPROC_LL_FILE_OPCODES, + LPROCFS_STATS_FLAG_NONE); + if (sbi->ll_stats == NULL) + GOTO(out, err = -ENOMEM); + /* do counter init */ + for (id = 0; id < LPROC_LL_FILE_OPCODES; id++) { + __u32 type = llite_opcode_table[id].type; + void *ptr = NULL; + if (type & LPROCFS_TYPE_REGS) + ptr = "regs"; + else if (type & LPROCFS_TYPE_BYTES) + ptr = "bytes"; + else if (type & LPROCFS_TYPE_PAGES) + ptr = "pages"; + lprocfs_counter_init(sbi->ll_stats, + llite_opcode_table[id].opcode, + (type & LPROCFS_CNTR_AVGMINMAX), + llite_opcode_table[id].opname, ptr); + } + err = lprocfs_register_stats(sbi->ll_proc_root, "stats", sbi->ll_stats); if (err) GOTO(out, err); - /* MDC info */ - obd = class_name2obd(mdc); - - LASSERT(obd != NULL); - LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC); - LASSERT(obd->obd_type->typ_name != NULL); - - dir = proc_mkdir(obd->obd_type->typ_name, sbi->ll_proc_root); - if (dir == NULL) + sbi->ll_ra_stats = lprocfs_alloc_stats(ARRAY_SIZE(ra_stat_string), + LPROCFS_STATS_FLAG_NONE); + if (sbi->ll_ra_stats == NULL) GOTO(out, err = -ENOMEM); - snprintf(name, MAX_STRING_SIZE, "common_name"); - lvars[0].fops = &llite_name_fops; - err = lprocfs_add_vars(dir, lvars, obd); + for (id = 0; id < ARRAY_SIZE(ra_stat_string); id++) + lprocfs_counter_init(sbi->ll_ra_stats, id, 0, + ra_stat_string[id], "pages"); + err = lprocfs_register_stats(sbi->ll_proc_root, "read_ahead_stats", + sbi->ll_ra_stats); if (err) GOTO(out, err); - snprintf(name, MAX_STRING_SIZE, "uuid"); - lvars[0].fops = &llite_uuid_fops; - err = lprocfs_add_vars(dir, lvars, obd); + + err = lprocfs_add_vars(sbi->ll_proc_root, lprocfs_llite_obd_vars, sb); if (err) GOTO(out, err); - /* OSC */ - obd = class_name2obd(osc); +out: + if (err) { + lprocfs_remove(&sbi->ll_proc_root); + lprocfs_free_stats(&sbi->ll_ra_stats); + lprocfs_free_stats(&sbi->ll_stats); + } + RETURN(err); +} + +int lprocfs_ll_register_obd(struct super_block *sb, const char *obdname) +{ + struct lprocfs_vars lvars[2]; + struct ll_sb_info *sbi = ll_s2sbi(sb); + struct obd_device *obd; + struct proc_dir_entry *dir; + char name[MAX_STRING_SIZE + 1]; + int err; + ENTRY; + + memset(lvars, 0, sizeof(lvars)); + + name[MAX_STRING_SIZE] = '\0'; + lvars[0].name = name; + + LASSERT(sbi != NULL); + LASSERT(obdname != NULL); + + obd = class_name2obd(obdname); LASSERT(obd != NULL); LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC); @@ -1378,6 +1378,9 @@ int lprocfs_register_mountpoint(struct proc_dir_entry *parent, snprintf(name, MAX_STRING_SIZE, "uuid"); lvars[0].fops = &llite_uuid_fops; err = lprocfs_add_vars(dir, lvars, obd); + if (err) + GOTO(out, err); + out: if (err) { lprocfs_remove(&sbi->ll_proc_root); @@ -1387,7 +1390,7 @@ out: RETURN(err); } -void lprocfs_unregister_mountpoint(struct ll_sb_info *sbi) +void lprocfs_ll_unregister_mountpoint(struct ll_sb_info *sbi) { if (sbi->ll_proc_root) { lprocfs_remove(&sbi->ll_proc_root); diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index 10054d9..4c1a314 100755 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -5366,6 +5366,34 @@ test_76c() { } run_test 76c "verify changelog_mask is applied with set_param -P" +test_76d() { #LU-9399 + setupall + + local xattr_cache="llite.*.xattr_cache" + local cmd="$LCTL get_param -n $xattr_cache | head -1" + local new=$((($(eval $cmd) + 1) % 2)) + + echo "lctl set_param -P llite.*.xattr_cache=$new" + do_facet mgs $LCTL set_param -P $xattr_cache=$new || + error "Can't change xattr_cache" + wait_update $HOSTNAME "$cmd" "$new" + + echo "Check $xattr_cache on client $MOUNT" + umount_client $MOUNT || error "umount $MOUNT failed" + mount_client $MOUNT || error "mount $MOUNT failed" + [ $(eval $cmd) -eq $new ] || + error "$xattr_cache != $new on client $MOUNT" + + echo "Check $xattr_cache on the new client $MOUNT2" + mount_client $MOUNT2 || error "mount $MOUNT2 failed" + [ $(eval $cmd) -eq $new ] || + error "$xattr_cache != $new on client $MOUNT2" + umount_client $MOUNT2 || error "umount $MOUNT2 failed" + + stopall +} +run_test 76d "verify llite.*.xattr_cache can be set by 'set_param -P' correctly" + test_77() { # LU-3445 local server_version=$(lustre_version_code $SINGLEMDS) [[ $server_version -ge $(version_code 2.8.55) ]] || -- 1.8.3.1