On Thu, Oct 15, 2020 at 06:21:43PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Move the uncached buffer IO API into the xfs_buftarg.h and the local
> buftarg implementation. The uncached buffer IO implementation is
> different between kernel and userspace, but the API is the same.
> Hence implement it via the buftarg abstraction.
>
> Pull the "alloc_write_buf()" function from mkfs up into the API as
> xfs_buf_get_uncached_daddr() so that it can be used in other places
> that need the same functionality.
But this doesn't actually remove alloc_write_buf from xfs_mkfs.c...
> The API movement still uses the existing raw buffer allocation
> and read IO implementation. This requires us to temporarily export
> the the prototypes for these functions in xfs_buftarg.h. They will
> go away once the buftarg has it's own buffer allocation and IO
> engine implementations.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> libxfs/buftarg.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
> libxfs/libxfs_io.h | 22 +----------
> libxfs/rdwr.c | 88 ++++++-------------------------------------
> libxfs/xfs_buftarg.h | 39 +++++++++++++++++++
> mkfs/xfs_mkfs.c | 29 ++++++++++----
> 5 files changed, 164 insertions(+), 104 deletions(-)
>
> diff --git a/libxfs/buftarg.c b/libxfs/buftarg.c
> index d4bcb2936f01..2a0aad2e0f8c 100644
> --- a/libxfs/buftarg.c
> +++ b/libxfs/buftarg.c
> @@ -97,3 +97,93 @@ xfs_buftarg_free(
> platform_flush_device(btp->bt_fd, btp->bt_bdev);
> free(btp);
> }
> +
> +/*
> + * Allocate an uncached buffer that points at daddr. The refcount will be 1,
> + * and the cache node hash list will be empty to indicate that it's uncached.
> + */
> +int
> +xfs_buf_get_uncached_daddr(
> + struct xfs_buftarg *target,
> + xfs_daddr_t daddr,
> + size_t bblen,
> + struct xfs_buf **bpp)
> +{
> + struct xfs_buf *bp;
> +
> + bp = libxfs_getbufr(target, daddr, bblen);
> + if (!bp)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&bp->b_node.cn_hash);
> + bp->b_node.cn_count = 1;
> + bp->b_bn = daddr;
> + bp->b_maps[0].bm_bn = daddr;
This function has all kinds of indent weirdness.
The general approach seems ok so far, but WIP indeed. :)
--D
> + *bpp = bp;
> + return 0;
> +}
> +
> +int
> +xfs_buf_read_uncached(
> + struct xfs_buftarg *target,
> + xfs_daddr_t daddr,
> + size_t bblen,
> + int flags,
> + struct xfs_buf **bpp,
> + const struct xfs_buf_ops *ops)
> +{
> + struct xfs_buf *bp;
> + int error;
> +
> +
> + error = xfs_buf_get_uncached(target, bblen, flags, &bp);
> + if (error)
> + return error;
> +
> + error = libxfs_readbufr(target, daddr, bp, bblen, flags);
> + if (error)
> + goto release_buf;
> +
> + error = libxfs_readbuf_verify(bp, ops);
> + if (error)
> + goto release_buf;
> +
> + *bpp = bp;
> + return 0;
> +
> +release_buf:
> + libxfs_buf_relse(bp);
> + return error;
> +}
> +
> +/*
> + * Return a buffer associated to external memory via xfs_buf_associate_memory()
> + * back to it's empty state.
> + */
> +void
> +xfs_buf_set_empty(
> + struct xfs_buf *bp,
> + size_t numblks)
> +{
> + bp->b_addr = NULL;
> + bp->b_length = numblks;
> +
> + ASSERT(bp->b_map_count == 1);
> + bp->b_bn = XFS_BUF_DADDR_NULL;
> + bp->b_maps[0].bm_bn = XFS_BUF_DADDR_NULL;
> + bp->b_maps[0].bm_len = bp->b_length;
> +}
> +
> +/*
> + * Associate external memory with an empty uncached buffer.
> + */
> +int
> +xfs_buf_associate_memory(
> + struct xfs_buf *bp,
> + void *mem,
> + size_t len)
> +{
> + bp->b_addr = mem;
> + bp->b_length = BTOBB(len);
> + return 0;
> +}
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 0f9630e8e17a..7f8fd88f7de8 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -61,7 +61,7 @@ struct xfs_buf {
> struct xfs_mount *b_mount;
> struct xfs_buf_map *b_maps;
> struct xfs_buf_map __b_map;
> - int b_nmaps;
> + int b_map_count;
> struct list_head b_list;
> };
>
> @@ -77,8 +77,6 @@ bool xfs_verify_magic16(struct xfs_buf *bp, __be16 dmagic);
>
> typedef unsigned int xfs_buf_flags_t;
>
> -#define XFS_BUF_DADDR_NULL ((xfs_daddr_t) (-1LL))
> -
> #define xfs_buf_offset(bp, offset) ((bp)->b_addr + (offset))
> #define XFS_BUF_ADDR(bp) ((bp)->b_bn)
>
> @@ -148,10 +146,6 @@ extern int libxfs_bcache_overflowed(void);
>
> /* Buffer (Raw) Interfaces */
> int libxfs_bwrite(struct xfs_buf *bp);
> -extern int libxfs_readbufr(struct xfs_buftarg *, xfs_daddr_t,
> - struct xfs_buf *, int, int);
> -extern int libxfs_readbufr_map(struct xfs_buftarg *, struct xfs_buf *, int);
> -
> extern int libxfs_device_zero(struct xfs_buftarg *, xfs_daddr_t, uint);
>
> extern int libxfs_bhash_size;
> @@ -170,26 +164,12 @@ xfs_buf_update_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
> cksum_offset);
> }
>
> -static inline int
> -xfs_buf_associate_memory(struct xfs_buf *bp, void *mem, size_t len)
> -{
> - bp->b_addr = mem;
> - bp->b_length = BTOBB(len);
> - return 0;
> -}
> -
> static inline void
> xfs_buf_hold(struct xfs_buf *bp)
> {
> bp->b_node.cn_count++;
> }
>
> -int libxfs_buf_get_uncached(struct xfs_buftarg *targ, size_t bblen, int flags,
> - struct xfs_buf **bpp);
> -int libxfs_buf_read_uncached(struct xfs_buftarg *targ, xfs_daddr_t daddr,
> - size_t bblen, int flags, struct xfs_buf **bpp,
> - const struct xfs_buf_ops *ops);
> -
> /* Push a single buffer on a delwri queue. */
> static inline bool
> xfs_buf_delwri_queue(struct xfs_buf *bp, struct list_head *buffer_list)
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 5ab1987eb0fe..3e755402b024 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -249,7 +249,7 @@ __initbuf(struct xfs_buf *bp, struct xfs_buftarg *btp, xfs_daddr_t bno,
> INIT_LIST_HEAD(&bp->b_li_list);
>
> if (!bp->b_maps) {
> - bp->b_nmaps = 1;
> + bp->b_map_count = 1;
> bp->b_maps = &bp->__b_map;
> bp->b_maps[0].bm_bn = bp->b_bn;
> bp->b_maps[0].bm_len = bp->b_length;
> @@ -279,7 +279,7 @@ libxfs_initbuf_map(struct xfs_buf *bp, struct xfs_buftarg *btp,
> strerror(errno));
> exit(1);
> }
> - bp->b_nmaps = nmaps;
> + bp->b_map_count = nmaps;
>
> bytes = 0;
> for ( i = 0; i < nmaps; i++) {
> @@ -331,7 +331,7 @@ __libxfs_getbufr(int blen)
> return bp;
> }
>
> -static struct xfs_buf *
> +struct xfs_buf *
> libxfs_getbufr(struct xfs_buftarg *btp, xfs_daddr_t blkno, int bblen)
> {
> struct xfs_buf *bp;
> @@ -617,7 +617,7 @@ libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
>
> fd = libxfs_device_to_fd(btp->bt_bdev);
> buf = bp->b_addr;
> - for (i = 0; i < bp->b_nmaps; i++) {
> + for (i = 0; i < bp->b_map_count; i++) {
> off64_t offset = LIBXFS_BBTOOFF64(bp->b_maps[i].bm_bn);
> int len = BBTOB(bp->b_maps[i].bm_len);
>
> @@ -707,75 +707,6 @@ err:
> return error;
> }
>
> -/* Allocate a raw uncached buffer. */
> -static inline struct xfs_buf *
> -libxfs_getbufr_uncached(
> - struct xfs_buftarg *targ,
> - xfs_daddr_t daddr,
> - size_t bblen)
> -{
> - struct xfs_buf *bp;
> -
> - bp = libxfs_getbufr(targ, daddr, bblen);
> - if (!bp)
> - return NULL;
> -
> - INIT_LIST_HEAD(&bp->b_node.cn_hash);
> - bp->b_node.cn_count = 1;
> - return bp;
> -}
> -
> -/*
> - * Allocate an uncached buffer that points nowhere. The refcount will be 1,
> - * and the cache node hash list will be empty to indicate that it's uncached.
> - */
> -int
> -libxfs_buf_get_uncached(
> - struct xfs_buftarg *targ,
> - size_t bblen,
> - int flags,
> - struct xfs_buf **bpp)
> -{
> - *bpp = libxfs_getbufr_uncached(targ, XFS_BUF_DADDR_NULL, bblen);
> - return *bpp != NULL ? 0 : -ENOMEM;
> -}
> -
> -/*
> - * Allocate and read an uncached buffer. The refcount will be 1, and the cache
> - * node hash list will be empty to indicate that it's uncached.
> - */
> -int
> -libxfs_buf_read_uncached(
> - struct xfs_buftarg *targ,
> - xfs_daddr_t daddr,
> - size_t bblen,
> - int flags,
> - struct xfs_buf **bpp,
> - const struct xfs_buf_ops *ops)
> -{
> - struct xfs_buf *bp;
> - int error;
> -
> - *bpp = NULL;
> - bp = libxfs_getbufr_uncached(targ, daddr, bblen);
> - if (!bp)
> - return -ENOMEM;
> -
> - error = libxfs_readbufr(targ, daddr, bp, bblen, flags);
> - if (error)
> - goto err;
> -
> - error = libxfs_readbuf_verify(bp, ops);
> - if (error)
> - goto err;
> -
> - *bpp = bp;
> - return 0;
> -err:
> - libxfs_buf_relse(bp);
> - return error;
> -}
> -
> static int
> __write_buf(int fd, void *buf, int len, off64_t offset, int flags)
> {
> @@ -836,7 +767,7 @@ libxfs_bwrite(
> int i;
> void *buf = bp->b_addr;
>
> - for (i = 0; i < bp->b_nmaps; i++) {
> + for (i = 0; i < bp->b_map_count; i++) {
> off64_t offset = LIBXFS_BBTOOFF64(bp->b_maps[i].bm_bn);
> int len = BBTOB(bp->b_maps[i].bm_len);
>
> @@ -1207,6 +1138,7 @@ libxfs_log_clear(
> xfs_daddr_t blk;
> xfs_daddr_t end_blk;
> char *ptr;
> + int error;
>
> if (((btp && dptr) || (!btp && !dptr)) ||
> (btp && !btp->bt_bdev) || !fs_uuid)
> @@ -1236,7 +1168,9 @@ libxfs_log_clear(
> /* write out the first log record */
> ptr = dptr;
> if (btp) {
> - bp = libxfs_getbufr_uncached(btp, start, len);
> + error = xfs_buf_get_uncached_daddr(btp, start, len, &bp);
> + if (error)
> + return error;
> ptr = bp->b_addr;
> }
> libxfs_log_header(ptr, fs_uuid, version, sunit, fmt, lsn, tail_lsn,
> @@ -1284,7 +1218,9 @@ libxfs_log_clear(
>
> ptr = dptr;
> if (btp) {
> - bp = libxfs_getbufr_uncached(btp, blk, len);
> + error = xfs_buf_get_uncached_daddr(btp, blk, len, &bp);
> + if (error)
> + return error;
> ptr = bp->b_addr;
> }
> /*
> diff --git a/libxfs/xfs_buftarg.h b/libxfs/xfs_buftarg.h
> index 1bc3a4d0bc9c..5429c96c0547 100644
> --- a/libxfs/xfs_buftarg.h
> +++ b/libxfs/xfs_buftarg.h
> @@ -11,6 +11,8 @@ struct xfs_mount;
> struct xfs_buf;
> struct xfs_buf_ops;
>
> +#define XFS_BUF_DADDR_NULL ((xfs_daddr_t) (-1LL))
> +
> /*
> * The xfs_buftarg contains 2 notions of "sector size" -
> *
> @@ -52,4 +54,41 @@ int xfs_buftarg_setsize(struct xfs_buftarg *target, unsigned int size);
>
> #define xfs_getsize_buftarg(buftarg) block_size((buftarg)->bt_bdev)
>
> +/*
> + * Low level buftarg IO routines.
> + *
> + * This includes the uncached buffer IO API, as the memory management associated
> + * with uncached buffers is tightly tied to the kernel buffer implementation.
> + */
> +
> +void xfs_buf_set_empty(struct xfs_buf *bp, size_t numblks);
> +int xfs_buf_associate_memory(struct xfs_buf *bp, void *mem, size_t length);
> +
> +int xfs_buf_get_uncached_daddr(struct xfs_buftarg *target, xfs_daddr_t daddr,
> + size_t bblen, struct xfs_buf **bpp);
> +static inline int
> +xfs_buf_get_uncached(
> + struct xfs_buftarg *target,
> + size_t bblen,
> + int flags,
> + struct xfs_buf **bpp)
> +{
> + return xfs_buf_get_uncached_daddr(target, XFS_BUF_DADDR_NULL, bblen, bpp);
> +}
> +
> +int xfs_buf_read_uncached(struct xfs_buftarg *target, xfs_daddr_t daddr,
> + size_t bblen, int flags, struct xfs_buf **bpp,
> + const struct xfs_buf_ops *ops);
> +
> +/*
> + * Raw buffer access functions. These exist as temporary bridges for uncached IO
> + * that uses direct access to the buffers to submit IO. These will go away with
> + * the new buffer cache IO engine.
> + */
> +struct xfs_buf *libxfs_getbufr(struct xfs_buftarg *btp, xfs_daddr_t blkno,
> + int bblen);
> +int libxfs_readbufr(struct xfs_buftarg *, xfs_daddr_t, struct xfs_buf *, int,
> + int);
> +int libxfs_readbufr_map(struct xfs_buftarg *, struct xfs_buf *, int);
> +
> #endif /* __XFS_BUFTARG_H */
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 794955a9624c..87e1881e3152 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3463,6 +3463,7 @@ prepare_devices(
> struct xfs_buf *buf;
> int whack_blks = BTOBB(WHACK_SIZE);
> int lsunit;
> + int error;
>
> /*
> * If there's an old XFS filesystem on the device with enough intact
> @@ -3496,8 +3497,10 @@ prepare_devices(
> * the end of the device. (MD sb is ~64k from the end, take out a wider
> * swath to be sure)
> */
> - buf = alloc_write_buf(mp->m_ddev_targp, (xi->dsize - whack_blks),
> - whack_blks);
> + error = xfs_buf_get_uncached_daddr(mp->m_ddev_targp,
> + (xi->dsize - whack_blks), whack_blks, &buf);
> + if (error)
> + goto out_error;
> memset(buf->b_addr, 0, WHACK_SIZE);
> libxfs_buf_mark_dirty(buf);
> libxfs_buf_relse(buf);
> @@ -3508,14 +3511,18 @@ prepare_devices(
> * swap (somewhere around the page size), jfs (32k),
> * ext[2,3] and reiserfs (64k) - and hopefully all else.
> */
> - buf = alloc_write_buf(mp->m_ddev_targp, 0, whack_blks);
> + error = xfs_buf_get_uncached_daddr(mp->m_ddev_targp, 0, whack_blks, &buf);
> + if (error)
> + goto out_error;
> memset(buf->b_addr, 0, WHACK_SIZE);
> libxfs_buf_mark_dirty(buf);
> libxfs_buf_relse(buf);
>
> /* OK, now write the superblock... */
> - buf = alloc_write_buf(mp->m_ddev_targp, XFS_SB_DADDR,
> - XFS_FSS_TO_BB(mp, 1));
> + error = xfs_buf_get_uncached_daddr(mp->m_ddev_targp, XFS_SB_DADDR,
> + XFS_FSS_TO_BB(mp, 1), &buf);
> + if (error)
> + goto out_error;
> buf->b_ops = &xfs_sb_buf_ops;
> memset(buf->b_addr, 0, cfg->sectorsize);
> libxfs_sb_to_disk(buf->b_addr, sbp);
> @@ -3536,14 +3543,22 @@ prepare_devices(
> /* finally, check we can write the last block in the realtime area */
> if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_bdev &&
> cfg->rtblocks > 0) {
> - buf = alloc_write_buf(mp->m_rtdev_targp,
> + error = xfs_buf_get_uncached_daddr(mp->m_rtdev_targp,
> XFS_FSB_TO_BB(mp, cfg->rtblocks - 1LL),
> - BTOBB(cfg->blocksize));
> + BTOBB(cfg->blocksize), &buf);
> + if (error)
> + goto out_error;
> memset(buf->b_addr, 0, cfg->blocksize);
> libxfs_buf_mark_dirty(buf);
> libxfs_buf_relse(buf);
> }
>
> + return;
> +
> +out_error:
> + fprintf(stderr, _("Could not get memory for buffer, err=%d\n"),
> + error);
> + exit(1);
> }
>
> static void
> --
> 2.28.0
>
