Hi, On Fri, Jan 8, 2016 at 5:35 AM, Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote: > On Thu, Jan 07, 2016 at 04:36:44PM -0800, Douglas Anderson wrote: >> This patch adds the DMA_ATTR_NOHUGEPAGE attribute to the DMA-mapping >> subsystem. > > It would be nicer to keep things in the positive-logic sense if at all > possible: flags that indicate "we don't want something" tend to end up > with double or triple negatives somewhere which makes understanding > the code much harder. It's a shame we have MADV_NOHUGEPAGE... > > That's not a strong view, but a preference. Thanks for your thoughts. I agree that double-negatives can be confusing... IMHO There are two problems with changing to DMA_ATTR_HUGE_PAGE, though: 1. I have to go and touch all existing DMA-mapping code to set DMA_ATTR_HUGE_PAGE. That will be a big patchset and touch more code, making it more likely to break something. If I have to do that I can, but I prefer not to because changing defaults like this tends to make for subtle bugs. One other thing to think about is that it's my understanding that a large chunk of the ARM developers out there are working on various differing versions of the kernel. If you've got drivers in your tree that haven't been patched to account for the new default but you pick my patch you won't get a compile time error--you'll get a very subtle performance regression. Maybe we don't care too much about those out-of-tree and old kernel folks, but it's something to think about. 2. Personally I think of this attribute like a tristate with 3 values: A) Do whatever you think best. Maybe allocate huge pages if it's super easy and there are lots of huge pages around. AKA similar to today's behavior. B) Try extra hard to get huge pages. Maybe do some extra sorting of pages to build up big ones. Maybe do a little extra collection. Something like that. C) Don't waste even en extra CPU cycle getting huge pages. I don't need them. Encoding a tristate with bitfields basically means you need two opposite bit definitions: DMA_ATTR_NO_HUGE_PAGE and DMA_ATTR_HUGE_PAGE (and specifying both is an error case). Right now I only need two states of the theoretical tristate: A) and C). I could add "DMA_ATTR_HUGE_PAGE" in my patch, but I have no patches queued up to use it. As Christoph Hellwig has reinforced in his reply, folks tend not to want unused code in the kernel, so my thoughts are that someone can add this second attribute when they have a use for it. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
