xfs: fix min bufsize bugs in two places
PATCH 2/10 xfs: fix min bufsize bugs in two places
This fixes a bug in two places that I found by inspection. In xlog_find_verify_cycle() and xlog_write_log_records(), the code attempts to allocate a buffer to hold as many blocks as possible. It gives up if the number of blocks to be allocated gets too small. Right now it uses log->l_sectbb_log as that lower bound, but I'm sure it's supposed to be the actual log sector size instead. That is, the lower bound should be (1 << log->l_sectbb_log).
Also define a simple macro xlog_sectbb(log) to represent the number of basic blocks in a sector for the given log.
(No change from original submission; I have implemented Christoph's suggestion about storing l_sectsize rather than l_sectbb_log in a new, separate patch in this series.)
---fs/xfs/xfs_log_recover.c | 21 +++++++++++++++++----
fs/xfs/xfs_log_recover.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
fs/xfs/xfs_log_recover.c +17 -4
@@ -66,6 +66,9 @@ STATIC void xlog_recover_check_summary(x ((bbs + (log)->l_sectbb_mask + 1) & ~(log)->l_sectbb_mask) : (bbs) )#define XLOG_SECTOR_ROUNDDOWN_BLKNO (log, bno) ((bno) & ~(log)->l_sectbb_mask) +/* Number of basic blocks in a log sector */ +#define xlog_sectbb (log) (1 << (log)->l_sectbb_log) + STATIC xfs_buf_t * xlog_get_bp( xlog_t *log,
@@ -376,12 +379,16 @@ xlog_find_verify_cycle(xfs_caddr_t buf =NULL ;int error =0 ; +/* +* Greedily allocate a buffer big enough to handle the full +* range of basic blocks we'll be examining. If that fails, +* try a smaller size. We need to be able to read at least +* a log sector, or we're out of luck. +*/ bufblks =1 <<ffs (nbblks); -while (!(bp =xlog_get_bp (log, bufblks))) { -/* can't get enough memory to do everything in one big buffer */ bufblks >>=1 ; -if (bufblks <= log->l_sectbb_log) +if (bufblks <xlog_sectbb (log))return ENOMEM ; }
@@ -1158,10 +1165,16 @@ xlog_write_log_records(int error =0 ;int i, j =0 ; +/* +* Greedily allocate a buffer big enough to handle the full +* range of basic blocks to be written. If that fails, try +* a smaller size. We need to be able to write at least a +* log sector, or we're out of luck. +*/ bufblks =1 <<ffs (blocks);while (!(bp =xlog_get_bp (log, bufblks))) { bufblks >>=1 ; -if (bufblks <= log->l_sectbb_log) +if (bufblks <xlog_sectbb (log))return ENOMEM ; }
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 27 lines · Alex Elder
This fixes a bug in two places that I found by inspection. In
xlog_find_verify_cycle()andxlog_write_log_records(), the code attempts to allocate a buffer to hold as many blocks as possible. It gives up if the number of blocks to be allocated gets too small. Right now it useslog->l_sectbb_logas that lower bound, but I'm sure it's supposed to be the actual log sector size instead. That is, the lower bound should be (1 <<log->l_sectbb_log).Also define a simple macro
xlog_sectbb(log)to represent the number of basic blocks in a sector for the given log.(No change from original submission; I have implemented Christoph's suggestion about storing
l_sectsizerather thanl_sectbb_login a new, separate patch in this series.)Signed-off-by Alex Elder <[email protected]>---
fs/xfs/xfs_log_recover.c| 21 +++++++++++++++++----fs/xfs/xfs_log_recover.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)Index:
b/fs/xfs/xfs_log_recover.c
===================================================================
fs/xfs/xfs_log_recover.c STATIC void xlog_recover_check_summary(x · line 71
Quoted 6 lines
fs/xfs/xfs_log_recover.c +3
-0@@ -66,6 +66,9 @@ STATIC void xlog_recover_check_summary(x ((bbs + (log)->l_sectbb_mask + 1) & ~(log)->l_sectbb_mask) : (bbs) )#define XLOG_SECTOR_ROUNDDOWN_BLKNO (log, bno) ((bno) & ~(log)->l_sectbb_mask) +/* Number of basic blocks in a log sector */ +#define xlog_sectbb (log) (1 << (log)->l_sectbb_log) +
Can you do this without the #define? There's a spot in xlog_alloc_log() where the l_sectbb_mask is set up that open code this shift, so it seems to me that this woul dbe the spot to set up a l_sect_size variable and reference that instead...
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Dave Chinner
Reviewed-byQuoted 31 lines · Dave Chinner
Quoted 30 lines · Alex Elder
This fixes a bug in two places that I found by inspection. In
xlog_find_verify_cycle()andxlog_write_log_records(), the code attempts to allocate a buffer to hold as many blocks as possible. It gives up if the number of blocks to be allocated gets too small. Right now it useslog->l_sectbb_logas that lower bound, but I'm sure it's supposed to be the actual log sector size instead. That is, the lower bound should be (1 <<log->l_sectbb_log).Also define a simple macro
xlog_sectbb(log)to represent the number of basic blocks in a sector for the given log.(No change from original submission; I have implemented Christoph's suggestion about storing
l_sectsizerather thanl_sectbb_login a new, separate patch in this series.)Signed-off-by Alex Elder <[email protected]>---
fs/xfs/xfs_log_recover.c| 21 +++++++++++++++++----fs/xfs/xfs_log_recover.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)Index:
b/fs/xfs/xfs_log_recover.c
===================================================================fs/xfs/xfs_log_recover.c +0
-0@@ -66,6 +66,9 @@ STATIC void xlog_recover_check_summary(x ((bbs + (log)->l_sectbb_mask + 1) & ~(log)->l_sectbb_mask) : (bbs) )#define XLOG_SECTOR_ROUNDDOWN_BLKNO (log, bno) ((bno) & ~(log)->l_sectbb_mask)
Quoted 8 lines
fs/xfs/xfs_log_recover.c STATIC void xlog_recover_check_summary(x · line 71
Quoted 3 lines
fs/xfs/xfs_log_recover.c +3
-0+/* Number of basic blocks in a log sector */ +#define xlog_sectbb (log) (1 << (log)->l_sectbb_log) +Can you do this without the #define? There's a spot in
xlog_alloc_log()where thel_sectbb_maskis set up that open code this shift, so it seems to me that this woul dbe the spot to set up al_sect_sizevariable and reference that instead...
I note you did this with a later patch, so this one is good to go.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs