From: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> To: Konrad Rzeszutek Wilk <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> Cc: laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, will.deacon-5wv7dgnIgG8@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Varun.Sethi-KZfg59tc24xl57MIdRCFDg@public.gmane.org, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Subject: Re: [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions Date: Wed, 20 Aug 2014 16:15:15 +0200 [thread overview] Message-ID: <1689625.AvEiiWCphE@avalon> (raw) In-Reply-To: <20140820130250.GA3120-0iZWjJA6G8GSPmnEAIUT9EEOCMrvLtNR@public.gmane.org> Hi Konrad, On Wednesday 20 August 2014 09:02:50 Konrad Rzeszutek Wilk wrote: > On Tue, Aug 19, 2014 at 10:52:46PM +0200, Laurent Pinchart wrote: > > On Tuesday 19 August 2014 11:40:24 Olav Haugan wrote: > > > On 8/19/2014 9:11 AM, Laurent Pinchart wrote: > > > > On Tuesday 19 August 2014 13:59:54 Joerg Roedel wrote: [snip] > > > >> Okay, since you add these call-backs to all drivers I think I can > > > >> live with not doing a pointer check here. > > > > > > > > I suggested doing a > > > > > > > > if (ops is not NULL) > > > > return ops(); > > > > else > > > > return default_ops(); > > > > > > > > to avoid modifying all drivers. I'm not sure why that wasn't received > > > > with much enthusiasm. > > > > > > Both Thierry R. and Konrad W. argued for modifying the drivers instead > > > so I implemented what the majority wanted. :-) > > > > I'm not blaming you :-) I was just wondering what their rationale was. > > a). To be inline with the usage of this API. > > For this particular case the other functions (but maybe I missed some) > follow the idea that they implement the ops->func for every operation > and they don't have an fallback. That's because the other functions are mandatory, while this particular one is just an optimization and can be implemented generically. > b) Follow what x86 maintainers prefer (based on other API calls, > such as x86_init or any other platform overrides). > > c). When there are new IOMMUs it has to take in-to account all of the > function ops. If they fail to implement all of them the kernel > crashes instead of working (but maybe working incorrectly). I don't think that applies in this case, the generic implementation should work in all cases. > d). Lastly, we also want the bloat of kernel. If the compiler decides > to roll in the fallback implementation for 'iommu_map_sg' in - it will > needlessly expand the kernel wherever we use 'iommu_map_sg' call with > a fallback implementation (which might not be called 99% of time). > > Having the little inline static just do 'func_ops->func' will > stop the compiler from optimizing too much and convert this just > to a simple function. That's an argument I can buy. -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions Date: Wed, 20 Aug 2014 16:15:15 +0200 [thread overview] Message-ID: <1689625.AvEiiWCphE@avalon> (raw) In-Reply-To: <20140820130250.GA3120@laptop.dumpdata.com> Hi Konrad, On Wednesday 20 August 2014 09:02:50 Konrad Rzeszutek Wilk wrote: > On Tue, Aug 19, 2014 at 10:52:46PM +0200, Laurent Pinchart wrote: > > On Tuesday 19 August 2014 11:40:24 Olav Haugan wrote: > > > On 8/19/2014 9:11 AM, Laurent Pinchart wrote: > > > > On Tuesday 19 August 2014 13:59:54 Joerg Roedel wrote: [snip] > > > >> Okay, since you add these call-backs to all drivers I think I can > > > >> live with not doing a pointer check here. > > > > > > > > I suggested doing a > > > > > > > > if (ops is not NULL) > > > > return ops(); > > > > else > > > > return default_ops(); > > > > > > > > to avoid modifying all drivers. I'm not sure why that wasn't received > > > > with much enthusiasm. > > > > > > Both Thierry R. and Konrad W. argued for modifying the drivers instead > > > so I implemented what the majority wanted. :-) > > > > I'm not blaming you :-) I was just wondering what their rationale was. > > a). To be inline with the usage of this API. > > For this particular case the other functions (but maybe I missed some) > follow the idea that they implement the ops->func for every operation > and they don't have an fallback. That's because the other functions are mandatory, while this particular one is just an optimization and can be implemented generically. > b) Follow what x86 maintainers prefer (based on other API calls, > such as x86_init or any other platform overrides). > > c). When there are new IOMMUs it has to take in-to account all of the > function ops. If they fail to implement all of them the kernel > crashes instead of working (but maybe working incorrectly). I don't think that applies in this case, the generic implementation should work in all cases. > d). Lastly, we also want the bloat of kernel. If the compiler decides > to roll in the fallback implementation for 'iommu_map_sg' in - it will > needlessly expand the kernel wherever we use 'iommu_map_sg' call with > a fallback implementation (which might not be called 99% of time). > > Having the little inline static just do 'func_ops->func' will > stop the compiler from optimizing too much and convert this just > to a simple function. That's an argument I can buy. -- Regards, Laurent Pinchart
next prev parent reply other threads:[~2014-08-20 14:15 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-08-11 22:45 [PATCH v5 0/1] Add iommu map_sg/unmap_sg API Olav Haugan 2014-08-11 22:45 ` Olav Haugan 2014-08-11 22:45 ` [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions Olav Haugan 2014-08-11 22:45 ` Olav Haugan [not found] ` <1407797150-515-2-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2014-08-12 1:35 ` Konrad Rzeszutek Wilk 2014-08-12 1:35 ` Konrad Rzeszutek Wilk [not found] ` <20140812013559.GA25121-0iZWjJA6G8GSPmnEAIUT9EEOCMrvLtNR@public.gmane.org> 2014-08-12 16:53 ` Olav Haugan 2014-08-12 16:53 ` Olav Haugan 2014-08-12 1:51 ` Hiroshi Doyu 2014-08-12 1:51 ` Hiroshi Doyu 2014-08-12 10:48 ` Rob Clark 2014-08-12 10:48 ` Rob Clark 2014-08-12 16:56 ` Olav Haugan 2014-08-12 16:56 ` Olav Haugan [not found] ` <53EA472B.3020900-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2014-08-18 14:07 ` joro-zLv9SwRftAIdnm+yROfE0A 2014-08-18 14:07 ` joro at 8bytes.org [not found] ` <20140818140730.GC9809-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2014-08-18 18:32 ` Rob Clark 2014-08-18 18:32 ` Rob Clark [not found] ` <CAF6AEGtGmAQjMc5W5a6x5QiyUc8v0XHcsNAFe65Znc=DDKPS+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-08-18 20:48 ` Olav Haugan 2014-08-18 20:48 ` Olav Haugan [not found] ` <53F266AE.40303-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2014-08-18 21:26 ` joro-zLv9SwRftAIdnm+yROfE0A 2014-08-18 21:26 ` joro at 8bytes.org [not found] ` <20140818212627.GI9809-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2014-08-18 21:32 ` Olav Haugan 2014-08-18 21:32 ` Olav Haugan 2014-08-12 16:55 ` Laurent Pinchart 2014-08-12 16:55 ` Laurent Pinchart 2014-08-12 17:10 ` Olav Haugan 2014-08-12 17:10 ` Olav Haugan 2014-08-18 21:55 ` Joerg Roedel 2014-08-18 21:55 ` Joerg Roedel 2014-08-18 22:47 ` Olav Haugan 2014-08-18 22:47 ` Olav Haugan [not found] ` <53F2829C.2040809-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2014-08-19 11:59 ` Joerg Roedel 2014-08-19 11:59 ` Joerg Roedel [not found] ` <20140819115954.GC16329-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2014-08-19 16:11 ` Laurent Pinchart 2014-08-19 16:11 ` Laurent Pinchart 2014-08-19 18:40 ` Olav Haugan 2014-08-19 18:40 ` Olav Haugan [not found] ` <53F39A18.70409-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2014-08-19 20:52 ` Laurent Pinchart 2014-08-19 20:52 ` Laurent Pinchart 2014-08-20 5:21 ` Thierry Reding 2014-08-20 5:21 ` Thierry Reding 2014-08-20 13:02 ` Konrad Rzeszutek Wilk 2014-08-20 13:02 ` Konrad Rzeszutek Wilk [not found] ` <20140820130250.GA3120-0iZWjJA6G8GSPmnEAIUT9EEOCMrvLtNR@public.gmane.org> 2014-08-20 14:15 ` Laurent Pinchart [this message] 2014-08-20 14:15 ` Laurent Pinchart 2014-08-19 18:37 ` Olav Haugan 2014-08-19 18:37 ` Olav Haugan 2014-09-25 17:01 ` Joerg Roedel 2014-09-25 17:01 ` Joerg Roedel [not found] ` <20140925170108.GE8306-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2014-10-06 19:02 ` Olav Haugan 2014-10-06 19:02 ` Olav Haugan 2014-10-15 9:16 ` Thierry Reding 2014-10-15 9:16 ` Thierry Reding 2014-10-16 17:23 ` Olav Haugan 2014-10-16 17:23 ` Olav Haugan 2014-10-17 9:09 ` Joerg Roedel 2014-10-17 9:09 ` Joerg Roedel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1689625.AvEiiWCphE@avalon \ --to=laurent.pinchart-rylnwiuwjnjg/c1bvhzhaw@public.gmane.org \ --cc=Varun.Sethi-KZfg59tc24xl57MIdRCFDg@public.gmane.org \ --cc=dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \ --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \ --cc=kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \ --cc=konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org \ --cc=laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=will.deacon-5wv7dgnIgG8@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.