Commit 26f71445 authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers

xfs: remote attribute tail zeroing does too much

When an attribute data does not fill then entire remote block, we
zero the remaining part of the buffer. This, however, needs to take
into account that the buffer has a header, and so the offset where
zeroing starts and the length of zeroing need to take this into
account. Otherwise we end up with zeros over the end of the
attribute value when CRCs are enabled.

While there, make sure we only ask to map an extent that covers the
remaining range of the attribute, rather than asking every time for
the full length of remote data. If the remote attribute blocks are
contiguous with other parts of the attribute tree, it will map those
blocks as well and we can potentially zero them incorrectly. We can
also get buffer size mistmatches when trying to read or remove the
remote attribute, and this can lead to not finding the correct
buffer when looking it up in cache.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBen Myers <bpm@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>

(cherry picked from commit 4af3644c)
parent 551b382f
...@@ -296,10 +296,7 @@ xfs_attr_rmtval_set( ...@@ -296,10 +296,7 @@ xfs_attr_rmtval_set(
* and we may not need that many, so we have to handle this when * and we may not need that many, so we have to handle this when
* allocating the blocks below. * allocating the blocks below.
*/ */
if (!crcs) blkcnt = xfs_attr3_rmt_blocks(mp, args->valuelen);
blkcnt = XFS_B_TO_FSB(mp, args->valuelen);
else
blkcnt = xfs_attr3_rmt_blocks(mp, args->valuelen);
error = xfs_bmap_first_unused(args->trans, args->dp, blkcnt, &lfileoff, error = xfs_bmap_first_unused(args->trans, args->dp, blkcnt, &lfileoff,
XFS_ATTR_FORK); XFS_ATTR_FORK);
...@@ -394,8 +391,11 @@ xfs_attr_rmtval_set( ...@@ -394,8 +391,11 @@ xfs_attr_rmtval_set(
*/ */
lblkno = args->rmtblkno; lblkno = args->rmtblkno;
valuelen = args->valuelen; valuelen = args->valuelen;
blkcnt = args->rmtblkcnt;
while (valuelen > 0) { while (valuelen > 0) {
int byte_cnt; int byte_cnt;
int hdr_size;
int dblkcnt;
char *buf; char *buf;
/* /*
...@@ -404,7 +404,7 @@ xfs_attr_rmtval_set( ...@@ -404,7 +404,7 @@ xfs_attr_rmtval_set(
xfs_bmap_init(args->flist, args->firstblock); xfs_bmap_init(args->flist, args->firstblock);
nmap = 1; nmap = 1;
error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno, error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt, &map, &nmap, blkcnt, &map, &nmap,
XFS_BMAPI_ATTRFORK); XFS_BMAPI_ATTRFORK);
if (error) if (error)
return(error); return(error);
...@@ -413,26 +413,25 @@ xfs_attr_rmtval_set( ...@@ -413,26 +413,25 @@ xfs_attr_rmtval_set(
(map.br_startblock != HOLESTARTBLOCK)); (map.br_startblock != HOLESTARTBLOCK));
dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock), dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock),
blkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount); dblkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
bp = xfs_buf_get(mp->m_ddev_targp, dblkno, blkcnt, 0); bp = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt, 0);
if (!bp) if (!bp)
return ENOMEM; return ENOMEM;
bp->b_ops = &xfs_attr3_rmt_buf_ops; bp->b_ops = &xfs_attr3_rmt_buf_ops;
byte_cnt = BBTOB(bp->b_length);
byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, byte_cnt);
if (valuelen < byte_cnt)
byte_cnt = valuelen;
buf = bp->b_addr; buf = bp->b_addr;
buf += xfs_attr3_rmt_hdr_set(mp, dp->i_ino, offset,
byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, BBTOB(bp->b_length));
byte_cnt = min_t(int, valuelen, byte_cnt);
hdr_size = xfs_attr3_rmt_hdr_set(mp, dp->i_ino, offset,
byte_cnt, bp); byte_cnt, bp);
memcpy(buf, src, byte_cnt); ASSERT(hdr_size + byte_cnt <= BBTOB(bp->b_length));
if (byte_cnt < BBTOB(bp->b_length)) memcpy(buf + hdr_size, src, byte_cnt);
xfs_buf_zero(bp, byte_cnt,
BBTOB(bp->b_length) - byte_cnt); if (byte_cnt + hdr_size < BBTOB(bp->b_length))
xfs_buf_zero(bp, byte_cnt + hdr_size,
BBTOB(bp->b_length) - byte_cnt - hdr_size);
error = xfs_bwrite(bp); /* GROT: NOTE: synchronous write */ error = xfs_bwrite(bp); /* GROT: NOTE: synchronous write */
xfs_buf_relse(bp); xfs_buf_relse(bp);
...@@ -442,9 +441,9 @@ xfs_attr_rmtval_set( ...@@ -442,9 +441,9 @@ xfs_attr_rmtval_set(
src += byte_cnt; src += byte_cnt;
valuelen -= byte_cnt; valuelen -= byte_cnt;
offset += byte_cnt; offset += byte_cnt;
hdrcnt--;
lblkno += map.br_blockcount; lblkno += map.br_blockcount;
blkcnt -= map.br_blockcount;
} }
ASSERT(valuelen == 0); ASSERT(valuelen == 0);
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment