ffs_backgroundwritedone: Set INVAL on errored IOs to avoid bdirty

[cse_cem_gmail_com]

Summary

It is invalid to dirty an uncached buf, so do not. This avoids a
scenario where we would attempt to dirty a buf with NULL bufobj,
resulting in an ASSERT (INVARIANTS kernel) or NULL dereference
(non-DEBUG kernel).

Test Plan

Repro:

db> show mount
0xfffff80007833330 /dev/vtbd0p2 on / (ufs)

...
$ kgdb
...
(gdb) p ((struct ufsmount*)((struct mount*)0xfffff80007833330).mnt_data).um_devvp
$5 = (struct vnode *) 0xfffff80007846ce8

...
$ sysctl 'debug.fail_point.ffs_backgroundwritedone_error=return(0x07846ce8)'
$ sync

(Before): Panic in bdirty -> reassignbuf
(After): No panic, log shows "ffs_backgroundwritedone: Erroring IO ..."

[kib]

I would add a comment explaining the reason for B_INVAL there. Isn't there a bigger problem with the BIO_ERROR ? The real cylinder group buffer is marked clean in ffs_bufwrite(), which means that we lost the update, making the filesystem inconsistent.

For testing, Isilon contributed the KFAIL thing, which is mostly unused. Look at the buf_vm_page_count_severe how it was intended to introduce the sporadic faults at the selected points.

[cse_cem_gmail_com]

In D2900#56463, @kib wrote:

I would add a comment explaining the reason for B_INVAL there.

Ok.

Isn't there a bigger problem with the BIO_ERROR ? The real cylinder group buffer is marked clean in ffs_bufwrite(), which means that we lost the update, making the filesystem inconsistent.

Yep. In our case CAM kicked out the backing device on error so the filesystem is completely gone. I don't know if CAM does this on vanilla FreeBSD.

(ada1:ata3:0:1:0): WRITE_DMA. ACB: ca 00 c0 a8 92 40 00 00 00 00 20 00
(ada1:ata3:0:1:0): CAM status: ATA Status Error
(ada1:ata3:0:1:0): ATA status: 51 (DRDY SERV ERR), error: 10 (IDNF )
(ada1:ata3:0:1:0): RES: 51 10 c0 a8 92 00 00 00 00 20 00
(ada1:ata3:0:1:0): Retrying command

In D2900#56463, @kib wrote:

For testing, Isilon contributed the KFAIL thing, which is mostly unused. Look at the buf_vm_page_count_severe how it was intended to introduce the sporadic faults at the selected points.

Sure, it's easy enough to add something like this. It seems like we could vgone() the devvp here to match the scenario I am seeing. I am gating it on the devvp for now, but this is somewhat hacky. if you have a better suggestion, please let me know.

[cse_cem_gmail_com]

Address kib's feedback; add B_INVAL comment, add failpoint.

We cannot vgone() the devvp at this point because we can't sleep on the devvp
vnode lock in g_up thread.

[cse_cem_gmail_com]

Landed, with Konstantin's additional work (and Kirk's blessing) to avoid *CORRUPTION*, in r284887.