2015-04-29 7:16 GMT+09:00 James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>: > On Tue, 2015-04-28 at 14:27 -0700, Andrew Morton wrote: >> On Sat, 25 Apr 2015 23:56:16 +0900 Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote: >> >> > Some architectures enable sg chaining option while others do not. >> > >> > The requirement to enable sg chaining is that pages must be aligned >> > at a 32-bit boundary in order to overload the LSB of the pointer. >> > Regardless of whether ARCH_HAS_SG_CHAIN is defined or not, the above >> > requirement is always chacked by BUG_ON() in sg_assign_page. So >> > all architectures can enable sg chaining. >> > >> > As you can see from the changes in drivers/target/target_core_rd.c, >> > enabling SG chaining for all architectures allows us to allocate >> > discontiguous scatterlist tables which can be traversed throughout >> > by sg_next() without a special handling for some architectures. >> >> Thanks, I'll grab this. If anyone has concerns, speak now or hold both >> pieces! > > It breaks a host of architectures doesn't it? I can specifically speak > for PARISC: The problem is the way our iommus are consuming > scatterlists. They're assuming we can dereference the scatterlist as an > array (like this code in ccio-dma.c): > > static int > ccio_map_sg(struct device *dev, struct scatterlist *sglist, int nents, > enum dma_data_direction direction) > [...] > for(i = 0; i < nents; i++) > prev_len += sglist[i].length; > > If you turn on sg chaining on our architecture, we'll run off the end of > that array dereference and crash. > > This can all be fixed by making our architecture dma mapping code use > iterators instead of array lists, but that needs more code than this > patch provides. I assume there are similar issues on a lot of other > architectures, so before you can contemplate a patch like this, surely > all the architecture consumers have to be converted to iterator instead > of array format? > > The first place to start would be a survey of who's still using the > array format. Agreed. I could find similar issues in arch/m68k/kernel/dma.c. (git grep '[^a-z]sg++' shows that there are a lot of similar issues) Andrew, could you drop this patch from -mm for now? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
