On Thu, Aug 13, 2015 at 02:13:35PM -0600, Jason Gunthorpe wrote: > On Thu, Aug 13, 2015 at 03:20:49PM -0400, Jérôme Glisse wrote: > > > +#if IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING_HMM) > > +#else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING_HMM */ > > Yuk, what is wrong with > > #if !IS_ENABLED(...) > > ?
Just that latter patches add code btw #if and #else, and that originaly it was a bigger patch that added the #if code #else at the same time. Hence why this patch looks like this. > > > -#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING > > +#if IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING) > > +#if IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING_HMM) > > +#else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING_HMM */ > > Double yuk > > #if !(IS_ENABLED(..) && IS_ENABLED(..)) > > ? Same reason as above. > And the #ifdefs suck, as many as possible should be normal if > statements, and one should think carefully if we really need to remove > fields from structures.. My patch only add #if, i am not responsible for previous code that used #ifdef, i was told to convert to #if and that's what i am doing. Regarding fields, yes this is intentional, ODP is an infrastructure that is private to infiniband and thus needs more fields inside ib struct. While HMM is intended to be a common infrastructure not only for ib device but for other kind of devices too. Cheers, Jérôme -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
