Hi Alasdair, Mikulas, I have reviewed the barrier patch-set and found some possible regression from 2.6.29 in dm-implement-basic-barrier-support.patch: http://git.kernel.org/?p=linux/kernel/git/agk/linux-2.6-dm.git;a=commitdiff;h=af7e466a1acededbc10beaba9eec8531d561c566;hp=92c639021ca6e962645114f02e356e7feb131d0b Please take a look and consider to fix them (I don't have enough time to make patches now unfortunately). 1. The semantics of flush suspend has been changed. For flush suspend, dm used to flush all bios submitted before the invocation of suspend. But now, dm may not flush such bios: 1. submit a barrier => set QUEUE_IO_TO_THREAD flag and kick kdmflush 2. submit some bios => queued into md->defered if the barrier is still in progress 3. invoke suspend => set BLOCK_IO_FOR_SUSPEND flag => kdmflush stops flushing md->defered => complete suspend with holding the bios Then, we could expect bios submitted at 2 to be flushed, but we can't expect that any more now. I think no suspend user wants this semantics change. Is it intended change or just a bug? 2. Possible invalid pointer reference problem again. Milan had fixed a possible NULL pointer reference in 2.6.29 by http://git.kernel.org/?p=linux/kernel/git/agk/linux-2.6-dm.git;a=commitdiff;h=b35f8caa0890169000fec22902290d9a15274cbd;hp=b2174eebd1fadb76454dad09a1dacbc17081e6b0. His patch means that we must not touch md after calling bio_endio() in dec_pending() because the bio submitter can close/free the md once bio_endio() is called. The barrier patch breaks his fix and we get the possible invalid pointer reference problem again. The problematic part is below and free_io() should be called before bio_endio(). ---------------------------------------------------------------------- - free_io(md, io); + if (bio_barrier(bio)) { + /* + * There can be just one barrier request so we use + * a per-device variable for error reporting. + * Note that you can't touch the bio after end_io_acct + */ + md->barrier_error = io_error; + end_io_acct(io); + } else { + end_io_acct(io); - if (io_error != DM_ENDIO_REQUEUE) { - trace_block_bio_complete(md->queue, bio); + if (io_error != DM_ENDIO_REQUEUE) { + trace_block_bio_complete(md->queue, bio); - bio_endio(bio, io_error); + bio_endio(bio, io_error); + } } + + free_io(md, io); ---------------------------------------------------------------------- Thanks, Kiyoshi Ueda -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
