Messages in this thread | | | From | Daniel Phillips <> | Subject | Re: [PATCH] ide write barrier support | Date | Mon, 20 Oct 2003 19:10:48 +0200 |
| |
Hi Jens,
On Monday 13 October 2003 16:08, Jens Axboe wrote: > Forward ported and tested today (with the dummy ext3 patch included), > works for me. Some todo's left, but I thought I'd send it out to gauge > interest.
This is highly interesting of course, but is it suitable for submission during the stability freeze? There is no correctness issue so long as no filesystem in mainline sets the BIO_RW_BARRIER bit, which appears to be the case. Therefore this is really a performance patch that introduces a new internal API.
It seems to me there are a few unresolved issues with the barrier API. It needs to be clearly stated that only write barriers are supported, not read or read/write barriers, if that is in fact the intention. Assuming it is, then BIOs with read barriers need to be failed.
The current BIO API provides no way to express a rw barrier, only read barriers and write barriers (the combination of direction bit and barrier bit indicates the barrier type). This is minor but it but how nice it would be if the API was either orthogonal or there was a clear explanation of why RW barriers never make sense. And if they don't, why read barriers do make sense. Another possible wart is that the API doesn't allow for a read barrier carried by a write BIO or a write barrier carried by a read BIO. From a practical point of view the only immediate use we have for barriers is to accelerate journal writes and everything else comes under the heading of R&D. It would help if the code clearly reflected that modest goal.
The BIO barrier scheme doesn't mesh properly with your proposed QUEUE_ORDERED_* scheme. It seems to me that what you want is just QUEUE_ORDERED_NONE and QUEUE_ORDERED_WRITE. Is there any case where the distinction between a tag based implemenation versus a flush matters to high level code?
Also, the blk_queue_ordered function isn't a sufficient interface to enable the functionality at a high level, a filesystem also needs a way to know whether barriers are supported or not, short of just submitting a barrier request and seeing if it fails.
The high level interface needs to be able to handled stacked devices, i.e., device mapper, but not just device mapper. Barriers have to be supported by all the devices in the stack, not just the top or bottom one. I don't have a concrete suggestion on what the interface should be just now.
The point of this is, there still remain a number of open issues with this patch, no doubt more than just the ones I touched on. Though it is clearly headed in the right direction, I'd suggest holding off during the stability freeze and taking the needed time to get it right.
Regards,
Daniel
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |