Supress Buffer I/O errors when SCSI REQ_QUIET flag set

Allow the scsi request REQ_QUIET flag to be propagated to the buffer file system layer. The basic ideas is to pass the flag from the scsi request to the bio (block IO) and then to the buffer layer. The buffer layer can then suppress needless printks.

This patch declutters the kernel log by removed the 40-50 (per lun) buffer io error messages seen during a boot in my multipath setup . It is a good chance any real errors will be missed in the "noise" it the logs without this patch.

During boot I see blocks of messages like
"
__ratelimit: 211 callbacks suppressed
Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242847 Buffer I/O error on device sdm, logical block 1 Buffer I/O error on device sdm, logical block 5242878 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242872 "
in my logs.

My disk environment is multipath fiber channel using the SCSI_DH_RDAC code and multipathd. This topology includes an "active" and "ghost" path for each lun. IO's to the "ghost" path will never complete and the SCSI layer, via the scsi device handler rdac code, quick returns the IOs to theses paths and sets the REQ_QUIET scsi flag to suppress the scsi layer messages.

I am wanting to extend the QUIET behavior to include the buffer file
system layer to deal with these errors as well. I have been running this patch for a while now on several boxes without issue. A few runs of bonnie++ show no noticeable difference in performance in my setup.

Thanks for John Stultz for the quiet_error finalization.

Quoted 33 lines · Keith Mannthey

Allow the scsi request REQ_QUIET flag to be propagated to the buffer file system layer. The basic ideas is to pass the flag from the scsi request to the bio (block IO) and then to the buffer layer. The buffer layer can then suppress needless printks.

This patch declutters the kernel log by removed the 40-50 (per lun) buffer io error messages seen during a boot in my multipath setup . It is a good chance any real errors will be missed in the "noise" it the logs without this patch.

During boot I see blocks of messages like
"
__ratelimit: 211 callbacks suppressed
Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242847 Buffer I/O error on device sdm, logical block 1 Buffer I/O error on device sdm, logical block 5242878 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242879 Buffer I/O error on device sdm, logical block 5242872 "
in my logs.

My disk environment is multipath fiber channel using the SCSI_DH_RDAC code and multipathd. This topology includes an "active" and "ghost" path for each lun. IO's to the "ghost" path will never complete and the SCSI layer, via the scsi device handler rdac code, quick returns the IOs to theses paths and sets the REQ_QUIET scsi flag to suppress the scsi layer messages.

Quoted 6 lines

I am wanting to extend the QUIET behavior to include the buffer file
system layer to deal with these errors as well. I have been running this patch for a while now on several boxes without issue. A few runs of bonnie++ show no noticeable difference in performance in my setup.

Thanks for John Stultz for the quiet_error finalization.

Looks good to me. I'll merge it up for 2.6.29.

During the merge
window, giving a nice pile of rejects to keep me amused.

Seems that the rejects are due to other churn. Where the heck did all the slab defrag code disappear to??

I'll need to revert to yesterday's linux-next I think. Pekka, has that stuff been dropped?

Quoted 2 lines

I'll need to revert to yesterday's linux-next I think. Pekka, has that stuff been dropped?

Yup, there hasn't been any activity after the last merge window so I'm not expecting to be able to merge it for 2.6.29. As per linux-next rules, I dropped the 2.6.30 material until the merge window closes. The code is in topic/slub-defrag of slab.git, though.

Quoted 7 lines · Pekka Enberg

Quoted 2 lines

I'll need to revert to yesterday's linux-next I think. Pekka, has that stuff been dropped?

Yup, there hasn't been any activity after the last merge window so I'm not expecting to be able to merge it for 2.6.29. As per linux-next rules, I dropped the 2.6.30 material until the merge window closes. The code is in topic/slub-defrag of slab.git, though.

Well the problem currently is that there is no driver for these features. There has been a request for this functionality for a long time but both the primary drivers for this (Dave Chinner and I) are no longer with the company for which we started the project. I'd be glad to continue this if there would be an interest by the filesystem developers.

Quoted 1 line · On Tue, 25 Nov 2008 10:19:18 +0100 Jens Axboe <[email protected]> wrote:

On Mon, Nov 24 2008, Keith Mannthey wrote:

fs/buffer.c line 108

Quoted 7 lines

fs/buffer.c +6 -0

+static int quiet_error(struct buffer_head *bh)
+{
+       if (!test_bit(BH_Quiet, &bh->b_state) && printk_ratelimit())
+               return 0;
+       return 1;
+}

For better of for worse, we have a convention of using cpp-generated helper functions for the buffer_head flags. There's no reason why this new code needs to diverge from that. The above should use buffer_quiet().

The functions in fs/buffer.c have been nicely commented.

This function is poorly named. What does "quiet_error" *mean*?

<tries to work it out>

Every caller of this function does `if (!quiet_error(bh))'. Would it not make more sense to invert the sense of its return value?

static int permit_bh_errors(struct buffer_head *bh)

{
	if (buffer_quiet(bh))
		return 0;	/* IO layer suppressed error messages */
	return printk_ratelimit();
}

Did I translate that right? If so, then the addition of the printk_ratelimit() to the non-buffer_quiet() buffers is an unchangelogged and unrelated alteration.

The use of printk_ratelimit() needs some thought. It shares ratelimiting state with all other printk_ratelimit() callsites. Was that desirable? Would it have been better to create a private ratelimit_state for buffer_heads? Per physical device? Per something-else?

fs/buffer.c line 2924

Quoted 3 lines

fs/buffer.c +2 -0

+       if (unlikely (test_bit(BIO_QUIET,&bio->bi_flags)))
+               set_bit(BH_Quiet, &bh->b_state);

And the above (which has coding-style errors and has apparently not been checkpatched) should use set_buffer_quiet().

Quoted 9 lines

include/linux/buffer_head.h +1 -0

@@ -35,6 +35,7 @@ enum bh_state_bits {
        BH_Ordered,     /* ordered write */
        BH_Eopnotsupp,  /* operation not supported (barrier) */
        BH_Unwritten,   /* Buffer is allocated on disk but not written */
+       BH_Quiet,       /* Buffer Error Prinks to be quiet */
 

include/linux/buffer_head.h line 41

Quoted 2 lines

include/linux/buffer_head.h +0 -0

        BH_PrivateStart,/* not a state bit, but the first bit available
                         * for private allocation by other entities

Add

+ BUFFER_FNS(Quiet, quiet)

around line 123 to generate the helper functions.