On Friday, September 08, 2006 2:00 PM, Mike Christie wrote > > Mike Christie wrote: > > Edward Goggin wrote: > >> + > >> + rq->buffer = rq->data = h->buffer; > >> + rq->data_len = len; > >> + rq->bio = rq->biotail = NULL; > >> > > > > I think I only suggested that you use the block layer map > functions in > > the previous review. Now I will say, use them and fix the > core code to > > not allocate memory or use a mempool since it will also fix the path > > testers at the same time :) The reason is that the scsi > layer is trying > > to make every thing run with scatterlists. In 2.6.18, every place is > > converted (they should be at least), so you should not be > adding another > > place where we send down a data buffer like this. > > > > And if we are going to continue to do some scsi processing in dm I > already did some helpers you could base yourself off of here > > http://www.cs.wisc.edu/~michaelc/block/dm/v4/ I was first trying to submit these changes to the upstream code (I chose 2.6.18-rc4) since I this is what I thought you had suggested that I do when we talked about it briefly at the dm bof session at OLS. Should I not be taking this tack? > > > In dm_scsi_end_rq, you could handle errors like transient transport > failures for all hw handlers at one tine. > > http://www.cs.wisc.edu/~michaelc/block/dm/v4/02-dm-scsi-helpers.patch Again, doesn't this imply a change from what we already agreed upon? That is fine, I just need to know. > > And in fact a lot of your patch has already been done with this patch > > http://www.cs.wisc.edu/~michaelc/block/dm/v4/03-emc.patch a year ago. I certainly read your patch set at that time it was sent since this is how I learned about the availability of the blk_execute_rq_nowait interface. Yet, I certainly didn't mean to plagiarize any of your code. I apologize if I have inadvertently done so. I definitely remember having a difficult time getting the io initiated by dm-emc.c to work at all with my changes. Turns out I was trying to DMA from kernel static sections (as is your code). Yet, I don't remember looking back at your patch set for inspiration to solve the problem. The primary intention of my patch is to (1) allow dm-emc.c to distinguish between dm and non-dm initiated multipath path group switch events, (2) provide a dm-multipath hw-handler status callback for dm-emc.c, and (3) provide a somewhat more informative error logging capability for the dm-multipath hw-handler interface. I was also intent on having the possibly multiple ios initiated by dm-emc.c not involve allocating memory for request or bio structures from mempools or otherwise. Doing so is proving especially difficult. > > We really need to change how dm-multipath development works IMHO. > Alasdair is just too overworked. It is not fair to him to have so much > work piled on top of him and not fair to the community to > have problems > like this sit around for so long. > > The basics of the hw handler framework were done when I did > my pg group > callouts that improved on Joe's idea to put everything in the path > selectors. It took a year after that to get hw handlers, > which made some > of the same architectural mistakes as well as odd coding > choices that I > made, in the kernel. And now for another year we have been sitting on > these type of bugs and problems where we need some of the hw handler > functionality in the scsi layer as well as dm layer. > > With Alasdair having to maintain every driver that dm handles > and those > drivers increasing in complexity the work load is too large for one > person and this makes coordination with other subsystems impossible. > This is a large problem, when for something like dm-multipath it needs > help from the block layer, and low levels like scsi. > > We really need a system like in the scsi layer where there are driver > maintainers and then one core maintainer of the subsystem. The core > maintainer has final (well ok community has final say I guess) say but > trusts the low level driver maintainers to some extent. The low level > driver maintainers still have to post patches for review to the > community and maintainer but the core maintainer does not have to do a > line by line review of the dm target module and learn all the lower > level details of the sub-subsytem/lld/dm target module/transport/spec > unless it affects dm core or it is a suspicious patch. > > Just my 2 cents. Good ideas. This approach should scale more effectively. > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel