On Tue, Sep 09, 2025 at 08:49:18AM +0100, John Garry wrote:
> On 09/09/2025 07:44, Ojaswin Mujoo wrote:
> > > > > > +create_mixed_mappings() {
> > > > > Is this same as patch 08/12?
> > > > I believe you mean the [D]SYNC tests, yes it is the same.
> > > then maybe factor out the test, if possible. I assume that this sort of
> > > approach is taken for xfstests.
> > >
> > I'm not sure what you mean by factor out the*test*. We are testing
> > different things there and the only thing common in the tests is
> > creation of mixed mapping files and the check to ensure we didn't tear
> > data.
> >
> > In case you mean to factor out the create_mixed_mappings() helper into
> > common/rc, sure I can do that but I'm unsure if at this point it would
> > be very useful for other tests.
>
> above it was mentioned that the code in create_mixed_mappings was common, so
> that is why I suggested to factor it out. If it does not make sense, then
> fine (and don't).
Yes I mean Im unsure it will be useful for tests other than the two
tests in this patchset.
>
> >
> > > > > > + local file=$1
> > > > > > + local size_bytes=$2
> > > > > > +
> > > > > > + echo "# Filling file $file with alternate mappings till size $size_bytes" >> $seqres.full
> > > > > > + #Fill the file with alternate written and unwritten blocks
> > > > > > + local off=0
> > > > > > + local operations=("W" "U")
> > > > > > +
> > > > > > + for ((i=0; i<$((size_bytes / blksz )); i++)); do
> > > > > > + index=$(($i % ${#operations[@]}))
> > > > > > + map="${operations[$index]}"
> > > > > > +
> > <...>
>
>
> > > > > > + echo >> $seqres.full
> > > > > > + echo "# Starting filesize integrity test for atomic writes" >> $seqres.full
> > > > > what does "Starting filesize integrity test" mean?
> > > > Basically other tests already truncate the file to a higher value and
> > > > then perform the shut down test. Here we actually do append atomic
> > > > writes since we want to also stress the i_size update paths during
> > > > shutdown to ensure that doesn't cause any tearing with atomic writes.
> > > >
> > > > I can maybe rename it to:
> > > >
> > > >
> > > > echo "# Starting data integrity test for atomic append writes" >> $seqres.full
> > > >
> > > > Thanks for the review!
> > > >
> > > It's just the name "integrity" that throws me a bit..
> > So I mean integrity as in writes are not tearing after the shutdown.
> > That's how we have worded the other sub-tests above.
>
> you could mention "shutdown" also in the print.
Umm, do you mean something like:
"Starting shutdown data integrity tests ..."
Regards,
ojaswin
>
> Thanks!
>
