* [Cluster-devel] [PATCH] gfs2: Fix xattr fsync
@ 2017-10-04 18:17 Andreas Gruenbacher
2017-10-23 20:58 ` Andreas Gruenbacher
2017-10-24 10:46 ` Andrew Price
0 siblings, 2 replies; 3+ messages in thread
From: Andreas Gruenbacher @ 2017-10-04 18:17 UTC (permalink / raw)
To: cluster-devel.redhat.com
Make sure that changing xattrs marks the corresponding inode dirty so
that a subsequent fsync will sync those changes to disk. We set
I_DIRTY_SYNC as well as I_DIRTY_DATASYNC so that both fsync and
fdatasync will sync xattr changes: xattrs can contain information
critical to how the data can be accessed, so we don't want fdatasync
to skip them.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/xattr.c | 40 ++++++++--------------------------------
1 file changed, 8 insertions(+), 32 deletions(-)
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 36ca70924935..588d4f21a273 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -231,7 +231,6 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_rgrpd *rgd;
struct gfs2_holder rg_gh;
- struct buffer_head *dibh;
__be64 *dataptrs;
u64 bn = 0;
u64 bstart = 0;
@@ -308,13 +307,8 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
ea->ea_num_ptrs = 0;
}
- error = gfs2_meta_inode_buffer(ip, &dibh);
- if (!error) {
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
- gfs2_trans_add_meta(ip->i_gl, dibh);
- gfs2_dinode_out(ip, dibh->b_data);
- brelse(dibh);
- }
+ ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
gfs2_trans_end(sdp);
@@ -755,7 +749,6 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
ea_skeleton_call_t skeleton_call, void *private)
{
struct gfs2_alloc_parms ap = { .target = blks };
- struct buffer_head *dibh;
int error;
error = gfs2_rindex_update(GFS2_SB(&ip->i_inode));
@@ -780,13 +773,8 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
if (error)
goto out_end_trans;
- error = gfs2_meta_inode_buffer(ip, &dibh);
- if (!error) {
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
- gfs2_trans_add_meta(ip->i_gl, dibh);
- gfs2_dinode_out(ip, dibh->b_data);
- brelse(dibh);
- }
+ ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
out_end_trans:
gfs2_trans_end(GFS2_SB(&ip->i_inode));
@@ -897,7 +885,6 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
struct gfs2_ea_header *ea, struct ea_set *es)
{
struct gfs2_ea_request *er = es->es_er;
- struct buffer_head *dibh;
int error;
error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + 2 * RES_EATTR, 0);
@@ -914,14 +901,9 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
if (es->es_el)
ea_set_remove_stuffed(ip, es->es_el);
- error = gfs2_meta_inode_buffer(ip, &dibh);
- if (error)
- goto out;
ip->i_inode.i_ctime = current_time(&ip->i_inode);
- gfs2_trans_add_meta(ip->i_gl, dibh);
- gfs2_dinode_out(ip, dibh->b_data);
- brelse(dibh);
-out:
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+
gfs2_trans_end(GFS2_SB(&ip->i_inode));
return error;
}
@@ -1117,7 +1099,6 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
{
struct gfs2_ea_header *ea = el->el_ea;
struct gfs2_ea_header *prev = el->el_prev;
- struct buffer_head *dibh;
int error;
error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + RES_EATTR, 0);
@@ -1138,13 +1119,8 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
ea->ea_type = GFS2_EATYPE_UNUSED;
}
- error = gfs2_meta_inode_buffer(ip, &dibh);
- if (!error) {
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
- gfs2_trans_add_meta(ip->i_gl, dibh);
- gfs2_dinode_out(ip, dibh->b_data);
- brelse(dibh);
- }
+ ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
gfs2_trans_end(GFS2_SB(&ip->i_inode));
--
2.13.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Cluster-devel] [PATCH] gfs2: Fix xattr fsync
2017-10-04 18:17 [Cluster-devel] [PATCH] gfs2: Fix xattr fsync Andreas Gruenbacher
@ 2017-10-23 20:58 ` Andreas Gruenbacher
2017-10-24 10:46 ` Andrew Price
1 sibling, 0 replies; 3+ messages in thread
From: Andreas Gruenbacher @ 2017-10-23 20:58 UTC (permalink / raw)
To: cluster-devel.redhat.com
I forgot to mention:
On 4 October 2017 at 20:17, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> Make sure that changing xattrs marks the corresponding inode dirty so
> that a subsequent fsync will sync those changes to disk. We set
> I_DIRTY_SYNC as well as I_DIRTY_DATASYNC so that both fsync and
> fdatasync will sync xattr changes: xattrs can contain information
> critical to how the data can be accessed, so we don't want fdatasync
> to skip them.
Fixes xfstest generic/066.
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/xattr.c | 40 ++++++++--------------------------------
> 1 file changed, 8 insertions(+), 32 deletions(-)
>
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index 36ca70924935..588d4f21a273 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -231,7 +231,6 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct gfs2_rgrpd *rgd;
> struct gfs2_holder rg_gh;
> - struct buffer_head *dibh;
> __be64 *dataptrs;
> u64 bn = 0;
> u64 bstart = 0;
> @@ -308,13 +307,8 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
> ea->ea_num_ptrs = 0;
> }
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> gfs2_trans_end(sdp);
>
> @@ -755,7 +749,6 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> ea_skeleton_call_t skeleton_call, void *private)
> {
> struct gfs2_alloc_parms ap = { .target = blks };
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_rindex_update(GFS2_SB(&ip->i_inode));
> @@ -780,13 +773,8 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> if (error)
> goto out_end_trans;
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> out_end_trans:
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
> @@ -897,7 +885,6 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
> struct gfs2_ea_header *ea, struct ea_set *es)
> {
> struct gfs2_ea_request *er = es->es_er;
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + 2 * RES_EATTR, 0);
> @@ -914,14 +901,9 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
> if (es->es_el)
> ea_set_remove_stuffed(ip, es->es_el);
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (error)
> - goto out;
> ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> -out:
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
> +
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
> return error;
> }
> @@ -1117,7 +1099,6 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
> {
> struct gfs2_ea_header *ea = el->el_ea;
> struct gfs2_ea_header *prev = el->el_prev;
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + RES_EATTR, 0);
> @@ -1138,13 +1119,8 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
> ea->ea_type = GFS2_EATYPE_UNUSED;
> }
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
>
> --
> 2.13.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [Cluster-devel] [PATCH] gfs2: Fix xattr fsync
2017-10-04 18:17 [Cluster-devel] [PATCH] gfs2: Fix xattr fsync Andreas Gruenbacher
2017-10-23 20:58 ` Andreas Gruenbacher
@ 2017-10-24 10:46 ` Andrew Price
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Price @ 2017-10-24 10:46 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Wed, Oct 04, 2017 at 08:17:38PM +0200, Andreas Gruenbacher wrote:
> Make sure that changing xattrs marks the corresponding inode dirty so
> that a subsequent fsync will sync those changes to disk. We set
> I_DIRTY_SYNC as well as I_DIRTY_DATASYNC so that both fsync and
> fdatasync will sync xattr changes: xattrs can contain information
> critical to how the data can be accessed, so we don't want fdatasync
> to skip them.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Andrew Price <anprice@redhat.com>
The new __mark_inode_dirty() -> gfs2_dirty_inode() path looks ok in this context.
Andy
> ---
> fs/gfs2/xattr.c | 40 ++++++++--------------------------------
> 1 file changed, 8 insertions(+), 32 deletions(-)
>
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index 36ca70924935..588d4f21a273 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -231,7 +231,6 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct gfs2_rgrpd *rgd;
> struct gfs2_holder rg_gh;
> - struct buffer_head *dibh;
> __be64 *dataptrs;
> u64 bn = 0;
> u64 bstart = 0;
> @@ -308,13 +307,8 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
> ea->ea_num_ptrs = 0;
> }
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> gfs2_trans_end(sdp);
>
> @@ -755,7 +749,6 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> ea_skeleton_call_t skeleton_call, void *private)
> {
> struct gfs2_alloc_parms ap = { .target = blks };
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_rindex_update(GFS2_SB(&ip->i_inode));
> @@ -780,13 +773,8 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> if (error)
> goto out_end_trans;
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> out_end_trans:
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
> @@ -897,7 +885,6 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
> struct gfs2_ea_header *ea, struct ea_set *es)
> {
> struct gfs2_ea_request *er = es->es_er;
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + 2 * RES_EATTR, 0);
> @@ -914,14 +901,9 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
> if (es->es_el)
> ea_set_remove_stuffed(ip, es->es_el);
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (error)
> - goto out;
> ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> -out:
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
> +
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
> return error;
> }
> @@ -1117,7 +1099,6 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
> {
> struct gfs2_ea_header *ea = el->el_ea;
> struct gfs2_ea_header *prev = el->el_prev;
> - struct buffer_head *dibh;
> int error;
>
> error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + RES_EATTR, 0);
> @@ -1138,13 +1119,8 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
> ea->ea_type = GFS2_EATYPE_UNUSED;
> }
>
> - error = gfs2_meta_inode_buffer(ip, &dibh);
> - if (!error) {
> - ip->i_inode.i_ctime = current_time(&ip->i_inode);
> - gfs2_trans_add_meta(ip->i_gl, dibh);
> - gfs2_dinode_out(ip, dibh->b_data);
> - brelse(dibh);
> - }
> + ip->i_inode.i_ctime = current_time(&ip->i_inode);
> + __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>
> gfs2_trans_end(GFS2_SB(&ip->i_inode));
>
> --
> 2.13.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-24 10:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-04 18:17 [Cluster-devel] [PATCH] gfs2: Fix xattr fsync Andreas Gruenbacher
2017-10-23 20:58 ` Andreas Gruenbacher
2017-10-24 10:46 ` Andrew Price
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).