I am liking this diff quite a bit but it needs more testers. So if you
are using softraid crypto please try this diff.
On Fri, Jun 17, 2011 at 07:53:05PM +0100, Owain Ainsworth wrote:
So here's the problem: ENOMEM on io in a hba driver is bad juju.
In more detail:
When preparing for each io softraid crypto does a
sr_crypto_getcryptop(). This operation does a few things:
- allocate a uio and iovec from a pool.
- if this is a read then allocate a dma buffer with dma_alloc (up to
MAXPHYS, i.e. 64k)
- allocate a cryptop to be used with the crypto(9) framework.
- initialise the above for use in the io.
So if you are getting very low on memory, one of these operations
(probably the dma alloc) will fail. when this happens softraid returns
XS_DRIVER_STUFFUP to scsi land. This returns an EIO back to callers.
Now it is fairly common to use softdep in these situations, synchronous
filesystems are slw. softdep has certain assumptions about how
things will work. if a queued dependancy fails that invalidates these
assumptions. softraid panics in this case. I used to hit this repeatedly
(several times a day) before I bought more memory.
in general, most disk controllers prealloc everything they need (that
isn't passed down to them) so that this won't be a problem.
This diff does the same for softraid crypto. It'll eat just over 1meg of
kva, but it is hardly the only driver to need to do that.
The only slightly scary thing here is the cryptodesc list manipulations
(the comments explain why they are necesary).
I'm typing from a machine that runs this. todd@ is also testing it and
marco kindly did some horrible io torture on a machine running this too
and it runs quite nicely.
A side benefit is that due to missing out allocations in every disk io
this is actually slightly faster in io too.
More testing would be appreciated. Cheers,
-0-
diff --git dev/softraid_crypto.c dev/softraid_crypto.c
index 3259121..a60824e 100644
--- dev/softraid_crypto.c
+++ dev/softraid_crypto.c
@@ -56,9 +56,26 @@
#include dev/softraidvar.h
#include dev/rndvar.h
-struct cryptop *sr_crypto_getcryptop(struct sr_workunit *, int);
+/*
+ * the per-io data that we need to preallocate. We can't afford to allow io
+ * to start failing when memory pressure kicks in.
+ * We can store this in the WU because we assert that only one
+ * ccb per WU will ever be active.
+ */
+struct sr_crypto_wu {
+ TAILQ_ENTRY(sr_crypto_wu)cr_link;
+ struct uio cr_uio;
+ struct iovec cr_iov;
+ struct cryptop *cr_crp;
+ struct cryptodesc *cr_descs;
+ struct sr_workunit *cr_wu;
+ void*cr_dmabuf;
+};
+
+
+struct sr_crypto_wu *sr_crypto_wu_get(struct sr_workunit *, int);
+void sr_crypto_wu_put(struct sr_crypto_wu *);
int sr_crypto_create_keys(struct sr_discipline *);
-void *sr_crypto_putcryptop(struct cryptop *);
int sr_crypto_get_kdf(struct bioc_createraid *,
struct sr_discipline *);
int sr_crypto_decrypt(u_char *, u_char *, u_char *, size_t, int);
@@ -78,7 +95,7 @@ int sr_crypto_meta_opt_load(struct sr_discipline *,
struct sr_meta_opt *);
int sr_crypto_write(struct cryptop *);
int sr_crypto_rw(struct sr_workunit *);
-int sr_crypto_rw2(struct sr_workunit *, struct cryptop *);
+int sr_crypto_rw2(struct sr_workunit *, struct sr_crypto_wu *);
void sr_crypto_intr(struct buf *);
int sr_crypto_read(struct cryptop *);
void sr_crypto_finish_io(struct sr_workunit *);
@@ -230,40 +247,35 @@ done:
return (rv);
}
-struct cryptop *
-sr_crypto_getcryptop(struct sr_workunit *wu, int encrypt)
+
+struct sr_crypto_wu *
+sr_crypto_wu_get(struct sr_workunit *wu, int encrypt)
{
struct scsi_xfer*xs = wu-swu_xs;
struct sr_discipline*sd = wu-swu_dis;
- struct cryptop *crp = NULL;
+ struct sr_crypto_wu *crwu;
struct cryptodesc *crd;
- struct uio *uio = NULL;
- int flags, i, n, s;
+ int flags, i, n;
daddr64_t blk = 0;
u_int keyndx;
- DNPRINTF(SR_D_DIS, %s: sr_crypto_getcryptop wu: %p encrypt: %d\n,
+ DNPRINTF(SR_D_DIS, %s: sr_crypto_wu_get wu: %p encrypt: %d\n,
DEVNAME(sd-sd_sc), wu, encrypt);
- s = splbio();
- uio = pool_get(sd-mds.mdd_crypto.sr_uiopl, PR_ZERO | PR_NOWAIT);
- if (uio == NULL)
- goto poolunwind;
- uio-uio_iov = pool_get(sd-mds.mdd_crypto.sr_iovpl,
- PR_ZERO | PR_NOWAIT);
- if (uio-uio_iov == NULL)
- goto poolunwind;
- splx(s);
+ mtx_enter(sd-mds.mdd_crypto.scr_mutex);
+ if ((crwu =
