2018-09-28 23:16:05

by Eric Whitney

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] ext4: fix reserved cluster accounting at delayed write time

* kbuild test robot <[email protected]>:
> Hi Eric,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on ext4/dev]
> [also build test ERROR on v4.19-rc3 next-20180913]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Eric-Whitney/ext4-rework-bigalloc-reserved-cluster-accounting/20180914-053634
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> config: x86_64-randconfig-s4-09140719 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> Note: the linux-review/Eric-Whitney/ext4-rework-bigalloc-reserved-cluster-accounting/20180914-053634 HEAD efc30d747afd91b3bd9eb7fd218d0d1f7613c5a0 builds fine.
> It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
> fs//ext4/inode.c: In function 'ext4_insert_delayed_block':
> >> fs//ext4/inode.c:1816:33: error: 'ext4_es_is_delonly' undeclared (first use in this function); did you mean 'ext4_es_is_delayed'?
> if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
> ^~~~~~~~~~~~~~~~~~
> ext4_es_is_delayed
> fs//ext4/inode.c:1816:33: note: each undeclared identifier is reported only once for each function it appears in
>
> vim +1816 fs//ext4/inode.c
>
> 1782
> 1783 /*
> 1784 * ext4_insert_delayed_block - adds a delayed block to the extents status
> 1785 * tree, incrementing the reserved cluster/block
> 1786 * count or making a pending reservation
> 1787 * where needed
> 1788 *
> 1789 * @inode - file containing the newly added block
> 1790 * @lblk - logical block to be added
> 1791 *
> 1792 * Returns 0 on success, negative error code on failure.
> 1793 */
> 1794 static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> 1795 {
> 1796 struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> 1797 int ret;
> 1798 bool allocated = false;
> 1799
> 1800 /*
> 1801 * If the cluster containing lblk is shared with a delayed,
> 1802 * written, or unwritten extent in a bigalloc file system, it's
> 1803 * already been accounted for and does not need to be reserved.
> 1804 * A pending reservation must be made for the cluster if it's
> 1805 * shared with a written or unwritten extent and doesn't already
> 1806 * have one. Written and unwritten extents can be purged from the
> 1807 * extents status tree if the system is under memory pressure, so
> 1808 * it's necessary to examine the extent tree if a search of the
> 1809 * extents status tree doesn't get a match.
> 1810 */
> 1811 if (sbi->s_cluster_ratio == 1) {
> 1812 ret = ext4_da_reserve_space(inode);
> 1813 if (ret != 0) /* ENOSPC */
> 1814 goto errout;
> 1815 } else { /* bigalloc */
> > 1816 if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
> 1817 if (!ext4_es_scan_clu(inode,
> 1818 &ext4_es_is_mapped, lblk)) {
> 1819 ret = ext4_clu_mapped(inode,
> 1820 EXT4_B2C(sbi, lblk));
> 1821 if (ret < 0)
> 1822 goto errout;
> 1823 if (ret == 0) {
> 1824 ret = ext4_da_reserve_space(inode);
> 1825 if (ret != 0) /* ENOSPC */
> 1826 goto errout;
> 1827 } else {
> 1828 allocated = true;
> 1829 }
> 1830 } else {
> 1831 allocated = true;
> 1832 }
> 1833 }
> 1834 }
> 1835
> 1836 ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
> 1837
> 1838 errout:
> 1839 return ret;
> 1840 }
> 1841
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation


Hi-

Please pardon the delay - I'm just back from vacation.

Thanks for catching this. Although this patch series isn't functionally
bisectable (in the sense that ext4 won't function correctly during a
bisection within this patch series - unavoidably, all six patches are
required for the code to work), failing compilation after a patch needs
to be fixed. It is, in the v3 I've just posted, by simply moving the
definition of ext4_es_is_delonly() one patch earlier in the sequence.

Thanks for the testing!
Eric