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

From: Christophe Saout
Date: Sun Feb 15 2004 - 21:55:28 EST


Hi,

[shoot me, I forgot the Cc's the first time]

> > + /*
> > + * 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?

Were there changes to the VM?

> How come?

I'm not exactly sure either. But this is what I suspected:

The VM wants to write out some pages. dm-crypt wants to allocate buffers
and starts digging into the reservers because PF_MEMALLOC is set which
causes some sort of low-memory condition.

If PF_MEMALLOC is dropped here the VM can just drop some cache pages in
order to allocate buffers.

If there wasn't a lot of free (unused) memory the machine often started
writing out data when I tried to write a lot of data using dd. The
seeking killed performance, just for the first seconds though.

It's not really important, I can drop that.

> Should restore PF_MEMALLOC here.

Right...

> > + 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.

Right again. I changed that several times. It shouldn't happen that
schedule returns but there's not data available, but ok. I changed the
while to an if and call kcryptd_get_bios after schedule().

> Why is this code mucking with signals?

For thread termination, that's what kthread does. The other kthread
users are doing this too. I changed the for(;;) back to a while
(!signal_pending(current)) since I killed the inner while loop.

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

Huh? Why that? This is the path for reads.

> sprintf("%02x")?

Ok. :)


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/