Re: dm-crypt using kthread (was: Oopsing cryptoapi (or loop device?) on 2.6.*)

[Andrew Morton <akpm@osdl.org>]

Christophe Saout <christophe@saout.de> wrote:
>
> +static struct bio *
> +crypt_alloc_buffer(struct crypt_config *cc, unsigned int size,
> +                   struct bio *base_bio, int *bio_vec_idx)
> +{
> ...
> +	/*
> +	 * Tell VM to act less aggressively and fail earlier.
> +	 * This is not necessary but increases throughput.
> +	 * FIXME: Is this really intelligent?
> +	 */
> +	current->flags &= ~PF_MEMALLOC;

This is a bit peculiar.  Is it still the case that it increases throughput?
 How come?

> +	if (base_bio)
> +		bio = bio_clone(base_bio, GFP_NOIO);
> +	else
> +		bio = bio_alloc(GFP_NOIO, nr_iovecs);
> +	if (!bio)
> +		return NULL;

Should restore PF_MEMALLOC here.

> +static int kcryptd_do_work(void *data)
> +{
> +	current->flags |= PF_IOTHREAD;
> +
> +	for(;;) {
> +		struct bio *bio;
> +
> +		set_task_state(current, TASK_INTERRUPTIBLE);
> +		while (!(bio = kcryptd_get_bios())) {
> +			schedule();
> +			if (signal_pending(current))
> +				return 0;
> +		}

This will turn into a busy-loop, because schedule() sets current->state to
TASK_RUNNING.  You need to move the set_task_state(current,
TASK_INTERRUPTIBLE); inside the loop.

Why is this code mucking with signals?

Perhaps a call to blk_congestion_wait() would be appropriate here.

> +/*
> + * Encode key into its hex representation
> + */
> +static void crypt_encode_key(char *hex, u8 *key, int size)
> +{
> +	static char hex_digits[] = "0123456789abcdef";
> +	int i;
> +
> +	for(i = 0; i < size; i++) {
> +		*hex++ = hex_digits[*key >> 4];
> +		*hex++ = hex_digits[*key & 0x0f];
> +		key++;
> +	}
> +
> +	*hex++ = '\0';
> +}

sprintf("%02x")?