Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031409AbbD2Aet (ORCPT ); Tue, 28 Apr 2015 20:34:49 -0400 Received: from mail-lb0-f178.google.com ([209.85.217.178]:33786 "EHLO mail-lb0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031083AbbD2Aep (ORCPT ); Tue, 28 Apr 2015 20:34:45 -0400 MIME-Version: 1.0 In-Reply-To: <1430259419.2181.26.camel@HansenPartnership.com> References: <1429973776-7499-1-git-send-email-akinobu.mita@gmail.com> <20150428142743.578d1c930aca013b596d7546@linux-foundation.org> <1430259419.2181.26.camel@HansenPartnership.com> Date: Wed, 29 Apr 2015 09:34:42 +0900 Message-ID: Subject: Re: [PATCH] scatterlist: enable sg chaining for all architectures From: Akinobu Mita To: James Bottomley Cc: Andrew Morton , LKML , Arnd Bergmann , linux-arch@vger.kernel.org, Christoph Hellwig , "linux-scsi@vger.kernel.org" , "Nicholas A. Bellinger" , target-devel@vger.kernel.org, Parisc List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2515 Lines: 55 2015-04-29 7:16 GMT+09:00 James Bottomley : > On Tue, 2015-04-28 at 14:27 -0700, Andrew Morton wrote: >> On Sat, 25 Apr 2015 23:56:16 +0900 Akinobu Mita 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-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/