On Tue, Sep 25, 2012 at 07:51:07AM +0200, Hannes Reinecke wrote:
On 09/25/2012 12:34 AM, Kent Overstreet wrote:
quoted
+/**
+ * submit_bio_wait - submit a bio, and wait until it completes
+ * @rw: whether to %READ or %WRITE, or maybe to %READA (read ahead)
+ * @bio: The &struct bio which describes the I/O
+ *
+ * Simple wrapper around submit_bio(). Returns 0 on success, or the error from
+ * bio_endio() on failure.
+ */
+int submit_bio_wait(int rw, struct bio *bio)
+{
+ struct submit_bio_ret ret;
+
+ rw |= REQ_SYNC;
+ init_completion(&ret.event);
+ bio->bi_private = &ret;
+ bio->bi_end_io = submit_bio_wait_endio;
Hmm. As this is meant to be a generic function, blindly overwriting
the bi_end_io pointer doesn't look like a good idea; the caller
could have set something there.
Please add at least a WARN_ON(bio->bi_end_io) prior to modifying it.
Nah, the general rule with bios is after it's completed anything
could've been modified; we don't document or enforce otherwise with
bi_end_io (and there's a fair amount of code that saves/sets bi_end_io,
and I don't think it all restores the original before calling it).
I'm not going to special case this unless we start documenting/enforcing
it in general.
Besides that, setting a callback on something that's being used
synchronously is just dumb. Personally, I make damn sure to read and
understand code I'm using. I mean, maybe if this restriction was in the
slightest way subtle, but... how else would submit_bio_wait() be
implemented? It's kind of obvious if you think for two seconds about it.