Skip to content

Commit 91781ff

Browse files
author
Darrick J. Wong
committed
xfs: split freemap from xchk_xattr_buf.buf
Move the free space bitmap from somewhere in xchk_xattr_buf.buf[] to an explicit pointer. This is the start of removing the complex overloaded memory buffer that is the source of weird memory misuse bugs. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com>
1 parent 4cb7602 commit 91781ff

4 files changed

Lines changed: 49 additions & 19 deletions

File tree

fs/xfs/scrub/attr.c

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@
2020
#include "scrub/dabtree.h"
2121
#include "scrub/attr.h"
2222

23+
/* Free the buffers linked from the xattr buffer. */
24+
static void
25+
xchk_xattr_buf_cleanup(
26+
void *priv)
27+
{
28+
struct xchk_xattr_buf *ab = priv;
29+
30+
kvfree(ab->freemap);
31+
ab->freemap = NULL;
32+
}
33+
2334
/*
2435
* Allocate enough memory to hold an attr value and attr block bitmaps,
2536
* reallocating the buffer if necessary. Buffer contents are not preserved
@@ -32,15 +43,18 @@ xchk_setup_xattr_buf(
3243
gfp_t flags)
3344
{
3445
size_t sz;
46+
size_t bmp_sz;
3547
struct xchk_xattr_buf *ab = sc->buf;
48+
unsigned long *old_freemap = NULL;
49+
50+
bmp_sz = sizeof(long) * BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
3651

3752
/*
3853
* We need enough space to read an xattr value from the file or enough
39-
* space to hold two copies of the xattr free space bitmap. We don't
54+
* space to hold one copy of the xattr free space bitmap. We don't
4055
* need the buffer space for both purposes at the same time.
4156
*/
42-
sz = 2 * sizeof(long) * BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
43-
sz = max_t(size_t, sz, value_size);
57+
sz = max_t(size_t, bmp_sz, value_size);
4458

4559
/*
4660
* If there's already a buffer, figure out if we need to reallocate it
@@ -49,6 +63,7 @@ xchk_setup_xattr_buf(
4963
if (ab) {
5064
if (sz <= ab->sz)
5165
return 0;
66+
old_freemap = ab->freemap;
5267
kvfree(ab);
5368
sc->buf = NULL;
5469
}
@@ -60,9 +75,18 @@ xchk_setup_xattr_buf(
6075
ab = kvmalloc(sizeof(*ab) + sz, flags);
6176
if (!ab)
6277
return -ENOMEM;
63-
6478
ab->sz = sz;
6579
sc->buf = ab;
80+
sc->buf_cleanup = xchk_xattr_buf_cleanup;
81+
82+
if (old_freemap) {
83+
ab->freemap = old_freemap;
84+
} else {
85+
ab->freemap = kvmalloc(bmp_sz, flags);
86+
if (!ab->freemap)
87+
return -ENOMEM;
88+
}
89+
6690
return 0;
6791
}
6892

@@ -222,21 +246,21 @@ xchk_xattr_check_freemap(
222246
unsigned long *map,
223247
struct xfs_attr3_icleaf_hdr *leafhdr)
224248
{
225-
unsigned long *freemap = xchk_xattr_freemap(sc);
249+
struct xchk_xattr_buf *ab = sc->buf;
226250
unsigned int mapsize = sc->mp->m_attr_geo->blksize;
227251
int i;
228252

229253
/* Construct bitmap of freemap contents. */
230-
bitmap_zero(freemap, mapsize);
254+
bitmap_zero(ab->freemap, mapsize);
231255
for (i = 0; i < XFS_ATTR_LEAF_MAPSIZE; i++) {
232-
if (!xchk_xattr_set_map(sc, freemap,
256+
if (!xchk_xattr_set_map(sc, ab->freemap,
233257
leafhdr->freemap[i].base,
234258
leafhdr->freemap[i].size))
235259
return false;
236260
}
237261

238262
/* Look for bits that are set in freemap and are marked in use. */
239-
return !bitmap_intersects(freemap, map, mapsize);
263+
return !bitmap_intersects(ab->freemap, map, mapsize);
240264
}
241265

242266
/*

fs/xfs/scrub/attr.h

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
* Temporary storage for online scrub and repair of extended attributes.
1111
*/
1212
struct xchk_xattr_buf {
13+
/* Bitmap of free space in xattr leaf blocks. */
14+
unsigned long *freemap;
15+
1316
/* Size of @buf, in bytes. */
1417
size_t sz;
1518

@@ -20,8 +23,7 @@ struct xchk_xattr_buf {
2023
*
2124
* Each bitmap contains enough bits to track every byte in an attr
2225
* block (rounded up to the size of an unsigned long). The attr block
23-
* used space bitmap starts at the beginning of the buffer; the free
24-
* space bitmap follows immediately after.
26+
* used space bitmap starts at the beginning of the buffer.
2527
*/
2628
uint8_t buf[];
2729
};
@@ -46,13 +48,4 @@ xchk_xattr_usedmap(
4648
return (unsigned long *)ab->buf;
4749
}
4850

49-
/* A bitmap of free space computed by walking attr leaf block free info. */
50-
static inline unsigned long *
51-
xchk_xattr_freemap(
52-
struct xfs_scrub *sc)
53-
{
54-
return xchk_xattr_usedmap(sc) +
55-
BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
56-
}
57-
5851
#endif /* __XFS_SCRUB_ATTR_H__ */

fs/xfs/scrub/scrub.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,10 @@ xchk_teardown(
189189
if (sc->flags & XCHK_REAPING_DISABLED)
190190
xchk_start_reaping(sc);
191191
if (sc->buf) {
192+
if (sc->buf_cleanup)
193+
sc->buf_cleanup(sc->buf);
192194
kvfree(sc->buf);
195+
sc->buf_cleanup = NULL;
193196
sc->buf = NULL;
194197
}
195198

fs/xfs/scrub/scrub.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,17 @@ struct xfs_scrub {
7777
*/
7878
struct xfs_inode *ip;
7979

80+
/* Kernel memory buffer used by scrubbers; freed at teardown. */
8081
void *buf;
82+
83+
/*
84+
* Clean up resources owned by whatever is in the buffer. Cleanup can
85+
* be deferred with this hook as a means for scrub functions to pass
86+
* data to repair functions. This function must not free the buffer
87+
* itself.
88+
*/
89+
void (*buf_cleanup)(void *buf);
90+
8191
uint ilock_flags;
8292

8393
/* See the XCHK/XREP state flags below. */

0 commit comments

Comments
 (0)