On Mon, 5 May 2014, Darrick J. Wong wrote:
> Date: Mon, 5 May 2014 15:23:33 -0700
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> To: Lukáš Czerner <lczerner@xxxxxxxxxx>
> Cc: tytso@xxxxxxx, linux-ext4@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 05/37] debugfs: teach logdump to deal with 64bit revoke
> tables
>
> On Fri, May 02, 2014 at 01:38:04PM +0200, Lukáš Czerner wrote:
> > On Thu, 1 May 2014, Darrick J. Wong wrote:
> >
> > > Date: Thu, 01 May 2014 16:12:55 -0700
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > To: tytso@xxxxxxx, darrick.wong@xxxxxxxxxx
> > > Cc: linux-ext4@xxxxxxxxxxxxxxx
> > > Subject: [PATCH 05/37] debugfs: teach logdump to deal with 64bit revoke tables
> > >
> > > The logdump command doesn't know how to deal with revoke tables in
> > > 64bit journals, so teach it to do this.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > ---
> > > debugfs/logdump.c | 20 ++++-
> > > tests/f_jnl_64bit/expect.0 | 171 --------------------------------------------
> > > 2 files changed, 15 insertions(+), 176 deletions(-)
> > >
> > >
> > > diff --git a/debugfs/logdump.c b/debugfs/logdump.c
> > > index 2d0efaf..8b9dc5b 100644
> > > --- a/debugfs/logdump.c
> > > +++ b/debugfs/logdump.c
> > > @@ -526,28 +526,38 @@ static void dump_revoke_block(FILE *out_file, char *buf,
> > > {
> > > int offset, max;
> > > journal_revoke_header_t *header;
> > > - unsigned int *entry, rblock;
> > > + unsigned int *entry;
> > > + unsigned long long *bentry, rblock;
> > > + int tag_size = sizeof(*entry);
> > >
> > > if (dump_all)
> > > fprintf(out_file, "Dumping revoke block, sequence %u, at "
> > > "block %u:\n", transaction, blocknr);
> > >
> > > + if (be32_to_cpu(jsb->s_feature_incompat) & JFS_FEATURE_INCOMPAT_64BIT)
> > > + tag_size = sizeof(*bentry);
> > > +
> > > header = (journal_revoke_header_t *) buf;
> > > offset = sizeof(journal_revoke_header_t);
> > > max = be32_to_cpu(header->r_count);
> > >
> > > while (offset < max) {
> > > - entry = (unsigned int *) (buf + offset);
> > > - rblock = be32_to_cpu(*entry);
> > > + if (tag_size == sizeof(*entry)) {
> > > + entry = (unsigned int *) (buf + offset);
> > > + rblock = be32_to_cpu(*entry);
> > > + } else {
> > > + bentry = (unsigned long long *)(buf + offset);
> > > + rblock = ext2fs_be64_to_cpu(*bentry);
> > > + }
> >
> > I wonder whether we really need to have bentry and entry since those
> > are just pointers and should be of the same size regardless of what
> > they are pointing at.
> >
> > Would not it be better from the readability pov ? Otherwise it looks
> > good.
>
> One could eliminate the local variables by writing it as such:
>
> if (...)
> rblock = be32_to_cpu(*((__u32 *)(buf + offset)));
> else
> rblock = ext2fs_be64_to_cpu(*((__u64 *)(buf + offset)));
>
> The parentheses are a little harder to figure out in the second version, but I
> don't have a strong opinion either way.
Yes, this seems better to me. But I do not have especially strong
opinion either.
Thanks!
-Lukas
>
> --D
> >
> > Thanks!
> > -Lukas
> >
> > > if (dump_all || rblock == block_to_dump) {
> > > - fprintf(out_file, " Revoke FS block %u", rblock);
> > > + fprintf(out_file, " Revoke FS block %llu", rblock);
> > > if (dump_all)
> > > fprintf(out_file, "\n");
> > > else
> > > fprintf(out_file," at block %u, sequence %u\n",
> > > blocknr, transaction);
> > > }
> > > - offset += 4;
> > > + offset += tag_size;
> > > }
> > > }
> > >
> > > diff --git a/tests/f_jnl_64bit/expect.0 b/tests/f_jnl_64bit/expect.0
> > > index 2007f03..5cef2d8 100644
> > > --- a/tests/f_jnl_64bit/expect.0
> > > +++ b/tests/f_jnl_64bit/expect.0
> > > @@ -1,189 +1,97 @@
> > > Journal starts at block 67, transaction 32
> > > Found expected sequence 32, type 5 (revoke table) at block 67
> > > Dumping revoke block, sequence 32, at block 67:
> > > - Revoke FS block 0
> > > Revoke FS block 1536
> > > - Revoke FS block 0
> > > Revoke FS block 1472
> > > - Revoke FS block 0
> > > Revoke FS block 1473
> > > - Revoke FS block 0
> > > Revoke FS block 1474
> > > - Revoke FS block 0
> > > Revoke FS block 1475
> > > - Revoke FS block 0
> > > Revoke FS block 1476
> > > - Revoke FS block 0
> > > Revoke FS block 1541
> > > - Revoke FS block 0
> > > Revoke FS block 1477
> > > - Revoke FS block 0
> > > Revoke FS block 1478
> > > - Revoke FS block 0
> > > Revoke FS block 1479
> > > - Revoke FS block 0
> > > Revoke FS block 1480
> > > - Revoke FS block 0
> > > Revoke FS block 1481
> > > - Revoke FS block 0
> > > Revoke FS block 1482
> > > - Revoke FS block 0
> > > Revoke FS block 1483
> > > - Revoke FS block 0
> > > Revoke FS block 1484
> > > - Revoke FS block 0
> > > Revoke FS block 1485
> > > - Revoke FS block 0
> > > Revoke FS block 1486
> > > - Revoke FS block 0
> > > Revoke FS block 1487
> > > - Revoke FS block 0
> > > Revoke FS block 1488
> > > - Revoke FS block 0
> > > Revoke FS block 1489
> > > - Revoke FS block 0
> > > Revoke FS block 1490
> > > - Revoke FS block 0
> > > Revoke FS block 1491
> > > - Revoke FS block 0
> > > Revoke FS block 1556
> > > - Revoke FS block 0
> > > Revoke FS block 1492
> > > - Revoke FS block 0
> > > Revoke FS block 1493
> > > - Revoke FS block 0
> > > Revoke FS block 1429
> > > - Revoke FS block 0
> > > Revoke FS block 1494
> > > - Revoke FS block 0
> > > Revoke FS block 1495
> > > - Revoke FS block 0
> > > Revoke FS block 1496
> > > - Revoke FS block 0
> > > Revoke FS block 1432
> > > - Revoke FS block 0
> > > Revoke FS block 1497
> > > - Revoke FS block 0
> > > Revoke FS block 1498
> > > - Revoke FS block 0
> > > Revoke FS block 1434
> > > - Revoke FS block 0
> > > Revoke FS block 1499
> > > - Revoke FS block 0
> > > Revoke FS block 1435
> > > - Revoke FS block 0
> > > Revoke FS block 1500
> > > - Revoke FS block 0
> > > Revoke FS block 1501
> > > - Revoke FS block 0
> > > Revoke FS block 1502
> > > - Revoke FS block 0
> > > Revoke FS block 1503
> > > - Revoke FS block 0
> > > Revoke FS block 1504
> > > - Revoke FS block 0
> > > Revoke FS block 1505
> > > - Revoke FS block 0
> > > Revoke FS block 1506
> > > - Revoke FS block 0
> > > Revoke FS block 1442
> > > - Revoke FS block 0
> > > Revoke FS block 1507
> > > - Revoke FS block 0
> > > Revoke FS block 1508
> > > - Revoke FS block 0
> > > Revoke FS block 1444
> > > - Revoke FS block 0
> > > Revoke FS block 1509
> > > - Revoke FS block 0
> > > Revoke FS block 1445
> > > - Revoke FS block 0
> > > Revoke FS block 1510
> > > - Revoke FS block 0
> > > Revoke FS block 1511
> > > - Revoke FS block 0
> > > Revoke FS block 1512
> > > - Revoke FS block 0
> > > Revoke FS block 1513
> > > - Revoke FS block 0
> > > Revoke FS block 1449
> > > - Revoke FS block 0
> > > Revoke FS block 1514
> > > - Revoke FS block 0
> > > Revoke FS block 1515
> > > - Revoke FS block 0
> > > Revoke FS block 1516
> > > - Revoke FS block 0
> > > Revoke FS block 1517
> > > - Revoke FS block 0
> > > Revoke FS block 1453
> > > - Revoke FS block 0
> > > Revoke FS block 1518
> > > - Revoke FS block 0
> > > Revoke FS block 1519
> > > - Revoke FS block 0
> > > Revoke FS block 1520
> > > - Revoke FS block 0
> > > Revoke FS block 1456
> > > - Revoke FS block 0
> > > Revoke FS block 1521
> > > - Revoke FS block 0
> > > Revoke FS block 1457
> > > - Revoke FS block 0
> > > Revoke FS block 1522
> > > - Revoke FS block 0
> > > Revoke FS block 1458
> > > - Revoke FS block 0
> > > Revoke FS block 1523
> > > - Revoke FS block 0
> > > Revoke FS block 1459
> > > - Revoke FS block 0
> > > Revoke FS block 1524
> > > - Revoke FS block 0
> > > Revoke FS block 1460
> > > - Revoke FS block 0
> > > Revoke FS block 1525
> > > - Revoke FS block 0
> > > Revoke FS block 1461
> > > - Revoke FS block 0
> > > Revoke FS block 1526
> > > - Revoke FS block 0
> > > Revoke FS block 1462
> > > - Revoke FS block 0
> > > Revoke FS block 1527
> > > - Revoke FS block 0
> > > Revoke FS block 1463
> > > - Revoke FS block 0
> > > Revoke FS block 1528
> > > - Revoke FS block 0
> > > Revoke FS block 1464
> > > - Revoke FS block 0
> > > Revoke FS block 1529
> > > - Revoke FS block 0
> > > Revoke FS block 1465
> > > - Revoke FS block 0
> > > Revoke FS block 1530
> > > - Revoke FS block 0
> > > Revoke FS block 1466
> > > - Revoke FS block 0
> > > Revoke FS block 1531
> > > - Revoke FS block 0
> > > Revoke FS block 1467
> > > - Revoke FS block 0
> > > Revoke FS block 1532
> > > - Revoke FS block 0
> > > Revoke FS block 1468
> > > - Revoke FS block 0
> > > Revoke FS block 1533
> > > - Revoke FS block 0
> > > Revoke FS block 1469
> > > - Revoke FS block 0
> > > Revoke FS block 1534
> > > - Revoke FS block 0
> > > Revoke FS block 1470
> > > - Revoke FS block 0
> > > Revoke FS block 1535
> > > - Revoke FS block 0
> > > Revoke FS block 1471
> > > Found expected sequence 32, type 1 (descriptor block) at block 68
> > > Dumping descriptor block, sequence 32, at block 68:
> > > @@ -323,163 +231,84 @@ Dumping descriptor block, sequence 32, at block 150:
> > > Found expected sequence 32, type 2 (commit block) at block 201
> > > Found expected sequence 33, type 5 (revoke table) at block 202
> > > Dumping revoke block, sequence 33, at block 202:
> > > - Revoke FS block 0
> > > Revoke FS block 1600
> > > - Revoke FS block 0
> > > Revoke FS block 1601
> > > - Revoke FS block 0
> > > Revoke FS block 1537
> > > - Revoke FS block 0
> > > Revoke FS block 1602
> > > - Revoke FS block 0
> > > Revoke FS block 1538
> > > - Revoke FS block 0
> > > Revoke FS block 1603
> > > - Revoke FS block 0
> > > Revoke FS block 1539
> > > - Revoke FS block 0
> > > Revoke FS block 1604
> > > - Revoke FS block 0
> > > Revoke FS block 1540
> > > - Revoke FS block 0
> > > Revoke FS block 1605
> > > - Revoke FS block 0
> > > Revoke FS block 1606
> > > - Revoke FS block 0
> > > Revoke FS block 1542
> > > - Revoke FS block 0
> > > Revoke FS block 1607
> > > - Revoke FS block 0
> > > Revoke FS block 1543
> > > - Revoke FS block 0
> > > Revoke FS block 1608
> > > - Revoke FS block 0
> > > Revoke FS block 1544
> > > - Revoke FS block 0
> > > Revoke FS block 1609
> > > - Revoke FS block 0
> > > Revoke FS block 1545
> > > - Revoke FS block 0
> > > Revoke FS block 1610
> > > - Revoke FS block 0
> > > Revoke FS block 1546
> > > - Revoke FS block 0
> > > Revoke FS block 1611
> > > - Revoke FS block 0
> > > Revoke FS block 1547
> > > - Revoke FS block 0
> > > Revoke FS block 1612
> > > - Revoke FS block 0
> > > Revoke FS block 1548
> > > - Revoke FS block 0
> > > Revoke FS block 1613
> > > - Revoke FS block 0
> > > Revoke FS block 1549
> > > - Revoke FS block 0
> > > Revoke FS block 1614
> > > - Revoke FS block 0
> > > Revoke FS block 1550
> > > - Revoke FS block 0
> > > Revoke FS block 1615
> > > - Revoke FS block 0
> > > Revoke FS block 1551
> > > - Revoke FS block 0
> > > Revoke FS block 1616
> > > - Revoke FS block 0
> > > Revoke FS block 1552
> > > - Revoke FS block 0
> > > Revoke FS block 1617
> > > - Revoke FS block 0
> > > Revoke FS block 1553
> > > - Revoke FS block 0
> > > Revoke FS block 1554
> > > - Revoke FS block 0
> > > Revoke FS block 1555
> > > - Revoke FS block 0
> > > Revoke FS block 1557
> > > - Revoke FS block 0
> > > Revoke FS block 1558
> > > - Revoke FS block 0
> > > Revoke FS block 1559
> > > - Revoke FS block 0
> > > Revoke FS block 1560
> > > - Revoke FS block 0
> > > Revoke FS block 1561
> > > - Revoke FS block 0
> > > Revoke FS block 1562
> > > - Revoke FS block 0
> > > Revoke FS block 1563
> > > - Revoke FS block 0
> > > Revoke FS block 1564
> > > - Revoke FS block 0
> > > Revoke FS block 1565
> > > - Revoke FS block 0
> > > Revoke FS block 1566
> > > - Revoke FS block 0
> > > Revoke FS block 1567
> > > - Revoke FS block 0
> > > Revoke FS block 1568
> > > - Revoke FS block 0
> > > Revoke FS block 1569
> > > - Revoke FS block 0
> > > Revoke FS block 1570
> > > - Revoke FS block 0
> > > Revoke FS block 1571
> > > - Revoke FS block 0
> > > Revoke FS block 1572
> > > - Revoke FS block 0
> > > Revoke FS block 1573
> > > - Revoke FS block 0
> > > Revoke FS block 1574
> > > - Revoke FS block 0
> > > Revoke FS block 1575
> > > - Revoke FS block 0
> > > Revoke FS block 1576
> > > - Revoke FS block 0
> > > Revoke FS block 1577
> > > - Revoke FS block 0
> > > Revoke FS block 1578
> > > - Revoke FS block 0
> > > Revoke FS block 1579
> > > - Revoke FS block 0
> > > Revoke FS block 1580
> > > - Revoke FS block 0
> > > Revoke FS block 1581
> > > - Revoke FS block 0
> > > Revoke FS block 1582
> > > - Revoke FS block 0
> > > Revoke FS block 1583
> > > - Revoke FS block 0
> > > Revoke FS block 1584
> > > - Revoke FS block 0
> > > Revoke FS block 1585
> > > - Revoke FS block 0
> > > Revoke FS block 1586
> > > - Revoke FS block 0
> > > Revoke FS block 1587
> > > - Revoke FS block 0
> > > Revoke FS block 1588
> > > - Revoke FS block 0
> > > Revoke FS block 1589
> > > - Revoke FS block 0
> > > Revoke FS block 1590
> > > - Revoke FS block 0
> > > Revoke FS block 1591
> > > - Revoke FS block 0
> > > Revoke FS block 1592
> > > - Revoke FS block 0
> > > Revoke FS block 1593
> > > - Revoke FS block 0
> > > Revoke FS block 1594
> > > - Revoke FS block 0
> > > Revoke FS block 1595
> > > - Revoke FS block 0
> > > Revoke FS block 1596
> > > - Revoke FS block 0
> > > Revoke FS block 1597
> > > - Revoke FS block 0
> > > Revoke FS block 1598
> > > - Revoke FS block 0
> > > Revoke FS block 1599
> > > Found expected sequence 33, type 1 (descriptor block) at block 203
> > > Dumping descriptor block, sequence 33, at block 203:
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
