| Message ID | 1490665245-15374-1-git-send-email-junxiao.bi@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Acked-by: Joseph Qi <jiangqi903@gmail.com> On 17/3/28 09:40, Junxiao Bi wrote: > Configfs is the interface for ocfs2-tools to set configure to > kernel. Change heartbeat dead threshold name in configfs will > cause compatible issue, so revert it. > > Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") > Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> > --- > fs/ocfs2/cluster/heartbeat.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c > index f6e871760f8d..0da0332725aa 100644 > --- a/fs/ocfs2/cluster/heartbeat.c > +++ b/fs/ocfs2/cluster/heartbeat.c > @@ -2242,13 +2242,13 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group, > spin_unlock(&o2hb_live_lock); > } > > -static ssize_t o2hb_heartbeat_group_threshold_show(struct config_item *item, > +static ssize_t o2hb_heartbeat_group_dead_threshold_show(struct config_item *item, > char *page) > { > return sprintf(page, "%u\n", o2hb_dead_threshold); > } > > -static ssize_t o2hb_heartbeat_group_threshold_store(struct config_item *item, > +static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *item, > const char *page, size_t count) > { > unsigned long tmp; > @@ -2297,11 +2297,11 @@ static ssize_t o2hb_heartbeat_group_mode_store(struct config_item *item, > > } > > -CONFIGFS_ATTR(o2hb_heartbeat_group_, threshold); > +CONFIGFS_ATTR(o2hb_heartbeat_group_, dead_threshold); > CONFIGFS_ATTR(o2hb_heartbeat_group_, mode); > > static struct configfs_attribute *o2hb_heartbeat_group_attrs[] = { > - &o2hb_heartbeat_group_attr_threshold, > + &o2hb_heartbeat_group_attr_dead_threshold, > &o2hb_heartbeat_group_attr_mode, > NULL, > };
On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: > Configfs is the interface for ocfs2-tools to set configure to > kernel. Change heartbeat dead threshold name in configfs will > cause compatible issue, so revert it. > > Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") I don't get it. 45b997737a80 was merged nearly two years ago, so isn't it a bit late to fix compatibility issues?
On 03/29/2017 06:31 AM, Andrew Morton wrote: > On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: > >> Configfs is the interface for ocfs2-tools to set configure to >> kernel. Change heartbeat dead threshold name in configfs will >> cause compatible issue, so revert it. >> >> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") > > I don't get it. 45b997737a80 was merged nearly two years ago, so isn't > it a bit late to fix compatibility issues? > This compatibility will not cause ocfs2 down, just some configure (hb dead threshold) lose effect. If someone want to use the new kernel, they should apply this fix. Thanks, Junxiao.
On Wed, 29 Mar 2017 09:07:08 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: > On 03/29/2017 06:31 AM, Andrew Morton wrote: > > On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: > > > >> Configfs is the interface for ocfs2-tools to set configure to > >> kernel. Change heartbeat dead threshold name in configfs will > >> cause compatible issue, so revert it. > >> > >> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") > > > > I don't get it. 45b997737a80 was merged nearly two years ago, so isn't > > it a bit late to fix compatibility issues? > > > This compatibility will not cause ocfs2 down, just some configure (hb > dead threshold) lose effect. If someone want to use the new kernel, they > should apply this fix. Well could someone please send a better changelog? One which carefully describes the present behaviour, what is wrong with it and how the patch fixes it? One reason for doing this is to permit effecitive patch review. Another reason is to permit others to decide whether the patch should be backported into -stable kernels. Yet another reason is so that maintainers of other kernels can determine whether this patch will fix behaviour which their users are reporting. Thanks.
On 17/3/29 09:07, Junxiao Bi wrote: > On 03/29/2017 06:31 AM, Andrew Morton wrote: >> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: >> >>> Configfs is the interface for ocfs2-tools to set configure to >>> kernel. Change heartbeat dead threshold name in configfs will >>> cause compatible issue, so revert it. >>> >>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") >> I don't get it. 45b997737a80 was merged nearly two years ago, so isn't >> it a bit late to fix compatibility issues? >> > This compatibility will not cause ocfs2 down, just some configure (hb > dead threshold) lose effect. If someone want to use the new kernel, they > should apply this fix. The threshold configuration file has default value in kernel, so it will only affect changing this value in user space. Thanks, Joseph > > Thanks, > Junxiao. > > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel
Hi Andrew, On 03/29/2017 11:31 AM, Andrew Morton wrote: > On Wed, 29 Mar 2017 09:07:08 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: > >> On 03/29/2017 06:31 AM, Andrew Morton wrote: >>> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote: >>> >>>> Configfs is the interface for ocfs2-tools to set configure to >>>> kernel. Change heartbeat dead threshold name in configfs will >>>> cause compatible issue, so revert it. >>>> >>>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") >>> >>> I don't get it. 45b997737a80 was merged nearly two years ago, so isn't >>> it a bit late to fix compatibility issues? >>> >> This compatibility will not cause ocfs2 down, just some configure (hb >> dead threshold) lose effect. If someone want to use the new kernel, they >> should apply this fix. > > Well could someone please send a better changelog? One which carefully > describes the present behaviour, what is wrong with it and how the > patch fixes it? A new one, please help review. Configfs is the interface for ocfs2-tools to set configure to kernel and there $configfs_dir/cluster/$clustername/heartbeat/dead_threshold is the one used to configure heartbeat dead threshold. Kernel has a default value of it but user can set O2CB_HEARTBEAT_THRESHOLD in /etc/sysconfig/o2cb to override it. Commit 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") changed heartbeat dead threshold name while ocfs2-tools not, so ocfs2-tools won't set this configure and default value is always used. So revert it. Thanks, Junxiao. > > One reason for doing this is to permit effecitive patch review. > > Another reason is to permit others to decide whether the patch should > be backported into -stable kernels. > > Yet another reason is so that maintainers of other kernels can > determine whether this patch will fix behaviour which their users are > reporting. > > Thanks. >
On 03/29/2017 12:01 PM, Joseph Qi wrote: > > > On 17/3/29 09:07, Junxiao Bi wrote: >> On 03/29/2017 06:31 AM, Andrew Morton wrote: >>> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> >>> wrote: >>> >>>> Configfs is the interface for ocfs2-tools to set configure to >>>> kernel. Change heartbeat dead threshold name in configfs will >>>> cause compatible issue, so revert it. >>>> >>>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and >>>> store methods") >>> I don't get it. 45b997737a80 was merged nearly two years ago, so isn't >>> it a bit late to fix compatibility issues? >>> >> This compatibility will not cause ocfs2 down, just some configure (hb >> dead threshold) lose effect. If someone want to use the new kernel, they >> should apply this fix. > The threshold configuration file has default value in kernel, so it will > only affect changing this value in user space. Right. Thank you. > > Thanks, > Joseph >> >> Thanks, >> Junxiao. >> >> _______________________________________________ >> Ocfs2-devel mailing list >> Ocfs2-devel@oss.oracle.com >> https://oss.oracle.com/mailman/listinfo/ocfs2-devel >
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index f6e871760f8d..0da0332725aa 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -2242,13 +2242,13 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group, spin_unlock(&o2hb_live_lock); } -static ssize_t o2hb_heartbeat_group_threshold_show(struct config_item *item, +static ssize_t o2hb_heartbeat_group_dead_threshold_show(struct config_item *item, char *page) { return sprintf(page, "%u\n", o2hb_dead_threshold); } -static ssize_t o2hb_heartbeat_group_threshold_store(struct config_item *item, +static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *item, const char *page, size_t count) { unsigned long tmp; @@ -2297,11 +2297,11 @@ static ssize_t o2hb_heartbeat_group_mode_store(struct config_item *item, } -CONFIGFS_ATTR(o2hb_heartbeat_group_, threshold); +CONFIGFS_ATTR(o2hb_heartbeat_group_, dead_threshold); CONFIGFS_ATTR(o2hb_heartbeat_group_, mode); static struct configfs_attribute *o2hb_heartbeat_group_attrs[] = { - &o2hb_heartbeat_group_attr_threshold, + &o2hb_heartbeat_group_attr_dead_threshold, &o2hb_heartbeat_group_attr_mode, NULL, };
Configfs is the interface for ocfs2-tools to set configure to kernel. Change heartbeat dead threshold name in configfs will cause compatible issue, so revert it. Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods") Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> --- fs/ocfs2/cluster/heartbeat.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)