xfs: log ticket reservation underestimates the number of iclogs
When allocation a ticket for a transaction, the ticket is initialised with the worst case log space usage based on the number of bytes the transaction may consume. Part of this calculation is the number of log headers required for the iclog space used up by the transaction. This calculation makes an undocumented assumption that if the transaction uses the log header space reservation on an iclog, then it consumes either the entire iclog or it completes. That is - the transaction that is first in an iclog is the transaction that the log header reservation is accounted to. If the transaction is larger than the iclog, then it will use the entire iclog itself. Document this assumption. Further, the current calculation uses the rule that we can fit iclog_size bytes of transaction data into an iclog. This is in correct - the amount of space available in an iclog for transaction data is the size of the iclog minus the space used for log record headers. This means that the calculation is out by 512 bytes per 32k of log space the transaction can consume. This is rarely an issue because maximally sized transactions are extremely uncommon, and for 4k block size filesystems maximal transaction reservations are about 400kb. Hence the error in this case is less than the size of an iclog, so that makes it even harder to hit. However, anyone using larger directory blocks (16k directory blocks push the maximum transaction size to approx. 900k on a 4k block size filesystem) or larger block size (e.g. 64k blocks push transactions to the 3-4MB size) could see the error grow to more than an iclog and at this point the transaction is guaranteed to get a reservation underrun and shutdown the filesystem. Fix this by adjusting the calculation to calculate the correct number of iclogs required and account for them all up front. Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
This commit is contained in:
parent
b1c1b5b610
commit
9b9fc2b760
|
@ -664,7 +664,10 @@ xfs_log_item_init(
|
||||||
/*
|
/*
|
||||||
* Write region vectors to log. The write happens using the space reservation
|
* Write region vectors to log. The write happens using the space reservation
|
||||||
* of the ticket (tic). It is not a requirement that all writes for a given
|
* of the ticket (tic). It is not a requirement that all writes for a given
|
||||||
* transaction occur with one call to xfs_log_write().
|
* transaction occur with one call to xfs_log_write(). However, it is important
|
||||||
|
* to note that the transaction reservation code makes an assumption about the
|
||||||
|
* number of log headers a transaction requires that may be violated if you
|
||||||
|
* don't pass all the transaction vectors in one call....
|
||||||
*/
|
*/
|
||||||
int
|
int
|
||||||
xfs_log_write(
|
xfs_log_write(
|
||||||
|
@ -3170,14 +3173,16 @@ xfs_log_ticket_get(
|
||||||
* Allocate and initialise a new log ticket.
|
* Allocate and initialise a new log ticket.
|
||||||
*/
|
*/
|
||||||
STATIC xlog_ticket_t *
|
STATIC xlog_ticket_t *
|
||||||
xlog_ticket_alloc(xlog_t *log,
|
xlog_ticket_alloc(
|
||||||
|
struct log *log,
|
||||||
int unit_bytes,
|
int unit_bytes,
|
||||||
int cnt,
|
int cnt,
|
||||||
char client,
|
char client,
|
||||||
uint xflags)
|
uint xflags)
|
||||||
{
|
{
|
||||||
xlog_ticket_t *tic;
|
struct xlog_ticket *tic;
|
||||||
uint num_headers;
|
uint num_headers;
|
||||||
|
int iclog_space;
|
||||||
|
|
||||||
tic = kmem_zone_zalloc(xfs_log_ticket_zone, KM_SLEEP|KM_MAYFAIL);
|
tic = kmem_zone_zalloc(xfs_log_ticket_zone, KM_SLEEP|KM_MAYFAIL);
|
||||||
if (!tic)
|
if (!tic)
|
||||||
|
@ -3221,16 +3226,40 @@ xlog_ticket_alloc(xlog_t *log,
|
||||||
/* for start-rec */
|
/* for start-rec */
|
||||||
unit_bytes += sizeof(xlog_op_header_t);
|
unit_bytes += sizeof(xlog_op_header_t);
|
||||||
|
|
||||||
/* for LR headers */
|
/*
|
||||||
num_headers = ((unit_bytes + log->l_iclog_size-1) >> log->l_iclog_size_log);
|
* for LR headers - the space for data in an iclog is the size minus
|
||||||
|
* the space used for the headers. If we use the iclog size, then we
|
||||||
|
* undercalculate the number of headers required.
|
||||||
|
*
|
||||||
|
* Furthermore - the addition of op headers for split-recs might
|
||||||
|
* increase the space required enough to require more log and op
|
||||||
|
* headers, so take that into account too.
|
||||||
|
*
|
||||||
|
* IMPORTANT: This reservation makes the assumption that if this
|
||||||
|
* transaction is the first in an iclog and hence has the LR headers
|
||||||
|
* accounted to it, then the remaining space in the iclog is
|
||||||
|
* exclusively for this transaction. i.e. if the transaction is larger
|
||||||
|
* than the iclog, it will be the only thing in that iclog.
|
||||||
|
* Fundamentally, this means we must pass the entire log vector to
|
||||||
|
* xlog_write to guarantee this.
|
||||||
|
*/
|
||||||
|
iclog_space = log->l_iclog_size - log->l_iclog_hsize;
|
||||||
|
num_headers = howmany(unit_bytes, iclog_space);
|
||||||
|
|
||||||
|
/* for split-recs - ophdrs added when data split over LRs */
|
||||||
|
unit_bytes += sizeof(xlog_op_header_t) * num_headers;
|
||||||
|
|
||||||
|
/* add extra header reservations if we overrun */
|
||||||
|
while (!num_headers ||
|
||||||
|
howmany(unit_bytes, iclog_space) > num_headers) {
|
||||||
|
unit_bytes += sizeof(xlog_op_header_t);
|
||||||
|
num_headers++;
|
||||||
|
}
|
||||||
unit_bytes += log->l_iclog_hsize * num_headers;
|
unit_bytes += log->l_iclog_hsize * num_headers;
|
||||||
|
|
||||||
/* for commit-rec LR header - note: padding will subsume the ophdr */
|
/* for commit-rec LR header - note: padding will subsume the ophdr */
|
||||||
unit_bytes += log->l_iclog_hsize;
|
unit_bytes += log->l_iclog_hsize;
|
||||||
|
|
||||||
/* for split-recs - ophdrs added when data split over LRs */
|
|
||||||
unit_bytes += sizeof(xlog_op_header_t) * num_headers;
|
|
||||||
|
|
||||||
/* for roundoff padding for transaction data and one for commit record */
|
/* for roundoff padding for transaction data and one for commit record */
|
||||||
if (xfs_sb_version_haslogv2(&log->l_mp->m_sb) &&
|
if (xfs_sb_version_haslogv2(&log->l_mp->m_sb) &&
|
||||||
log->l_mp->m_sb.sb_logsunit > 1) {
|
log->l_mp->m_sb.sb_logsunit > 1) {
|
||||||
|
@ -3252,7 +3281,7 @@ xlog_ticket_alloc(xlog_t *log,
|
||||||
tic->t_trans_type = 0;
|
tic->t_trans_type = 0;
|
||||||
if (xflags & XFS_LOG_PERM_RESERV)
|
if (xflags & XFS_LOG_PERM_RESERV)
|
||||||
tic->t_flags |= XLOG_TIC_PERM_RESERV;
|
tic->t_flags |= XLOG_TIC_PERM_RESERV;
|
||||||
sv_init(&(tic->t_wait), SV_DEFAULT, "logtick");
|
sv_init(&tic->t_wait, SV_DEFAULT, "logtick");
|
||||||
|
|
||||||
xlog_tic_reset_res(tic);
|
xlog_tic_reset_res(tic);
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue