> + if (bc->bc_key->q)
> + atomic_inc(&bc->bc_key->q->blk_key_use_count);
> }
I don't think a global atomic for each I/O is a good idea.
> +static int blk_crypto_destroy_default_key(struct request_queue *q)
> +{
> + int ret;
> +
> + if (!q->blk_key)
> + return 0;
> +
> + blk_mq_freeze_queue(q);
> + blk_mq_quiesce_queue(q);
> + if (atomic_read(&q->blk_key_use_count)) {
But once the queue is frozen all I/O is gone at least for blk-mq
drivers, so I don't think we'll need this as long as we limit
the feature to blk-mq drivers. Eventually we'll need to make
blk_mq_freeze_queue also track outstanding I/Os on bio based
drivers, in which case it will work there as well.
Alternatively we can support evicting the key only if the device
is not otherwise opened only.
> + if (q->blk_key) {
> + pr_err("Crypto key is already configured\n");
> + return -EBUSY;
> + }
Don't we nee locking to make this check race free?
> + switch (cmd) {
> + case BLKCRYPTOSETKEY:
> + if (mode & FMODE_EXCL)
> + return blk_crypto_ioctl_create_key(q, argp);
> +
> + if (IS_ERR(blkdev_get_by_dev(bdev->bd_dev, mode | FMODE_EXCL,
> + &bdev)))
> + return -EBUSY;
> + ret = blk_crypto_ioctl_create_key(q, argp);
> + blkdev_put(bdev, mode | FMODE_EXCL);
I think we can just safely require an FMODE_EXCL open here.
> #ifdef CONFIG_BLK_INLINE_ENCRYPTION
> struct blk_crypto_profile *crypto_profile;
> struct kobject *crypto_kobject;
> + struct blk_crypto_key *blk_key;
> + atomic_t blk_key_use_count;
> #endif
Both the existing and new fields really should go into the gendisk,
but I think we need to clean up the existing bits first
> +#ifndef __UAPI_LINUX_BLK_CRYPTO_H
> +#define __UAPI_LINUX_BLK_CRYPTO_H
> +
> +enum blk_crypto_mode_num {
> + BLK_ENCRYPTION_MODE_INVALID,
> + BLK_ENCRYPTION_MODE_AES_256_XTS,
> + BLK_ENCRYPTION_MODE_AES_128_CBC_ESSIV,
> + BLK_ENCRYPTION_MODE_ADIANTUM,
> + BLK_ENCRYPTION_MODE_MAX,
> +};
Please make these #defines so that userspace can probe for newly
added ones. a _MAX value should not be part of the uapi headers.
> +struct blk_crypto_set_key_arg {
Why is this in fs.h and not the new blk-cryto uapi header?
