[Cluster-devel] [GFS2 PATCH] gfs2: initialize tr_ail1_list when creating transactions

[Cluster-devel] [GFS2 PATCH] gfs2: initialize tr_ail1_list when creating transactions

[Bob Peterson]

Hi,

In function gfs2_trans_begin, new transactions were created but their
ail1 list, tr_ail1_list was never initialized. Therefore it relied
upon other circumstances when the list became empty.
This patch adds proper initialization of the list.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/trans.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index 9d4227330de4..d0efa322f940 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -53,6 +53,7 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
 						   sizeof(u64));
 	INIT_LIST_HEAD(&tr->tr_databuf);
 	INIT_LIST_HEAD(&tr->tr_buf);
+	INIT_LIST_HEAD(&tr->tr_ail1_list);
 
 	sb_start_intwrite(sdp->sd_vfs);
 

[Cluster-devel] [GFS2 PATCH] gfs2: initialize tr_ail1_list when creating transactions

[Andreas Gruenbacher]

On Thu, Nov 14, 2019 at 6:10 PM Bob Peterson <rpeterso@redhat.com> wrote:
> In function gfs2_trans_begin, new transactions were created but their
> ail1 list, tr_ail1_list was never initialized. Therefore it relied
> upon other circumstances when the list became empty.
> This patch adds proper initialization of the list.

Could you please give a better explanation for this change?

> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>  fs/gfs2/trans.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
> index 9d4227330de4..d0efa322f940 100644
> --- a/fs/gfs2/trans.c
> +++ b/fs/gfs2/trans.c
> @@ -53,6 +53,7 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
>                                                    sizeof(u64));
>         INIT_LIST_HEAD(&tr->tr_databuf);
>         INIT_LIST_HEAD(&tr->tr_buf);
> +       INIT_LIST_HEAD(&tr->tr_ail1_list);
>
>         sb_start_intwrite(sdp->sd_vfs);
>
>

What code path accesses tr_ail1_list before it is initialized in gfs2_log_flush?
Should the INIT_LIST_HEAD(&tr->tr_ail1_list) in gfs2_log_flush
consequently be removed?

Thanks,
Andreas

[Cluster-devel] [GFS2 PATCH] gfs2: initialize tr_ail1_list when creating transactions

[Bob Peterson]

----- Original Message -----
> On Thu, Nov 14, 2019 at 6:10 PM Bob Peterson <rpeterso@redhat.com> wrote:
> > In function gfs2_trans_begin, new transactions were created but their
> > ail1 list, tr_ail1_list was never initialized. Therefore it relied
> > upon other circumstances when the list became empty.
> > This patch adds proper initialization of the list.
> 
> Could you please give a better explanation for this change?

Let's drop it for now. I'll reevaluate if it's still needed.

Bob