Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] ext4: properly check for dirty state in
 ext4_inode_datasync_dirty()
To:     "Theodore Y. Ts'o" <tytso@mit.edu>
Cc:     harshad shirwadkar <harshadshirwadkar@gmail.com>,
        Andrea Righi <andrea.righi@canonical.com>,
        Andreas Dilger <adilger.kernel@dilger.ca>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>,
        linux-kernel@vger.kernel.org
References: <20201024140115.GA35973@xps-13-7390>
 <CAD+ocby3hA0GCm5Rf6T3UF+2UWgWoUjrz7=VzbeUMjX6Qx8D5g@mail.gmail.com>
 <da6697a0-4a23-ee68-fa2e-121b3d23c972@linux.ibm.com>
 <20201028152930.GQ5691@mit.edu>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Message-ID: <0a81bf78-1ccc-d10a-67f0-f6d24119408c@linux.ibm.com>
Date:   Wed, 28 Oct 2020 22:56:04 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <20201028152930.GQ5691@mit.edu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 10/28/20 8:59 PM, Theodore Y. Ts'o wrote:
> On Wed, Oct 28, 2020 at 08:57:03AM +0530, Ritesh Harjani wrote:
>>
>> Well, I too noticed this yesterday while I was testing xfstests -g swap.
>> Those tests were returning _notrun, hence that could be the reason why
>> it didn't get notice in XFSTESTing from Ted.
> 
> Yeah, one of the things I discussed with Harshad is we really need a
> test that looks like generic/472, but which is in shared/NNN, and
> which unconditionally tries to use swapon for those file systems where
> swapfiles are expected to work.  This is actually the second
> regression caused by our breaking swapfile support (the other being
> the iomap bmap change), which escaped our testing because we didn't
> notice that generic/472 was skipped.

Yes, agreed this is second in a row.
So with fast-commit, swap tests returned _not_run, since
swapon syscall returned -EINVAL in _require_scratch_swapfile() itself.
This is due to some old commit in fstests to make swap tests work on
btrfs on both kernels (with and w/o support of swapon in btrfs), it
first checks in _require_scratch_swapfile() to see if swapon even works
or not. Hence it skips to run further if _require_scratch_swapfile()
fails.

Secondly with bmap to iomap interface, I guess it should pass
all tests except for case with fallocate files, which I think is
tests/generic/496. But here too it assumes that if 1st time it fails
with falloc then swapon may not be supported for that fs and hence does
_notrun.

I am actually working on this to make these swap tests return some
definitive pass or failure status. Will be sending some patches soon.
I could use your idea to add a test in shared/NNN for testing swap with
fallocate files for ext4 and xfs (for bmap to iomap ext4 regression
category of tests)

Thanks
-ritesh