| Message ID | 158258957631.451378.6297312804413916157.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfsprogs: refactor buffer function names | expand |
On Mon, Feb 24, 2020 at 04:12:56PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Convert the log clearing function to use uncached buffers like > everything else, instead of using the raw buffer get/put functions. > This will eventually enable us to hide them more effectively. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > libxfs/rdwr.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > > diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c > index d607a565..739f4aed 100644 > --- a/libxfs/rdwr.c > +++ b/libxfs/rdwr.c > @@ -1413,15 +1413,13 @@ libxfs_log_clear( > /* write out the first log record */ > ptr = dptr; > if (btp) { > - bp = libxfs_getbufr(btp, start, len); > + bp = libxfs_getbufr_uncached(btp, start, len); Any reason this isn't using the public libxfs_buf_get_uncached function? Yes, that requires setting up the address, but it avoids a dependency on internal helpers. And I think we should add initializing the block, zeroing the buffer and setting up ops into (lib)xfs_buf_get_uncached, basically moving most of xfs_get_aghdr_buf into and improve the API eventually.
On Tue, Feb 25, 2020 at 09:49:41AM -0800, Christoph Hellwig wrote: > On Mon, Feb 24, 2020 at 04:12:56PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Convert the log clearing function to use uncached buffers like > > everything else, instead of using the raw buffer get/put functions. > > This will eventually enable us to hide them more effectively. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > libxfs/rdwr.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > > > diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c > > index d607a565..739f4aed 100644 > > --- a/libxfs/rdwr.c > > +++ b/libxfs/rdwr.c > > @@ -1413,15 +1413,13 @@ libxfs_log_clear( > > /* write out the first log record */ > > ptr = dptr; > > if (btp) { > > - bp = libxfs_getbufr(btp, start, len); > > + bp = libxfs_getbufr_uncached(btp, start, len); > > Any reason this isn't using the public libxfs_buf_get_uncached > function? Yes, that requires setting up the address, but it avoids > a dependency on internal helpers. > > And I think we should add initializing the block, zeroing the buffer > and setting up ops into (lib)xfs_buf_get_uncached, basically moving > most of xfs_get_aghdr_buf into and improve the API eventually. We can certainly do that for kernel 5.7, but please keep in mind that we're currently reviewing changes for xfsprogs 5.6, and it's too late in the kernel 5.6 cycle to be messing around with core xfs apis. --D
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c index d607a565..739f4aed 100644 --- a/libxfs/rdwr.c +++ b/libxfs/rdwr.c @@ -1413,15 +1413,13 @@ libxfs_log_clear( /* write out the first log record */ ptr = dptr; if (btp) { - bp = libxfs_getbufr(btp, start, len); + bp = libxfs_getbufr_uncached(btp, start, len); ptr = bp->b_addr; } libxfs_log_header(ptr, fs_uuid, version, sunit, fmt, lsn, tail_lsn, next, bp); - if (bp) { - bp->b_flags |= LIBXFS_B_DIRTY; - libxfs_putbufr(bp); - } + if (bp) + libxfs_writebuf(bp, 0); /* * There's nothing else to do if this is a log reset. The kernel detects @@ -1461,7 +1459,7 @@ libxfs_log_clear( ptr = dptr; if (btp) { - bp = libxfs_getbufr(btp, blk, len); + bp = libxfs_getbufr_uncached(btp, blk, len); ptr = bp->b_addr; } /* @@ -1470,10 +1468,8 @@ libxfs_log_clear( */ libxfs_log_header(ptr, fs_uuid, version, BBTOB(len), fmt, lsn, tail_lsn, next, bp); - if (bp) { - bp->b_flags |= LIBXFS_B_DIRTY; - libxfs_putbufr(bp); - } + if (bp) + libxfs_writebuf(bp, 0); blk += len; if (dptr)