Re: [[ EXT ]] [BUG REPORT] cifs/smb data corruption when writing, x86_64, kernel 6.6.71

[Mark A Whiting <whitingm@opentext.com>]

Hi Enzo,

I now have a couple days of testing done. The good news is that we've run several terabytes of data through cifs and haven't had a single failure with the patch you provided.

Now for the not as good news. Even though we aren't seeing any data corruption or failures. We are regularly and very frequently hitting a WARN_ON in cifs_extend_writeback() on line 2866. 

>for (i = 0; i < folio_batch_count(&batch); i++) {
>	folio = batch.folios[i];
>	/* The folio should be locked, dirty and not undergoing
>	 * writeback from the loop above.
>	 */
>	if (!folio_clear_dirty_for_io(folio))
>		WARN_ON(1);

Reading through the folio_clear_dirty_for_io() function it appears the only way this would happen is if the folio is clean, i.e., the dirty flag is not set.

>if (folio_test_writeback(folio)) {
>	/*
>	 * For data-integrity syscalls (fsync(), msync()) we must wait for
>	 * the I/O to complete on the page.
>	 * For other cases (!sync), we can just skip this page, even if
>	 * it's dirty.
>	 */
>	if (!sync) {
>		stop = false;
>		goto unlock_next;
>	} else {
>		folio_wait_writeback(folio);

Reading through your patch, unless I missed something, this folio_wait_writeback() call is the only addition that could affect the dirty flag indirectly. I'm assuming that when the current writeback is complete it would mark the folio clean. Then when it's added to the current batch and later checked, it's clean instead of the dirty flag being set as expected.

Since you wrote the patch, is this expected behavior and that WARN_ON isn't valid anymore? Or is this something I should be worried about?

Thanks,
Mark Whiting