On 12/08/2015 06:15 PM, Or Gerlitz wrote: > On Wed, Dec 9, 2015 at 1:13 AM, Or Gerlitz wrote: >> On Wed, Dec 9, 2015 at 12:59 AM, Jason Gunthorpe >> wrote: >>> On Wed, Dec 09, 2015 at 12:47:55AM +0200, Or Gerlitz wrote: >> >>>> The patch is three liner to add the cached attrs -- >>>> http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK >>>> with that, I will add a 2nd patch that ports all ULPs to use the >>>> cached copy instead of their code which does the query. >> >>>> Actually, why not start with this approach and later decide if we need >>>> to go further of this is enough? >> >>> Or, can we please stop this bikeshedding. Christoph's patch is done, >>> well tested and does a lot more clean up that this hacky three liner. >> >> Christoph patch is here indeed, it does two things >> >> 1. remove all the ULP device attr alloc, device query, attr free hassle >> 2. adds tons of new fields to struct ib_device >> >> I think it just goes too much and needlessly adds tons of these new >> fields directly to struct ib_device where we can have them all well >> scoped into ib_device_attr member or pointer from struct ib_device >> >>> It is a good patch, >> >> I didn't say it's bad, I said I think we can achieve #1 above with way >> less drastic patch, and I'd like to hear the maintainer option, and >> have the chance to argu with the maintainer if needed, yours I heard, >> you like it, I know. > > and I will be OOO for the rest of this week, since this is in the air > for ***months***, it would be fair enough to ask the maintainer not to > cut it this way or another before next week, Doug, agree? Or, you specifically asked me to wait until this week. I made my initial impressions clear (I don't necessarily like the removal of the attr struct, but I like the removal of all of the query calls, and I'm inclined to take the patch in spite of not liking the removal of the struct). Do you have anything to add or have we beat this horse to death? -- Doug Ledford GPG KeyID: 0E572FDD