random vs blk-mq

A tester found out that we need add_disk_randomness for my scsi-mq branch. The patch applied exports add_disk_randomness so that modular scsi still works with a driver that either uses the low-level I/O completion routines for the old block layer, or blk-mq, and I'd love to get this in through Jens' tree which has all the other block work scsi-mq requires.

But this also brings up an interesting question: blk-mq currently does not set QUEUE_FLAG_ADD_RANDOM in the default queue flags, so simply converting a driver to blk-mq will mean it stops contributing to the random pool. Do we need a more fine grained way to control this, especially for SCSI?

This will be needed for pending changes to the scsi midlayer that now calls lower level block APIs, as well as any blk-mq driver that wants to contribute to the random pool.

---
 drivers/char/random.c |    1 +
 1 file changed, 1 insertion(+)

index 6b75713..0a19d86 100644
@@ -902,6 +902,7 @@ void add_disk_randomness(struct gendisk *disk)
 	add_timer_randomness(disk->random, 0x100 + disk_devt(disk));
 	trace_add_disk_randomness(disk_devt(disk), ENTROPY_BITS(&input_pool));
 }
+EXPORT_SYMBOL_GPL(add_disk_randomness);
 #endif
 
 /*********************************************************************

1.7.10.4

Jens, can you pull this into the block tree for me?

Yep, I'll add it, thanks.

Jens Axboe

In general, more fine grained control is always good.

But getting the defaults right is even more important. It occurs to me that what might make sense is to turn on QUEUE_FLAG_ADD_RANDOM if we know that it is a rotational disk. But in the case where we know it's a SSD or a NVMe, it's likely that add_disk_randomness() is not going to do much in the way that's useful, since we estimate entropy credits based on the jiffies delta, so if we interrupts more frequently than the 10ms, we're not going to get any entropy credit anyway.

Besides, we are sampling all interrupts, so we'll be gathering timing information and granting a minimal amount of entropy credit (on average 1/64'th of a bit of entropy per interupt) for each disk interrupt evenw/o the QUEUE_FLAG_ADD_RANDOM. The only reason to call add_disk_randomness() is if we want to give credit for the higher grade of entropy theoretically available from a disk interrupt --- which would only be true if we're talking about a rotational storage device anyway.

- Ted

This is exactly why the flag was added in the first place:

commit e2e1a148bc45855816ae6b4692ce29d0020fa22e

Author: Jens Axboe <[email protected]>
Date:   Wed Jun 9 10:42:09 2010 +0200

block: add sysfs knob for turning off disk entropy contributions

There are two reasons for doing this:

- On SSD disks, the completion times aren't as random as they are for rotational drives. So it's questionable whether they should contribute to the random pool in the first place.

- Calling add_disk_randomness() has a lot of overhead.

Jens Axboe

Or, we could just fix the horribly slow pool mix and have it always be on if it's "free", or almost free. Perhaps just default to on for rotating devices, that'd lessen the impact.

Jens Axboe