From mboxrd@z Thu Jan 1 00:00:00 1970 From: Douglas Gilbert Subject: Re: [PATCH v2 2/9] dmapool: cleanup error messages Date: Fri, 3 Aug 2018 15:07:44 -0400 Message-ID: <7dac7664-c224-efba-8a7c-ccdc7a4ecd40@interlog.com> References: <7a943124-c65e-f0ed-cc5c-20b23f021505@cybernetics.com> <20180803162212.GA4718@bombadil.infradead.org> Reply-To: dgilbert-qazKcTl6WRFWk0Htik3J/w@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-CA List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Andy Shevchenko , Tony Battersby Cc: linux-scsi , Chaitra P B , Suganath Prabu Subramani , Sathya Prakash , Matthew Wilcox , linux-mm , iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, MPT-FusionLinux.pdl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org, Christoph Hellwig List-Id: linux-scsi@vger.kernel.org On 2018-08-03 02:38 PM, Andy Shevchenko wrote: > On Fri, Aug 3, 2018 at 8:03 PM, Tony Battersby wrote: >> On 08/03/2018 12:22 PM, Matthew Wilcox wrote: >>> On Fri, Aug 03, 2018 at 06:59:20PM +0300, Andy Shevchenko wrote: >>>>>>> I'm pretty sure this was created in an order to avoid bad looking (and >>>>>>> in some cases frightening) "NULL device *" part. >>>> JFYI: git log --no-merges --grep 'NULL device \*' >>> I think those commits actually argue in favour of Tony's patch to remove >>> the special casing. Is it really useful to create dma pools with a NULL >>> device? > >> dma_alloc_coherent() does appear to support a NULL dev, so it might make >> sense in theory. But I can't find any in-tree callers that actually >> pass a NULL dev to dma_pool_create(). So for one of the dreaded (NULL >> device *) messages to show up, it would take both a new caller that >> passes a NULL dev to dma_pool_create() and a bug to cause the message to >> be printed. Is that worth the special casing? > > So, then you need to rephrase the commit message explaining this > ("NULL device is wrong to pass in the first place... bla bla bla"). > "Pre-condition(s)", you might use that term for non-obvious requirements for a function. The assumption then is if it/they are violated that your function won't work. It also implies your function does not check them. One implicit pre-condition on almost all C functions that take a pointer: that the pointer points to accessible memory. Doug Gilbert From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by kanga.kvack.org (Postfix) with ESMTP id 1A9736B000D for ; Fri, 3 Aug 2018 15:07:50 -0400 (EDT) Received: by mail-lf1-f71.google.com with SMTP id r1-v6so733384lfi.16 for ; Fri, 03 Aug 2018 12:07:50 -0700 (PDT) Received: from smtp.infotech.no (smtp.infotech.no. [82.134.31.41]) by mx.google.com with ESMTP id v8-v6si2272518lfd.26.2018.08.03.12.07.48 for ; Fri, 03 Aug 2018 12:07:48 -0700 (PDT) Reply-To: dgilbert@interlog.com Subject: Re: [PATCH v2 2/9] dmapool: cleanup error messages References: <7a943124-c65e-f0ed-cc5c-20b23f021505@cybernetics.com> <20180803162212.GA4718@bombadil.infradead.org> From: Douglas Gilbert Message-ID: <7dac7664-c224-efba-8a7c-ccdc7a4ecd40@interlog.com> Date: Fri, 3 Aug 2018 15:07:44 -0400 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-CA Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Andy Shevchenko , Tony Battersby Cc: Matthew Wilcox , Christoph Hellwig , Marek Szyprowski , Sathya Prakash , Chaitra P B , Suganath Prabu Subramani , iommu@lists.linux-foundation.org, linux-mm , linux-scsi , MPT-FusionLinux.pdl@broadcom.com On 2018-08-03 02:38 PM, Andy Shevchenko wrote: > On Fri, Aug 3, 2018 at 8:03 PM, Tony Battersby wrote: >> On 08/03/2018 12:22 PM, Matthew Wilcox wrote: >>> On Fri, Aug 03, 2018 at 06:59:20PM +0300, Andy Shevchenko wrote: >>>>>>> I'm pretty sure this was created in an order to avoid bad looking (and >>>>>>> in some cases frightening) "NULL device *" part. >>>> JFYI: git log --no-merges --grep 'NULL device \*' >>> I think those commits actually argue in favour of Tony's patch to remove >>> the special casing. Is it really useful to create dma pools with a NULL >>> device? > >> dma_alloc_coherent() does appear to support a NULL dev, so it might make >> sense in theory. But I can't find any in-tree callers that actually >> pass a NULL dev to dma_pool_create(). So for one of the dreaded (NULL >> device *) messages to show up, it would take both a new caller that >> passes a NULL dev to dma_pool_create() and a bug to cause the message to >> be printed. Is that worth the special casing? > > So, then you need to rephrase the commit message explaining this > ("NULL device is wrong to pass in the first place... bla bla bla"). > "Pre-condition(s)", you might use that term for non-obvious requirements for a function. The assumption then is if it/they are violated that your function won't work. It also implies your function does not check them. One implicit pre-condition on almost all C functions that take a pointer: that the pointer points to accessible memory. Doug Gilbert